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

diff --git a/apt-pkg/cachefile.cc b/apt-pkg/cachefile.cc
index 909da5cb4d6fe25a69aa6707e0317cd8a8e65666..0e21929731dff13cc56dcd4a3400cd6ca664973c 100644 (file)
--- a/apt-pkg/cachefile.cc
+++ b/apt-pkg/cachefile.cc
@@ -32,6 +32,7 @@
 #include <unistd.h>
 #include <string>
 #include <vector>
 #include <unistd.h>
 #include <string>
 #include <vector>

+#include <memory>

 
 #include <apti18n.h>
                                                                        /*}}}*/
 
 #include <apti18n.h>
                                                                        /*}}}*/


@@ -69,9 +70,13 @@ public:
    ScopedErrorMerge() { _error->PushToStack(); }
    ~ScopedErrorMerge() { _error->MergeWithStack(); }
 };
    ScopedErrorMerge() { _error->PushToStack(); }
    ~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;
 
    ScopedErrorMerge sem;
       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;
       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;
       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)
    }
 
    if (WithLock == true)


@@ -94,11 +104,15 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
    if (_error->PendingError() == true)
       return false;
 
    if (_error->PendingError() == true)
       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)
       Progress->Done();
    if (Res == false)
    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)
       _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;
    if (_error->PendingError() == true)
       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)
       return _error->Error(_("The list of sources could not be read."));
    if (SrcList->ReadMainList() == false)
       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)
       return false;
 
    if (ReadPinFile(*Policy) == false || ReadPinDir(*Policy) == false)
       return false;
 
    if (_error->PendingError() == true)
       return false;
 
    if (ReadPinFile(*Policy) == false || ReadPinDir(*Policy) == 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;
 
    if (BuildPolicy(Progress) == false)
       return false;
 
       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 (_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       /*{{{*/
 }
                                                                        /*}}}*/
 // 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 false;
         }
         Cache = new pkgCache(Map);

-        return _error->PendingError() == false;
+        if (_error->PendingError() == true) {
+           delete Cache;
+           Cache = nullptr;
+           return false;
+        }
+        return true;

       }
       else
       {
       }
       else
       {

diff --git a/apt-pkg/pkgcache.cc b/apt-pkg/pkgcache.cc
index 159b5e00e5c1dfec1a1a62bf902e4f74d4728739..83cb6149ef157f98b361ae321f1f834cf9f24017 100644 (file)
--- 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
 // ---------------------------------------------------------------------
 /* */
 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
 {
    // 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 (executable)
index 0000000..84776dd
--- /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