Browse Source

cachefile: Only set members that were initialized successfully

Otherwise, things will just start failing later down the stack,
because (a) the lazy getters do not check if building was successful
and (b) any further getter call would return the invalid object
anyway.

Also initialize VS in pkgCache to nullptr by default.

Closes: #818628
Julian Andres Klode 10 years ago
parent
commit
f40fdaa432
3 changed files with 89 additions and 17 deletions
  1. 46 16
      apt-pkg/cachefile.cc
  2. 1 1
      apt-pkg/pkgcache.cc
  3. 42 0
      test/integration/test-bug-818628-unreadable-source

+ 46 - 16
apt-pkg/cachefile.cc

@@ -32,6 +32,7 @@
 #include <unistd.h>
 #include <unistd.h>
 #include <string>
 #include <string>
 #include <vector>
 #include <vector>
+#include <memory>
 
 
 #include <apti18n.h>
 #include <apti18n.h>
 									/*}}}*/
 									/*}}}*/
@@ -69,9 +70,13 @@ public:
    ScopedErrorMerge() { _error->PushToStack(); }
    ScopedErrorMerge() { _error->PushToStack(); }
    ~ScopedErrorMerge() { _error->MergeWithStack(); }
    ~ScopedErrorMerge() { _error->MergeWithStack(); }
 };
 };
+
 bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
 bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
 {
 {
-   if (Cache != NULL)
+   std::unique_ptr<pkgCache> Cache;
+   std::unique_ptr<MMap> Map;
+
+   if (this->Cache != NULL)
       return true;
       return true;
 
 
    ScopedErrorMerge sem;
    ScopedErrorMerge sem;
@@ -80,11 +85,16 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
       FileFd file(_config->FindFile("Dir::Cache::pkgcache"), FileFd::ReadOnly);
       FileFd file(_config->FindFile("Dir::Cache::pkgcache"), FileFd::ReadOnly);
       if (file.IsOpen() == false || file.Failed())
       if (file.IsOpen() == false || file.Failed())
 	 return false;
 	 return false;
-      Map = new MMap(file, MMap::Public|MMap::ReadOnly);
+      Map.reset(new MMap(file, MMap::Public|MMap::ReadOnly));
       if (unlikely(Map->validData() == false))
       if (unlikely(Map->validData() == false))
 	 return false;
 	 return false;
-      Cache = new pkgCache(Map);
-      return _error->PendingError() == false;
+      Cache.reset(new pkgCache(Map.get()));
+      if (_error->PendingError() == true)
+	 return false;
+
+      this->Cache = Cache.release();
+      this->Map = Map.release();
+      return true;
    }
    }
 
 
    if (WithLock == true)
    if (WithLock == true)
@@ -94,11 +104,15 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
    if (_error->PendingError() == true)
    if (_error->PendingError() == true)
       return false;
       return false;
 
 
-   BuildSourceList(Progress);
+   if (BuildSourceList(Progress) == false)
+      return false;
 
 
    // Read the caches
    // Read the caches
-   Cache = nullptr;
-   bool Res = pkgCacheGenerator::MakeStatusCache(*SrcList,Progress,&Map, &Cache, true);
+   MMap *TmpMap = nullptr;
+   pkgCache *TmpCache = nullptr;
+   bool Res = pkgCacheGenerator::MakeStatusCache(*SrcList,Progress,&TmpMap, &TmpCache, true);
+   Map.reset(TmpMap);
+   Cache.reset(TmpCache);
    if (Progress != NULL)
    if (Progress != NULL)
       Progress->Done();
       Progress->Done();
    if (Res == false)
    if (Res == false)
@@ -109,9 +123,12 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
       _error->Warning(_("You may want to run apt-get update to correct these problems"));
       _error->Warning(_("You may want to run apt-get update to correct these problems"));
 
 
    if (Cache == nullptr)
    if (Cache == nullptr)
-      Cache = new pkgCache(Map);
+      Cache.reset(new pkgCache(Map.get()));
    if (_error->PendingError() == true)
    if (_error->PendingError() == true)
       return false;
       return false;
+   this->Map = Map.release();
+   this->Cache = Cache.release();
+
    return true;
    return true;
 }
 }
 									/*}}}*/
 									/*}}}*/
@@ -120,12 +137,14 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
 /* */
 /* */
 bool pkgCacheFile::BuildSourceList(OpProgress * /*Progress*/)
 bool pkgCacheFile::BuildSourceList(OpProgress * /*Progress*/)
 {
 {
-   if (SrcList != NULL)
+   std::unique_ptr<pkgSourceList> SrcList;
+   if (this->SrcList != NULL)
       return true;
       return true;
 
 
-   SrcList = new pkgSourceList();
+   SrcList.reset(new pkgSourceList());
    if (SrcList->ReadMainList() == false)
    if (SrcList->ReadMainList() == false)
       return _error->Error(_("The list of sources could not be read."));
       return _error->Error(_("The list of sources could not be read."));
+   this->SrcList = SrcList.release();
    return true;
    return true;
 }
 }
 									/*}}}*/
 									/*}}}*/
@@ -134,16 +153,18 @@ bool pkgCacheFile::BuildSourceList(OpProgress * /*Progress*/)
 /* */
 /* */
 bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/)
 bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/)
 {
 {
-   if (Policy != NULL)
+   std::unique_ptr<pkgPolicy> Policy;
+   if (this->Policy != NULL)
       return true;
       return true;
 
 
-   Policy = new pkgPolicy(Cache);
+   Policy.reset(new pkgPolicy(Cache));
    if (_error->PendingError() == true)
    if (_error->PendingError() == true)
       return false;
       return false;
 
 
    if (ReadPinFile(*Policy) == false || ReadPinDir(*Policy) == false)
    if (ReadPinFile(*Policy) == false || ReadPinDir(*Policy) == false)
       return false;
       return false;
 
 
+   this->Policy = Policy.release();
    return true;
    return true;
 }
 }
 									/*}}}*/
 									/*}}}*/
@@ -152,17 +173,21 @@ bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/)
 /* */
 /* */
 bool pkgCacheFile::BuildDepCache(OpProgress *Progress)
 bool pkgCacheFile::BuildDepCache(OpProgress *Progress)
 {
 {
-   if (DCache != NULL)
+   std::unique_ptr<pkgDepCache> DCache;
+   if (this->DCache != NULL)
       return true;
       return true;
 
 
    if (BuildPolicy(Progress) == false)
    if (BuildPolicy(Progress) == false)
       return false;
       return false;
 
 
-   DCache = new pkgDepCache(Cache,Policy);
+   DCache.reset(new pkgDepCache(Cache,Policy));
    if (_error->PendingError() == true)
    if (_error->PendingError() == true)
       return false;
       return false;
+   if (DCache->Init(Progress) == false)
+      return false;
 
 
-   return DCache->Init(Progress);
+   this->DCache = DCache.release();
+   return true;
 }
 }
 									/*}}}*/
 									/*}}}*/
 // CacheFile::Open - Open the cache files, creating if necessary	/*{{{*/
 // CacheFile::Open - Open the cache files, creating if necessary	/*{{{*/
@@ -225,7 +250,12 @@ bool pkgCacheFile::AddIndexFile(pkgIndexFile * const File)		/*{{{*/
 	       return false;
 	       return false;
 	 }
 	 }
 	 Cache = new pkgCache(Map);
 	 Cache = new pkgCache(Map);
-	 return _error->PendingError() == false;
+	 if (_error->PendingError() == true) {
+	    delete Cache;
+	    Cache = nullptr;
+	    return false;
+	 }
+	 return true;
       }
       }
       else
       else
       {
       {

+ 1 - 1
apt-pkg/pkgcache.cc

@@ -127,7 +127,7 @@ bool pkgCache::Header::CheckSizes(Header &Against) const
 // ---------------------------------------------------------------------
 // ---------------------------------------------------------------------
 /* */
 /* */
 APT_IGNORE_DEPRECATED_PUSH
 APT_IGNORE_DEPRECATED_PUSH
-pkgCache::pkgCache(MMap *Map, bool DoMap) : Map(*Map), d(NULL)
+pkgCache::pkgCache(MMap *Map, bool DoMap) : Map(*Map), VS(nullptr), d(NULL)
 {
 {
    // call getArchitectures() with cached=false to ensure that the 
    // call getArchitectures() with cached=false to ensure that the 
    // architectures cache is re-evaulated. this is needed in cases
    // architectures cache is re-evaulated. this is needed in cases

+ 42 - 0
test/integration/test-bug-818628-unreadable-source

@@ -0,0 +1,42 @@
+#!/bin/sh
+set -e
+
+TESTDIR="$(readlink -f "$(dirname "$0")")"
+. "$TESTDIR/framework"
+setupenvironment
+configarchitecture 'amd64' 'i386'
+
+if [ "$(id -u)" = "0" ]; then
+    msgskip "Tests for unreadable sources.list do not work as root"
+    exit 0
+fi
+
+insertinstalledpackage 'apt' 'i386' '1'
+buildsimplenativepackage 'apt' 'i386' '2' 'unstable'
+
+setupaptarchive
+
+# Test unreadable sources.list files
+chmod -r rootdir/etc/apt/sources.list.d
+
+testfailureequal "E: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/ - opendir (13: Permission denied)
+W: You may want to run apt-get update to correct these problems
+E: The package cache file is corrupted" aptcache policy apt
+testfailureequal "E: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/ - opendir (13: Permission denied)
+W: You may want to run apt-get update to correct these problems
+E: The package cache file is corrupted" apt search apt
+
+chmod +r rootdir/etc/apt/sources.list.d
+
+
+# Test unreadable sources.list files
+chmod -r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list
+
+testfailureequal "E: Opening $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - ifstream::ifstream (13: Permission denied)
+E: The list of sources could not be read.
+E: Opening $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - ifstream::ifstream (13: Permission denied)
+E: The list of sources could not be read." aptcache policy apt
+testfailureequal "E: Opening $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - ifstream::ifstream (13: Permission denied)
+E: The list of sources could not be read." apt search apt
+
+chmod +r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list