From 51751106976b1c6afa8f7991790db87b239fcc84 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 15 Jul 2017 15:08:35 +0200 Subject: show warnings instead of errors if files are unreadable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We used to fail on unreadable config/preferences/sources files, but at least for sources we didn't in the past and it seems harsh to refuse to work because of a single file, especially as the error messages are inconsistent and end up being silly (like suggesting to run apt update to fix the problem…). LP: #1701852 --- apt-pkg/cachefile.cc | 6 +-- apt-pkg/contrib/configuration.cc | 11 ++-- apt-pkg/contrib/fileutl.cc | 13 ++++- apt-pkg/contrib/fileutl.h | 1 + apt-pkg/init.cc | 20 ++++--- apt-pkg/policy.cc | 6 +-- apt-pkg/sourcelist.cc | 36 +++++-------- test/integration/test-bug-818628-unreadable-source | 63 ++++++++++++---------- 8 files changed, 79 insertions(+), 77 deletions(-) diff --git a/apt-pkg/cachefile.cc b/apt-pkg/cachefile.cc index 0116308e5..c070f4b9e 100644 --- a/apt-pkg/cachefile.cc +++ b/apt-pkg/cachefile.cc @@ -161,11 +161,11 @@ bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/) if (_error->PendingError() == true) return false; - if (ReadPinFile(*Policy) == false || ReadPinDir(*Policy) == false) - return false; + ReadPinFile(*Policy); + ReadPinDir(*Policy); this->Policy = Policy.release(); - return true; + return _error->PendingError() == false; } /*}}}*/ // CacheFile::BuildDepCache - Open and build the dependency cache /*{{{*/ diff --git a/apt-pkg/contrib/configuration.cc b/apt-pkg/contrib/configuration.cc index 65242bec5..eb873bdba 100644 --- a/apt-pkg/contrib/configuration.cc +++ b/apt-pkg/contrib/configuration.cc @@ -1150,13 +1150,10 @@ bool ReadConfigFile(Configuration &Conf,const string &FName,bool const &AsSectio bool ReadConfigDir(Configuration &Conf,const string &Dir, bool const &AsSectional, unsigned const &Depth) { - vector const List = GetListOfFilesInDir(Dir, "conf", true, true); - - // Read the files - for (vector::const_iterator I = List.begin(); I != List.end(); ++I) - if (ReadConfigFile(Conf,*I,AsSectional,Depth) == false) - return false; - return true; + bool good = true; + for (auto const &I : GetListOfFilesInDir(Dir, "conf", true, true)) + good = ReadConfigFile(Conf, I, AsSectional, Depth) && good; + return good; } /*}}}*/ // MatchAgainstConfig Constructor /*{{{*/ diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 7633b07ef..33f4f7e09 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -402,7 +402,10 @@ std::vector GetListOfFilesInDir(string const &Dir, std::vector c DIR *D = opendir(Dir.c_str()); if (D == 0) { - _error->Errno("opendir",_("Unable to read %s"),Dir.c_str()); + if (errno == EACCES) + _error->WarningE("opendir", _("Unable to read %s"), Dir.c_str()); + else + _error->Errno("opendir", _("Unable to read %s"), Dir.c_str()); return List; } @@ -3126,7 +3129,13 @@ bool DropPrivileges() /*{{{*/ /*}}}*/ bool OpenConfigurationFileFd(std::string const &File, FileFd &Fd) /*{{{*/ { + int const fd = open(File.c_str(), O_RDONLY | O_CLOEXEC | O_NOCTTY); + if (fd == -1) + return _error->WarningE("open", _("Unable to read %s"), File.c_str()); APT::Configuration::Compressor none(".", "", "", nullptr, nullptr, 0); - return Fd.Open(File, FileFd::ReadOnly, none); + if (Fd.OpenDescriptor(fd, FileFd::ReadOnly, none) == false) + return false; + Fd.SetFileName(File); + return true; } /*}}}*/ diff --git a/apt-pkg/contrib/fileutl.h b/apt-pkg/contrib/fileutl.h index 06c303809..19b4ed49e 100644 --- a/apt-pkg/contrib/fileutl.h +++ b/apt-pkg/contrib/fileutl.h @@ -161,6 +161,7 @@ class FileFd inline bool Eof() {return (Flags & HitEof) == HitEof;}; inline bool IsCompressed() {return (Flags & Compressed) == Compressed;}; inline std::string &Name() {return FileName;}; + inline void SetFileName(std::string const &name) { FileName = name; }; FileFd(std::string FileName,unsigned int const Mode,unsigned long AccessMode = 0666); FileFd(std::string FileName,unsigned int const Mode, CompressMode Compress, unsigned long AccessMode = 0666); diff --git a/apt-pkg/init.cc b/apt-pkg/init.cc index af4e6faa0..207f0d902 100644 --- a/apt-pkg/init.cc +++ b/apt-pkg/init.cc @@ -212,14 +212,13 @@ bool pkgInitConfig(Configuration &Cnf) Cnf.CndSet("Acquire::Changelogs::URI::Origin::Ultimedia", "http://packages.ultimediaos.com/changelogs/pool/@CHANGEPATH@/changelog.txt"); Cnf.CndSet("Acquire::Changelogs::AlwaysOnline::Origin::Ubuntu", true); - bool Res = true; - // Read an alternate config file + _error->PushToStack(); const char *Cfg = getenv("APT_CONFIG"); if (Cfg != 0 && strlen(Cfg) != 0) { if (RealFileExists(Cfg) == true) - Res &= ReadConfigFile(Cnf,Cfg); + ReadConfigFile(Cnf, Cfg); else _error->WarningE("RealFileExists",_("Unable to read %s"),Cfg); } @@ -227,30 +226,29 @@ bool pkgInitConfig(Configuration &Cnf) // Read the configuration parts dir std::string const Parts = Cnf.FindDir("Dir::Etc::parts", "/dev/null"); if (DirectoryExists(Parts) == true) - Res &= ReadConfigDir(Cnf,Parts); + ReadConfigDir(Cnf, Parts); else if (APT::String::Endswith(Parts, "/dev/null") == false) _error->WarningE("DirectoryExists",_("Unable to read %s"),Parts.c_str()); // Read the main config file std::string const FName = Cnf.FindFile("Dir::Etc::main", "/dev/null"); if (RealFileExists(FName) == true) - Res &= ReadConfigFile(Cnf,FName); - - if (Res == false) - return false; + ReadConfigFile(Cnf, FName); if (Cnf.FindB("Debug::pkgInitConfig",false) == true) Cnf.Dump(); - + #ifdef APT_DOMAIN if (Cnf.Exists("Dir::Locale")) - { + { bindtextdomain(APT_DOMAIN,Cnf.FindDir("Dir::Locale").c_str()); bindtextdomain(textdomain(0),Cnf.FindDir("Dir::Locale").c_str()); } #endif - return true; + auto const good = _error->PendingError() == false; + _error->MergeWithStack(); + return good; } /*}}}*/ // pkgInitSystem - Initialize the _system calss /*{{{*/ diff --git a/apt-pkg/policy.cc b/apt-pkg/policy.cc index 69c1fbe10..030bab26b 100644 --- a/apt-pkg/policy.cc +++ b/apt-pkg/policy.cc @@ -324,10 +324,10 @@ bool ReadPinDir(pkgPolicy &Plcy,string Dir) return false; // Read the files + bool good = true; for (vector::const_iterator I = List.begin(); I != List.end(); ++I) - if (ReadPinFile(Plcy, *I) == false) - return false; - return true; + good = ReadPinFile(Plcy, *I) && good; + return good; } /*}}}*/ // ReadPinFile - Load the pin file into a Policy /*{{{*/ diff --git a/apt-pkg/sourcelist.cc b/apt-pkg/sourcelist.cc index 9f0c7f8ae..adf598e48 100644 --- a/apt-pkg/sourcelist.cc +++ b/apt-pkg/sourcelist.cc @@ -300,37 +300,32 @@ pkgSourceList::~pkgSourceList() /* */ bool pkgSourceList::ReadMainList() { - // CNC:2003-03-03 - Multiple sources list support. - bool Res = true; -#if 0 - Res = ReadVendors(); - if (Res == false) - return false; -#endif - Reset(); // CNC:2003-11-28 - Entries in sources.list have priority over // entries in sources.list.d. string Main = _config->FindFile("Dir::Etc::sourcelist", "/dev/null"); string Parts = _config->FindDir("Dir::Etc::sourceparts", "/dev/null"); - + + _error->PushToStack(); if (RealFileExists(Main) == true) - Res &= ReadAppend(Main); + ReadAppend(Main); else if (DirectoryExists(Parts) == false && APT::String::Endswith(Parts, "/dev/null") == false) // Only warn if there are no sources.list.d. _error->WarningE("DirectoryExists", _("Unable to read %s"), Parts.c_str()); if (DirectoryExists(Parts) == true) - Res &= ReadSourceDir(Parts); + ReadSourceDir(Parts); else if (Main.empty() == false && RealFileExists(Main) == false && APT::String::Endswith(Parts, "/dev/null") == false) // Only warn if there is no sources.list file. _error->WarningE("RealFileExists", _("Unable to read %s"), Main.c_str()); for (auto && file: _config->FindVector("APT::Sources::With")) - Res &= AddVolatileFile(file, nullptr); + AddVolatileFile(file, nullptr); - return Res; + auto good = _error->PendingError() == false; + _error->MergeWithStack(); + return good; } /*}}}*/ // SourceList::Reset - Clear the sourcelist contents /*{{{*/ @@ -498,17 +493,12 @@ bool pkgSourceList::GetIndexes(pkgAcquire *Owner, bool GetAll) const /* */ bool pkgSourceList::ReadSourceDir(string const &Dir) { - std::vector ext; - ext.push_back("list"); - ext.push_back("sources"); - std::vector const List = GetListOfFilesInDir(Dir, ext, true); - + std::vector const ext = {"list", "sources"}; // Read the files - for (vector::const_iterator I = List.begin(); I != List.end(); ++I) - if (ReadAppend(*I) == false) - return false; - return true; - + bool good = true; + for (auto const &I : GetListOfFilesInDir(Dir, ext, true)) + good = ReadAppend(I) && good; + return good; } /*}}}*/ // GetLastModified() /*{{{*/ diff --git a/test/integration/test-bug-818628-unreadable-source b/test/integration/test-bug-818628-unreadable-source index 0c781c3b9..59051fb5f 100755 --- a/test/integration/test-bug-818628-unreadable-source +++ b/test/integration/test-bug-818628-unreadable-source @@ -16,6 +16,8 @@ insertpackage 'unstable' 'foo' 'amd64' '2' setupaptarchive --no-update +touch rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list +touch rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.sources touch rootdir/etc/apt/apt.conf.d/unreadable.conf touch rootdir/etc/apt/preferences.d/unreadable.pref @@ -31,56 +33,61 @@ foo/unstable 2 amd64 [upgradable from: 1] N: There is 1 additional version. Please use the '-a' switch to see it" apt list --upgradable runthemall() { - local ERR1="$1" - local ERR2="$1$2" - testfailureequal "$ERR1" aptcache policy - testfailureequal "$ERR1" aptcache policy foo - testfailureequal "$ERR2" aptcache depends foo - testfailureequal "$ERR2" aptcache rdepends foo - testfailureequal "$ERR2" aptcache search foo - testfailureequal "$ERR1" apt policy - testfailureequal "$ERR1" apt policy foo - testfailureequal "$ERR2" apt depends foo - testfailureequal "$ERR2" apt rdepends foo - testfailureequal "$ERR2" apt search foo - testfailureequal "$ERR2" apt list --upgradable - testfailureequal "$ERR2" apt show foo - testfailureequal "$ERR2" aptcache show foo --no-all-versions - testfailureequal "$ERR2" aptmark auto foo - testfailureequal "$ERR2" aptmark manual foo - testfailureequal "$ERR2" aptmark auto foo + local ERR="$1" + local ERRNOTICEVER="$1${2- +N: There is 1 additional version. Please use the '-a' switch to see it}" + local ERRNOTICEREC="$1${2- +N: There is 1 additional record. Please use the '-a' switch to see it}" + testwarningmsg "$ERR" aptcache policy + testwarningmsg "$ERR" aptcache policy foo + testwarningmsg "$ERR" aptcache depends foo + testwarningmsg "$ERR" aptcache rdepends foo + testwarningmsg "$ERR" aptcache search foo + testwarningmsg "$ERR" apt policy + testwarningmsg "$ERR" apt policy foo + testwarningmsg "$ERR" apt depends foo + testwarningmsg "$ERR" apt rdepends foo + testwarningmsg "$ERR" apt search foo + testwarningmsg "$ERRNOTICEVER" apt list --upgradable + testwarningmsg "$ERRNOTICEREC" apt show foo + testwarningmsg "$ERRNOTICEREC" aptcache show foo --no-all-versions + testwarningmsg "$ERR" aptmark auto foo + testwarningmsg "$ERR" aptmark manual foo + testwarningmsg "$ERR" aptmark auto foo } echo 'Apt::Cmd::Disable-Script-Warning "true";' >> aptconfig.conf -msgmsg 'Unreadable sources file' +msgmsg 'Unreadable one-line-style sources file' chmod -r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list -runthemall "E: Could not open file $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - open (13: Permission denied) -E: The list of sources could not be read." +runthemall "W: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - open (13: Permission denied)" chmod +r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list +msgmsg 'Unreadable deb822-style sources file' +chmod -r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.sources +runthemall "W: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.sources - open (13: Permission denied)" +chmod +r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.sources + msgmsg 'Unreadable config file' chmod -r rootdir/etc/apt/apt.conf.d/unreadable.conf -runthemall "E: Could not open file ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/apt.conf.d/unreadable.conf - open (13: Permission denied)" +runthemall "W: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/apt.conf.d/unreadable.conf - open (13: Permission denied)" chmod +r rootdir/etc/apt/apt.conf.d/unreadable.conf msgmsg 'Unreadable preferences file' chmod -r rootdir/etc/apt/preferences.d/unreadable.pref -runthemall "E: Could not open file ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/preferences.d/unreadable.pref - open (13: Permission denied)" +runthemall "W: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/preferences.d/unreadable.pref - open (13: Permission denied)" chmod +r rootdir/etc/apt/preferences.d/unreadable.pref msgmsg 'Unreadable sources directory' chmod -r rootdir/etc/apt/sources.list.d -runthemall "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" +runthemall "W: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/ - opendir (13: Permission denied)" "" chmod +r rootdir/etc/apt/sources.list.d msgmsg 'Unreadable config directory' chmod -r rootdir/etc/apt/apt.conf.d -runthemall "E: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/apt.conf.d/ - opendir (13: Permission denied)" +runthemall "W: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/apt.conf.d/ - opendir (13: Permission denied)" chmod +r rootdir/etc/apt/apt.conf.d msgmsg 'Unreadable preferences directory' chmod -r rootdir/etc/apt/preferences.d -runthemall "E: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/preferences.d/ - opendir (13: Permission denied)" +runthemall "W: Unable to read ${TMPWORKINGDIRECTORY}/rootdir/etc/apt/preferences.d/ - opendir (13: Permission denied)" chmod +r rootdir/etc/apt/preferences.d -- cgit v1.2.3