From 3317ad864c997f4897756c0a2989c4199e9cda62 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sat, 15 Jul 2017 14:12:50 +0200 Subject: use FileFd to parse all apt configuration files Using different ways of opening files means we have different behaviour and error messages for them, so by the same for all we can have more uniformity for users and apt developers alike. --- apt-pkg/sourcelist.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'apt-pkg/sourcelist.cc') diff --git a/apt-pkg/sourcelist.cc b/apt-pkg/sourcelist.cc index 17c5c7a11..9f0c7f8ae 100644 --- a/apt-pkg/sourcelist.cc +++ b/apt-pkg/sourcelist.cc @@ -368,13 +368,12 @@ bool pkgSourceList::ReadAppend(string const &File) /* */ bool pkgSourceList::ParseFileOldStyle(std::string const &File) { - // Open the stream for reading - ifstream F(File.c_str(),ios::in /*| ios::nocreate*/); - if (F.fail() == true) - return _error->Errno("ifstream::ifstream",_("Opening %s"),File.c_str()); + FileFd Fd; + if (OpenConfigurationFileFd(File, Fd) == false) + return false; std::string Buffer; - for (unsigned int CurLine = 1; std::getline(F, Buffer); ++CurLine) + for (unsigned int CurLine = 1; Fd.ReadLine(Buffer); ++CurLine) { // remove comments size_t curpos = 0; @@ -423,7 +422,9 @@ bool pkgSourceList::ParseFileOldStyle(std::string const &File) bool pkgSourceList::ParseFileDeb822(string const &File) { // see if we can read the file - FileFd Fd(File, FileFd::ReadOnly); + FileFd Fd; + if (OpenConfigurationFileFd(File, Fd) == false) + return false; pkgTagFile Sources(&Fd, pkgTagFile::SUPPORT_COMMENTS); if (Fd.IsOpen() == false || Fd.Failed()) return _error->Error(_("Malformed stanza %u in source list %s (type)"),0,File.c_str()); -- cgit v1.2.3-70-g09d2 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(-) (limited to 'apt-pkg/sourcelist.cc') 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-70-g09d2