Imported Upstream version 14.48.2

diff --git a/VERSION.cmake b/VERSION.cmake
index 8863f1e..c22aeb2 100644 (file)
--- a/VERSION.cmake
+++ b/VERSION.cmake
@@ -61,8 +61,8 @@
 SET(LIBZYPP_MAJOR "14")
 SET(LIBZYPP_COMPATMINOR "39")
 SET(LIBZYPP_MINOR "48")
-SET(LIBZYPP_PATCH "1")
+SET(LIBZYPP_PATCH "2")
 #
-# LAST RELEASED: 14.48.1 (39)
+# LAST RELEASED: 14.48.2 (39)
 # (The number in parenthesis is LIBZYPP_COMPATMINOR)
 #=======

diff --git a/package/libzypp.changes b/package/libzypp.changes
index 389dd5b..3353b2b 100644 (file)
--- a/package/libzypp.changes
+++ b/package/libzypp.changes
@@ -1,4 +1,10 @@
 -------------------------------------------------------------------
+Thu Jul 25 17:19:51 CEST 2019 - ma@suse.de
+
+- MediaCurl: Fix leaking filedescriptors (bsc#1116995)
+- version 14.48.2 (39)
+
+-------------------------------------------------------------------
 Fri Jul 19 10:22:58 CEST 2019 - ma@suse.de
 
 - commit: Run file conflict check on dry-run (best with download-only)

diff --git a/zypp/AutoDispose.h b/zypp/AutoDispose.h
index bb01549..6b789f2 100644 (file)
--- a/zypp/AutoDispose.h
+++ b/zypp/AutoDispose.h
@@ -194,6 +194,45 @@ namespace zypp
     inline std::ostream & operator<<( std::ostream & str, const AutoDispose<_Tp> & obj )
     { return str << obj.value(); }
 
+
+  ///////////////////////////////////////////////////////////////////
+  /// \class AutoFD
+  /// \brief \ref AutoDispose\<int>  calling \c ::close
+  /// \ingroup g_RAII
+  ///////////////////////////////////////////////////////////////////
+  struct AutoFD : public AutoDispose<int>
+  {
+    AutoFD( int fd_r = -1 ) : AutoDispose<int>( fd_r, [] ( int fd_r ) { if ( fd_r != -1 ) ::close( fd_r ); } ) {}
+  };
+
+  ///////////////////////////////////////////////////////////////////
+  /// \class AutoFILE
+  /// \brief \ref AutoDispose\<FILE*> calling \c ::fclose
+  /// \see \ref AutoDispose
+  /// \ingroup g_RAII
+  ///////////////////////////////////////////////////////////////////
+  struct AutoFILE : public AutoDispose<FILE*>
+  {
+    AutoFILE( FILE* file_r = nullptr ) : AutoDispose<FILE*>( file_r, [] ( FILE* file_r ) { if ( file_r ) ::fclose( file_r ); } ) {}
+  };
+
+  ///////////////////////////////////////////////////////////////////
+  /// \class AutoFREE<Tp>
+  /// \brief \ref AutoDispose\<Tp*> calling \c ::free
+  /// \ingroup g_RAII
+  ///////////////////////////////////////////////////////////////////
+  template <typename Tp>
+  struct AutoFREE : public AutoDispose<Tp*>
+  {
+    AutoFREE( Tp* ptr_r = nullptr ) : AutoDispose<Tp*>( ptr_r, [] ( Tp* ptr_r ) { if ( ptr_r ) ::free( ptr_r ); } ) {}
+    AutoFREE( void* ptr_r ) : AutoFREE( static_cast<Tp*>(ptr_r) ) {}
+  };
+
+  template <>
+  struct AutoFREE<void> : public AutoDispose<void*>
+  {
+    AutoFREE( void* ptr_r = nullptr ) : AutoDispose<void*>( ptr_r, [] ( void* ptr_r ) { if ( ptr_r ) ::free( ptr_r ); } ) {}
+  };
   /////////////////////////////////////////////////////////////////
 } // namespace zypp
 ///////////////////////////////////////////////////////////////////

diff --git a/zypp/MediaSetAccess.cc b/zypp/MediaSetAccess.cc
index 1be0824..5dad952 100644 (file)
--- a/zypp/MediaSetAccess.cc
+++ b/zypp/MediaSetAccess.cc
@@ -46,7 +46,9 @@ IMPL_PTR_TYPE(MediaSetAccess);
   {
     try
     {
-      release();
+      media::MediaManager manager;
+      for ( const auto & mm : _medias )
+       manager.close( mm.second );
     }
     catch(...) {} // don't let exception escape a dtor.
   }

diff --git a/zypp/media/MediaCurl.cc b/zypp/media/MediaCurl.cc
index 90e6194..d1db7f9 100644 (file)
--- a/zypp/media/MediaCurl.cc
+++ b/zypp/media/MediaCurl.cc
@@ -1217,7 +1217,7 @@ bool MediaCurl::doGetDoesFileExist( const Pathname & filename ) const
     ZYPP_THROW(MediaCurlSetOptException(url, _curlError));
   }
 
-  FILE *file = ::fopen( "/dev/null", "w" );
+  AutoFILE file { ::fopen( "/dev/null", "w" ) };
   if ( !file ) {
       ERR << "fopen failed for /dev/null" << endl;
       curl_easy_setopt( _curl, CURLOPT_NOBODY, 0L);
@@ -1234,9 +1234,8 @@ bool MediaCurl::doGetDoesFileExist( const Pathname & filename ) const
       ZYPP_THROW(MediaWriteException("/dev/null"));
   }
 
-  ret = curl_easy_setopt( _curl, CURLOPT_WRITEDATA, file );
+  ret = curl_easy_setopt( _curl, CURLOPT_WRITEDATA, (*file) );
   if ( ret != 0 ) {
-      ::fclose(file);
       std::string err( _curlError);
       curl_easy_setopt( _curl, CURLOPT_RANGE, NULL );
       curl_easy_setopt( _curl, CURLOPT_NOBODY, 0L);
@@ -1283,10 +1282,6 @@ bool MediaCurl::doGetDoesFileExist( const Pathname & filename ) const
     }
   }
 
-  // if the code is not zero, close the file
-  if ( ok != 0 )
-      ::fclose(file);
-
   // as we are not having user interaction, the user can't cancel
   // the file existence checking, a callback or timeout return code
   // will be always a timeout.
@@ -1356,34 +1351,34 @@ void MediaCurl::doGetFileCopy(const Pathname & filename , const Pathname & targe
     if( assert_dir( dest.dirname() ) )
     {
       DBG << "assert_dir " << dest.dirname() << " failed" << endl;
-      Url url(getFileUrl(filename));
-      ZYPP_THROW( MediaSystemException(url, "System error on " + dest.dirname().asString()) );
-    }
-    string destNew = target.asString() + ".new.zypp.XXXXXX";
-    char *buf = ::strdup( destNew.c_str());
-    if( !buf)
-    {
-      ERR << "out of memory for temp file name" << endl;
-      Url url(getFileUrl(filename));
-      ZYPP_THROW(MediaSystemException(url, "out of memory for temp file name"));
+      ZYPP_THROW( MediaSystemException(getFileUrl(filename), "System error on " + dest.dirname().asString()) );
     }
 
-    int tmp_fd = ::mkostemp( buf, O_CLOEXEC );
-    if( tmp_fd == -1)
+    ManagedFile destNew { target.extend( ".new.zypp.XXXXXX" ) };
+    AutoFILE file;
     {
-      free( buf);
-      ERR << "mkstemp failed for file '" << destNew << "'" << endl;
-      ZYPP_THROW(MediaWriteException(destNew));
-    }
-    destNew = buf;
-    free( buf);
-
-    FILE *file = ::fdopen( tmp_fd, "we" );
-    if ( !file ) {
-      ::close( tmp_fd);
-      filesystem::unlink( destNew );
-      ERR << "fopen failed for file '" << destNew << "'" << endl;
-      ZYPP_THROW(MediaWriteException(destNew));
+      AutoFREE<char> buf { ::strdup( (*destNew).c_str() ) };
+      if( ! buf )
+      {
+       ERR << "out of memory for temp file name" << endl;
+       ZYPP_THROW(MediaSystemException(getFileUrl(filename), "out of memory for temp file name"));
+      }
+
+      AutoFD tmp_fd { ::mkostemp( buf, O_CLOEXEC ) };
+      if( tmp_fd == -1 )
+      {
+       ERR << "mkstemp failed for file '" << destNew << "'" << endl;
+       ZYPP_THROW(MediaWriteException(destNew));
+      }
+      destNew = ManagedFile( (*buf), filesystem::unlink );
+
+      file = ::fdopen( tmp_fd, "we" );
+      if ( ! file )
+      {
+       ERR << "fopen failed for file '" << destNew << "'" << endl;
+       ZYPP_THROW(MediaWriteException(destNew));
+      }
+      tmp_fd.resetDispose();   // don't close it here! ::fdopen moved ownership to file
     }
 
     DBG << "dest: " << dest << endl;
@@ -1406,8 +1401,6 @@ void MediaCurl::doGetFileCopy(const Pathname & filename , const Pathname & targe
     }
     catch (Exception &e)
     {
-      ::fclose( file );
-      filesystem::unlink( destNew );
       curl_easy_setopt(_curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_NONE);
       curl_easy_setopt(_curl, CURLOPT_TIMEVALUE, 0L);
       ZYPP_RETHROW(e);
@@ -1441,22 +1434,20 @@ void MediaCurl::doGetFileCopy(const Pathname & filename , const Pathname & targe
       {
         ERR << "Failed to chmod file " << destNew << endl;
       }
-      if (::fclose( file ))
+
+      file.resetDispose();     // we're going to close it manually here
+      if ( ::fclose( file ) )
       {
         ERR << "Fclose failed for file '" << destNew << "'" << endl;
         ZYPP_THROW(MediaWriteException(destNew));
       }
+
       // move the temp file into dest
       if ( rename( destNew, dest ) != 0 ) {
         ERR << "Rename failed" << endl;
         ZYPP_THROW(MediaWriteException(dest));
       }
-    }
-    else
-    {
-      // close and remove the temp file
-      ::fclose( file );
-      filesystem::unlink( destNew );
+      destNew.resetDispose();  // no more need to unlink it
     }
 
     DBG << "done: " << PathInfo(dest) << endl;

diff --git a/zypp/media/MediaMultiCurl.cc b/zypp/media/MediaMultiCurl.cc
index 0d921ef..8eadeb0 100644 (file)
--- a/zypp/media/MediaMultiCurl.cc
+++ b/zypp/media/MediaMultiCurl.cc
@@ -26,6 +26,7 @@
 #include "zypp/base/Logger.h"
 #include "zypp/media/MediaMultiCurl.h"
 #include "zypp/media/MetaLinkParser.h"
+#include "zypp/ManagedFile.h"
 
 using namespace std;
 using namespace zypp::base;
@@ -1300,35 +1301,36 @@ void MediaMultiCurl::doGetFileCopy( const Pathname & filename , const Pathname &
   if( assert_dir( dest.dirname() ) )
   {
     DBG << "assert_dir " << dest.dirname() << " failed" << endl;
-    Url url(getFileUrl(filename));
-    ZYPP_THROW( MediaSystemException(url, "System error on " + dest.dirname().asString()) );
-  }
-  string destNew = target.asString() + ".new.zypp.XXXXXX";
-  char *buf = ::strdup( destNew.c_str());
-  if( !buf)
-  {
-    ERR << "out of memory for temp file name" << endl;
-    Url url(getFileUrl(filename));
-    ZYPP_THROW(MediaSystemException(url, "out of memory for temp file name"));
+    ZYPP_THROW( MediaSystemException(getFileUrl(filename), "System error on " + dest.dirname().asString()) );
   }
 
-  int tmp_fd = ::mkostemp( buf, O_CLOEXEC );
-  if( tmp_fd == -1)
+  ManagedFile destNew { target.extend( ".new.zypp.XXXXXX" ) };
+  AutoFILE file;
   {
-    free( buf);
-    ERR << "mkstemp failed for file '" << destNew << "'" << endl;
-    ZYPP_THROW(MediaWriteException(destNew));
-  }
-  destNew = buf;
-  free( buf);
-
-  FILE *file = ::fdopen( tmp_fd, "we" );
-  if ( !file ) {
-    ::close( tmp_fd);
-    filesystem::unlink( destNew );
-    ERR << "fopen failed for file '" << destNew << "'" << endl;
-    ZYPP_THROW(MediaWriteException(destNew));
+    AutoFREE<char> buf { ::strdup( (*destNew).c_str() ) };
+    if( ! buf )
+    {
+      ERR << "out of memory for temp file name" << endl;
+      ZYPP_THROW(MediaSystemException(getFileUrl(filename), "out of memory for temp file name"));
+    }
+
+    AutoFD tmp_fd { ::mkostemp( buf, O_CLOEXEC ) };
+    if( tmp_fd == -1 )
+    {
+      ERR << "mkstemp failed for file '" << destNew << "'" << endl;
+      ZYPP_THROW(MediaWriteException(destNew));
+    }
+    destNew = ManagedFile( (*buf), filesystem::unlink );
+
+    file = ::fdopen( tmp_fd, "we" );
+    if ( ! file )
+    {
+      ERR << "fopen failed for file '" << destNew << "'" << endl;
+      ZYPP_THROW(MediaWriteException(destNew));
+    }
+    tmp_fd.resetDispose();     // don't close it here! ::fdopen moved ownership to file
   }
+
   DBG << "dest: " << dest << endl;
   DBG << "temp: " << destNew << endl;
 
@@ -1347,15 +1349,13 @@ void MediaMultiCurl::doGetFileCopy( const Pathname & filename , const Pathname &
   curl_easy_setopt(_curl, CURLOPT_HTTPHEADER, _customHeadersMetalink);
   // change to our own progress funcion
   curl_easy_setopt(_curl, CURLOPT_PROGRESSFUNCTION, &progressCallback);
-  curl_easy_setopt(_curl, CURLOPT_PRIVATE, file);
+  curl_easy_setopt(_curl, CURLOPT_PRIVATE, (*file) );  // important to pass the FILE* explicitly (passing through varargs)
   try
     {
       MediaCurl::doGetFileCopyFile(filename, dest, file, report, expectedFileSize_r, options);
     }
   catch (Exception &ex)
     {
-      ::fclose(file);
-      filesystem::unlink(destNew);
       curl_easy_setopt(_curl, CURLOPT_TIMECONDITION, CURL_TIMECOND_NONE);
       curl_easy_setopt(_curl, CURLOPT_TIMEVALUE, 0L);
       curl_easy_setopt(_curl, CURLOPT_HTTPHEADER, _customHeaders);
@@ -1398,24 +1398,23 @@ void MediaMultiCurl::doGetFileCopy( const Pathname & filename , const Pathname &
       // some proxies do not store the content type, so also look at the file to find
       // out if we received a metalink (bnc#649925)
       fflush(file);
-      if (looks_like_metalink(Pathname(destNew)))
+      if (looks_like_metalink(destNew))
        ismetalink = true;
     }
 
   if (ismetalink)
     {
       bool userabort = false;
-      fclose(file);
-      file = NULL;
       Pathname failedFile = ZConfig::instance().repoCachePath() / "MultiCurl.failed";
+      file = nullptr;  // explicitly close destNew before the parser reads it.
       try
        {
          MetaLinkParser mlp;
-         mlp.parse(Pathname(destNew));
+         mlp.parse(destNew);
          MediaBlockList bl = mlp.getBlockList();
          vector<Url> urls = mlp.getUrls();
          XXX << bl << endl;
-         file = fopen(destNew.c_str(), "w+e");
+         file = fopen((*destNew).c_str(), "w+e");
          if (!file)
            ZYPP_THROW(MediaWriteException(destNew));
          if (PathInfo(target).isExist())
@@ -1454,9 +1453,7 @@ void MediaMultiCurl::doGetFileCopy( const Pathname & filename , const Pathname &
       catch (Exception &ex)
        {
          // something went wrong. fall back to normal download
-         if (file)
-           fclose(file);
-         file = NULL;
+         file = nullptr;       // explicitly close destNew before moving it
          if (PathInfo(destNew).size() >= 63336)
            {
              ::unlink(failedFile.asString().c_str());
@@ -1464,10 +1461,9 @@ void MediaMultiCurl::doGetFileCopy( const Pathname & filename , const Pathname &
            }
          if (userabort)
            {
-             filesystem::unlink(destNew);
              ZYPP_RETHROW(ex);
            }
-         file = fopen(destNew.c_str(), "w+e");
+         file = fopen((*destNew).c_str(), "w+e");
          if (!file)
            ZYPP_THROW(MediaWriteException(destNew));
          MediaCurl::doGetFileCopyFile(filename, dest, file, report, expectedFileSize_r, options | OPTION_NO_REPORT_START);
@@ -1478,17 +1474,22 @@ void MediaMultiCurl::doGetFileCopy( const Pathname & filename , const Pathname &
     {
       ERR << "Failed to chmod file " << destNew << endl;
     }
+
+  file.resetDispose(); // we're going to close it manually here
   if (::fclose(file))
     {
       filesystem::unlink(destNew);
       ERR << "Fclose failed for file '" << destNew << "'" << endl;
       ZYPP_THROW(MediaWriteException(destNew));
     }
+
   if ( rename( destNew, dest ) != 0 )
     {
       ERR << "Rename failed" << endl;
       ZYPP_THROW(MediaWriteException(dest));
     }
+  destNew.resetDispose();      // no more need to unlink it
+
   DBG << "done: " << PathInfo(dest) << endl;
 }