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

3 files changed, 89 insertions, 17 deletions

diff --git a/apt-pkg/cachefile.cc b/apt-pkg/cachefile.cc
index 909da5cb4..0e2192973 100644
--- a/apt-pkg/cachefile.cc
+++ b/apt-pkg/cachefile.cc
@@ -32,6 +32,7 @@
 #include <unistd.h>
 #include <string>
 #include <vector>
+#include <memory>
 
 #include <apti18n.h>
 									/*}}}*/
@@ -69,9 +70,13 @@ public:
    ScopedErrorMerge() { _error->PushToStack(); }
    ~ScopedErrorMerge() { _error->MergeWithStack(); }
 };
+
 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;
 
    ScopedErrorMerge sem;
@@ -80,11 +85,16 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
       FileFd file(_config->FindFile("Dir::Cache::pkgcache"), FileFd::ReadOnly);
       if (file.IsOpen() == false || file.Failed())
 	 return false;
-      Map = new MMap(file, MMap::Public|MMap::ReadOnly);
+      Map.reset(new MMap(file, MMap::Public|MMap::ReadOnly));
       if (unlikely(Map->validData() == 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)
@@ -94,11 +104,15 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
    if (_error->PendingError() == true)
       return false;
 
-   BuildSourceList(Progress);
+   if (BuildSourceList(Progress) == false)
+      return false;
 
    // 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)
       Progress->Done();
    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"));
 
    if (Cache == nullptr)
-      Cache = new pkgCache(Map);
+      Cache.reset(new pkgCache(Map.get()));
    if (_error->PendingError() == true)
       return false;
+   this->Map = Map.release();
+   this->Cache = Cache.release();
+
    return true;
 }
 									/*}}}*/
@@ -120,12 +137,14 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
 /* */
 bool pkgCacheFile::BuildSourceList(OpProgress * /*Progress*/)
 {
-   if (SrcList != NULL)
+   std::unique_ptr<pkgSourceList> SrcList;
+   if (this->SrcList != NULL)
       return true;
 
-   SrcList = new pkgSourceList();
+   SrcList.reset(new pkgSourceList());
    if (SrcList->ReadMainList() == false)
       return _error->Error(_("The list of sources could not be read."));
+   this->SrcList = SrcList.release();
    return true;
 }
 									/*}}}*/
@@ -134,16 +153,18 @@ bool pkgCacheFile::BuildSourceList(OpProgress * /*Progress*/)
 /* */
 bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/)
 {
-   if (Policy != NULL)
+   std::unique_ptr<pkgPolicy> Policy;
+   if (this->Policy != NULL)
       return true;
 
-   Policy = new pkgPolicy(Cache);
+   Policy.reset(new pkgPolicy(Cache));
    if (_error->PendingError() == true)
       return false;
 
    if (ReadPinFile(*Policy) == false || ReadPinDir(*Policy) == false)
       return false;
 
+   this->Policy = Policy.release();
    return true;
 }
 									/*}}}*/
@@ -152,17 +173,21 @@ bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/)
 /* */
 bool pkgCacheFile::BuildDepCache(OpProgress *Progress)
 {
-   if (DCache != NULL)
+   std::unique_ptr<pkgDepCache> DCache;
+   if (this->DCache != NULL)
       return true;
 
    if (BuildPolicy(Progress) == false)
       return false;
 
-   DCache = new pkgDepCache(Cache,Policy);
+   DCache.reset(new pkgDepCache(Cache,Policy));
    if (_error->PendingError() == true)
       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	/*{{{*/
@@ -225,7 +250,12 @@ bool pkgCacheFile::AddIndexFile(pkgIndexFile * const File)		/*{{{*/
 	       return false;
 	 }
 	 Cache = new pkgCache(Map);
-	 return _error->PendingError() == false;
+	 if (_error->PendingError() == true) {
+	    delete Cache;
+	    Cache = nullptr;
+	    return false;
+	 }
+	 return true;
       }
       else
       {
diff --git a/apt-pkg/pkgcache.cc b/apt-pkg/pkgcache.cc
index 159b5e00e..83cb6149e 100644
--- a/apt-pkg/pkgcache.cc
+++ b/apt-pkg/pkgcache.cc
@@ -127,7 +127,7 @@ bool pkgCache::Header::CheckSizes(Header &Against) const
 // ---------------------------------------------------------------------
 /* */
 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 
    // architectures cache is re-evaulated. this is needed in cases
diff --git a/test/integration/test-bug-818628-unreadable-source b/test/integration/test-bug-818628-unreadable-source
new file mode 100755
index 000000000..84776dd8d
--- /dev/null
+++ b/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