Look at non by-hash paths in copy and file methods

Ideally copy and file mirrors would support by-hash as well, but its
harder to setup and maintain especially if you want to cache an online
mirror who has by-hash enabled.

We can avoid unneeded roundtrips (in the best case) and entire cache
misses (in the usual worst case) by "just" telling methods that the URI
we passed it has the requested file perhaps also in other paths.

This is done in pseudo-relative paths as we would otherwise need to
teach redirection code to rewrite those URIs as well. A method like http
can easily ignore this value and await explicit instructions to look at
that file, but inspecting the path in local sources via file or copy is
(comparatively) free, so we just do it immediately. If that ends up
being the wrong version of the file as by-hash would have protected us
from we are in this feature branch now falling back to other mirrors
which are like the ones online and in support of by-hash.

8 files changed, 143 insertions, 102 deletions

diff --git a/apt-pkg/acquire-item.cc b/apt-pkg/acquire-item.cc
index 2c3f45076..3950ae030 100644
--- a/apt-pkg/acquire-item.cc
+++ b/apt-pkg/acquire-item.cc
@@ -473,15 +473,16 @@ bool pkgAcqTransactionItem::QueueURI(pkgAcquire::ItemDesc &Item)
       // now add the actual by-hash uris
       auto const Expected = GetExpectedHashes();
       auto const TargetHash = Expected.find(nullptr);
-      auto const PushByHashURI = [&](std::string U) {
+      auto const PushByHashURI = [&](std::string const &U) {
 	 if (unlikely(TargetHash == nullptr))
 	    return false;
-	 auto const trailing_slash = U.find_last_of("/");
+	 ::URI uri{U};
+	 auto const trailing_slash = uri.Path.find_last_of("/");
 	 if (unlikely(trailing_slash == std::string::npos))
 	    return false;
-	 auto byhashSuffix = "/by-hash/" + TargetHash->HashType() + "/" + TargetHash->HashValue();
-	 U.replace(trailing_slash, U.length() - trailing_slash, std::move(byhashSuffix));
-	 PushAlternativeURI(std::move(U), {}, false);
+	 auto altPath = uri.Path.substr(0, trailing_slash) + "/by-hash/" + TargetHash->HashType() + "/" + TargetHash->HashValue();
+	 std::swap(uri.Path, altPath);
+	 PushAlternativeURI(uri, {{"Alternate-Paths", "../../" + flNotDir(altPath)}}, false);
 	 return true;
       };
       PushByHashURI(Item.URI);
diff --git a/methods/aptmethod.h b/methods/aptmethod.h
index 1c24f3a98..1ea173075 100644
--- a/methods/aptmethod.h
+++ b/methods/aptmethod.h
@@ -514,6 +514,17 @@ protected:
       return part;
    }
 
+   static std::string CombineWithAlternatePath(std::string Path, std::string Change)
+   {
+      while (APT::String::Startswith(Change, "../"))
+      {
+	 Path.erase(Path.find_last_not_of('/'));
+	 Path = flNotFile(Path);
+	 Change.erase(0, 3);
+      }
+      return flCombine(Path, Change);
+   }
+
    aptMethod(std::string &&Binary, char const *const Ver, unsigned long const Flags) APT_NONNULL(3)
        : pkgAcqMethod(Ver, Flags), aptConfigWrapperForMethods(Binary), Binary(std::move(Binary)), SeccompFlags(0)
    {
diff --git a/methods/copy.cc b/methods/copy.cc
index 82eed150c..b32131a21 100644
--- a/methods/copy.cc
+++ b/methods/copy.cc
@@ -26,7 +26,7 @@
 
 class CopyMethod : public aptMethod
 {
-   virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE;
+   bool URIAcquire(std::string const &Message, FetchItem *Itm) APT_OVERRIDE;
 
    public:
    CopyMethod() : aptMethod("copy", "1.0", SingleInstance | SendConfig | SendURIEncoded)
@@ -36,55 +36,71 @@ class CopyMethod : public aptMethod
 };
 
 // CopyMethod::Fetch - Fetch a file					/*{{{*/
-// ---------------------------------------------------------------------
-/* */
-bool CopyMethod::Fetch(FetchItem *Itm)
+bool CopyMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
 {
-   // this ensures that relative paths work in copy
-   std::string const File = DecodeSendURI(Itm->Uri.substr(Itm->Uri.find(':')+1));
-
-   // Stat the file and send a start message
-   struct stat Buf;
-   if (stat(File.c_str(),&Buf) != 0)
-      return _error->Errno("stat",_("Failed to stat"));
+   struct FileCopyType {
+      std::string name;
+      struct stat stat{};
+      explicit FileCopyType(std::string &&file) : name{std::move(file)} {}
+   };
+   std::vector<FileCopyType> files;
+   // this ensures that relative paths work
+   files.emplace_back(DecodeSendURI(Itm->Uri.substr(Itm->Uri.find(':')+1)));
+   for (auto const &AltPath : VectorizeString(LookupTag(Message, "Alternate-Paths"), '\n'))
+      files.emplace_back(CombineWithAlternatePath(flNotFile(files[0].name), DecodeSendURI(AltPath)));
+   files.erase(std::remove_if(files.begin(), files.end(), [](auto &file)
+			      { return stat(file.name.c_str(), &file.stat) != 0; }),
+	       files.end());
+   if (files.empty())
+      return _error->Errno("copy-stat", _("Failed to stat"));
 
    // Forumulate a result and send a start message
    FetchResult Res;
-   Res.Size = Buf.st_size;
    Res.Filename = Itm->DestFile;
-   Res.LastModified = Buf.st_mtime;
    Res.IMSHit = false;
-   URIStart(Res);
 
-   // just calc the hashes if the source and destination are identical
-   if (File == Itm->DestFile || Itm->DestFile == "/dev/null")
+   for (auto const &File : files)
    {
+      Res.Size = File.stat.st_size;
+      Res.LastModified = File.stat.st_mtime;
+
+      // just calc the hashes if the source and destination are identical
+      if (Itm->DestFile == "/dev/null" || File.name == Itm->DestFile)
+      {
+	 URIStart(Res);
+	 CalculateHashes(Itm, Res);
+	 URIDone(Res);
+	 return true;
+      }
+
+      FileFd From(File.name, FileFd::ReadOnly);
+      FileFd To(Itm->DestFile, FileFd::WriteAtomic);
+      To.EraseOnFailure();
+      if (not From.IsOpen() || not To.IsOpen())
+	 continue;
+
+      // Copy the file
+      URIStart(Res);
+      if (not CopyFile(From, To))
+      {
+	 To.OpFail();
+	 continue;
+      }
+      From.Close();
+      To.Close();
+
       CalculateHashes(Itm, Res);
-      URIDone(Res);
-      return true;
-   }
+      if (not Itm->ExpectedHashes.empty() && Itm->ExpectedHashes != Res.Hashes)
+	 continue;
 
-   // See if the file exists
-   FileFd From(File,FileFd::ReadOnly);
-   FileFd To(Itm->DestFile,FileFd::WriteAtomic);
-   To.EraseOnFailure();
+      if (not TransferModificationTimes(File.name.c_str(), Res.Filename.c_str(), Res.LastModified))
+	 continue;
 
-   // Copy the file
-   if (CopyFile(From,To) == false)
-   {
-      To.OpFail();
-      return false;
+      URIDone(Res);
+      return true;
    }
 
-   From.Close();
-   To.Close();
-
-   if (TransferModificationTimes(File.c_str(), Res.Filename.c_str(), Res.LastModified) == false)
-      return false;
-
-   CalculateHashes(Itm, Res);
-   URIDone(Res);
-   return true;
+   return false;
 }
 									/*}}}*/
 
diff --git a/methods/file.cc b/methods/file.cc
index ff93f83d0..279711195 100644
--- a/methods/file.cc
+++ b/methods/file.cc
@@ -29,7 +29,7 @@
 
 class FileMethod : public aptMethod
 {
-   virtual bool Fetch(FetchItem *Itm) APT_OVERRIDE;
+   bool URIAcquire(std::string const &Message, FetchItem *Itm) APT_OVERRIDE;
 
    public:
    FileMethod() : aptMethod("file", "1.0", SingleInstance | SendConfig | LocalOnly | SendURIEncoded)
@@ -41,88 +41,101 @@ class FileMethod : public aptMethod
 // FileMethod::Fetch - Fetch a file					/*{{{*/
 // ---------------------------------------------------------------------
 /* */
-bool FileMethod::Fetch(FetchItem *Itm)
+bool FileMethod::URIAcquire(std::string const &Message, FetchItem *Itm)
 {
    URI Get(Itm->Uri);
-   std::string const File = DecodeSendURI(Get.Path);
-   FetchResult Res;
    if (Get.Host.empty() == false)
       return _error->Error(_("Invalid URI, local URIS must not start with //"));
 
-   struct stat Buf;
+   std::vector<std::string> Files;
+   Files.emplace_back(DecodeSendURI(Get.Path));
+   for (auto const &AltPath : VectorizeString(LookupTag(Message, "Alternate-Paths"), '\n'))
+      Files.emplace_back(CombineWithAlternatePath(flNotFile(Files[0]), DecodeSendURI(AltPath)));
+
+   FetchResult Res;
    // deal with destination files which might linger around
-   if (lstat(Itm->DestFile.c_str(), &Buf) == 0)
+   struct stat Buf;
+   if (lstat(Itm->DestFile.c_str(), &Buf) == 0 && S_ISLNK(Buf.st_mode) && Buf.st_size > 0)
+   {
+      char name[Buf.st_size + 1];
+      if (ssize_t const sp = readlink(Itm->DestFile.c_str(), name, Buf.st_size); sp == -1)
+      {
+	 Itm->LastModified = 0;
+	 RemoveFile("file", Itm->DestFile);
+      }
+   }
+
+   int olderrno = 0;
+   // See if the file exists
+   for (auto const &File : Files)
    {
-      if ((Buf.st_mode & S_IFREG) != 0)
+      if (stat(File.c_str(), &Buf) == 0)
       {
+	 Res.Size = Buf.st_size;
+	 Res.Filename = File;
+	 Res.LastModified = Buf.st_mtime;
+	 Res.IMSHit = false;
 	 if (Itm->LastModified == Buf.st_mtime && Itm->LastModified != 0)
 	 {
-	    if (Itm->ExpectedHashes.VerifyFile(File))
-	    {
-	       Res.Filename = Itm->DestFile;
+	    auto const filesize = Itm->ExpectedHashes.FileSize();
+	    if (filesize != 0 && filesize == Res.Size)
 	       Res.IMSHit = true;
-	    }
 	 }
+	 break;
       }
+      if (olderrno == 0)
+	 olderrno = errno;
    }
-   if (Res.IMSHit != true)
-      RemoveFile("file", Itm->DestFile);
 
-   int olderrno = 0;
-   // See if the file exists
-   if (stat(File.c_str(),&Buf) == 0)
+   if (not Res.IMSHit)
    {
-      Res.Size = Buf.st_size;
-      Res.Filename = File;
-      Res.LastModified = Buf.st_mtime;
-      Res.IMSHit = false;
-      if (Itm->LastModified == Buf.st_mtime && Itm->LastModified != 0)
+      RemoveFile("file", Itm->DestFile);
+      if (not Res.Filename.empty())
       {
-	 unsigned long long const filesize = Itm->ExpectedHashes.FileSize();
-	 if (filesize != 0 && filesize == Res.Size)
-	    Res.IMSHit = true;
+	 URIStart(Res);
+	 CalculateHashes(Itm, Res);
       }
-
-      CalculateHashes(Itm, Res);
    }
-   else
-      olderrno = errno;
-   if (Res.IMSHit == false)
-      URIStart(Res);
 
    // See if the uncompressed file exists and reuse it
    FetchResult AltRes;
-   AltRes.Filename.clear();
-   std::vector<std::string> extensions = APT::Configuration::getCompressorExtensions();
-   for (std::vector<std::string>::const_iterator ext = extensions.begin(); ext != extensions.end(); ++ext)
+   for (auto const &File : Files)
    {
-      if (APT::String::Endswith(File, *ext) == true)
+      for (const auto &ext : APT::Configuration::getCompressorExtensions())
       {
-	 std::string const unfile = File.substr(0, File.length() - ext->length());
-	 if (stat(unfile.c_str(),&Buf) == 0)
+	 if (APT::String::Endswith(File, ext))
 	 {
-	    AltRes.Size = Buf.st_size;
-	    AltRes.Filename = unfile;
-	    AltRes.LastModified = Buf.st_mtime;
-	    AltRes.IMSHit = false;
-	    if (Itm->LastModified == Buf.st_mtime && Itm->LastModified != 0)
-	       AltRes.IMSHit = true;
-	    if (Res.Filename.empty())
-	       CalculateHashes(Itm, AltRes);
-	    break;
+	    std::string const unfile = File.substr(0, File.length() - ext.length());
+	    if (stat(unfile.c_str(), &Buf) == 0)
+	    {
+	       AltRes.Size = Buf.st_size;
+	       AltRes.Filename = unfile;
+	       AltRes.LastModified = Buf.st_mtime;
+	       AltRes.IMSHit = false;
+	       if (Itm->LastModified == Buf.st_mtime && Itm->LastModified != 0)
+		  AltRes.IMSHit = true;
+	       if (Res.Filename.empty())
+	       {
+		  URIStart(Res);
+		  CalculateHashes(Itm, AltRes);
+	       }
+	       break;
+	    }
+	    // no break here as we could have situations similar to '.gz' vs '.tar.gz' here
 	 }
-	 // no break here as we could have situations similar to '.gz' vs '.tar.gz' here
       }
+      if (not AltRes.Filename.empty())
+	 break;
    }
 
-   if (AltRes.Filename.empty() == false)
+   if (not AltRes.Filename.empty())
       URIDone(Res,&AltRes);
-   else if (Res.Filename.empty() == false)
+   else if (not Res.Filename.empty())
       URIDone(Res);
    else
    {
       errno = olderrno;
-      return _error->Errno(File.c_str(), _("File not found"));
+      return _error->Errno(Files[0].c_str(), _("File not found"));
    }
 
    return true;
diff --git a/test/integration/test-apt-by-hash-update b/test/integration/test-apt-by-hash-update
index 65c3766d0..a33eb9117 100755
--- a/test/integration/test-apt-by-hash-update
+++ b/test/integration/test-apt-by-hash-update
@@ -13,6 +13,7 @@ insertpackage 'unstable' 'foo' 'all' '1.0'
 insertpackage 'unstable' 'bar' 'i386' '1.0'
 
 setupaptarchive --no-update
+changetowebserver
 
 # make Packages *only* accessible by-hash for this test
 makebyhashonly() {
@@ -100,8 +101,7 @@ msgmsg 'Test InRelease by-hash with' 'no fallback'
 rm -rf aptarchive/dists
 cp -a aptarchive/dists.bak aptarchive/dists
 
-testfailureequal "Get:1 file:${TMPWORKINGDIRECTORY}/aptarchive unstable InRelease
-Err:1 file:${TMPWORKINGDIRECTORY}/aptarchive unstable InRelease
-  File not found - ${TMPWORKINGDIRECTORY}/aptarchive/dists/unstable/by-hash/SHA256/${inrelease_hash} (2: No such file or directory)
+testfailureequal "Err:1 http://localhost:${APTHTTPPORT} unstable InRelease
+  404  Not Found
 Reading package lists...
-E: Failed to fetch file:${TMPWORKINGDIRECTORY}/aptarchive/dists/unstable/InRelease  File not found - ${TMPWORKINGDIRECTORY}/aptarchive/dists/unstable/by-hash/SHA256/${inrelease_hash} (2: No such file or directory)" aptget update
+E: Failed to fetch http://localhost:${APTHTTPPORT}/dists/unstable/InRelease  404  Not Found" aptget update
diff --git a/test/integration/test-apt-get-update-unauth-warning b/test/integration/test-apt-get-update-unauth-warning
index 46df14c29..9f252ad9d 100755
--- a/test/integration/test-apt-get-update-unauth-warning
+++ b/test/integration/test-apt-get-update-unauth-warning
@@ -28,9 +28,7 @@ testwarning aptget update --allow-insecure-repositories
 rm -rf rootdir/var/lib/apt/lists
 find "$APTARCHIVE/dists/unstable" -name '*Release*' -delete
 # update without authenticated files leads to warning
-testfailureequal "Get:1 file:$APTARCHIVE unstable InRelease
-Ign:1 file:$APTARCHIVE unstable InRelease
-Get:2 file:$APTARCHIVE unstable Release
+testfailureequal "Ign:1 file:$APTARCHIVE unstable InRelease
 Err:2 file:$APTARCHIVE unstable Release
   File not found - ${APTARCHIVE}/dists/unstable/Release (2: No such file or directory)
 Reading package lists...
@@ -45,9 +43,7 @@ lock
 partial' ls rootdir/var/lib/apt/lists
 
 # allow override
-testwarningequal "Get:1 file:$APTARCHIVE unstable InRelease
-Ign:1 file:$APTARCHIVE unstable InRelease
-Get:2 file:$APTARCHIVE unstable Release
+testwarningequal "Ign:1 file:$APTARCHIVE unstable InRelease
 Ign:2 file:$APTARCHIVE unstable Release
 Get:3 file:$APTARCHIVE unstable/main Sources
 Get:4 file:$APTARCHIVE unstable/main i386 Packages
diff --git a/test/integration/test-apt-update-incomplete-file-mirror b/test/integration/test-apt-update-incomplete-file-mirror
index 959ddc1a6..dec9383c9 100755
--- a/test/integration/test-apt-update-incomplete-file-mirror
+++ b/test/integration/test-apt-update-incomplete-file-mirror
@@ -37,6 +37,7 @@ testfailure apt update -o Debug::pkgAcquire::Worker=1
 testsuccessequal '4' grep -c -- '- Filesize:' rootdir/tmp/testfailure.output
 testsuccessequal '2' grep -c '%0aAlt-Checksum-FileSize-Hash:%20' rootdir/tmp/testfailure.output
 
+echo 'Acquire::By-Hash "force";' > rootdir/etc/apt/apt.conf.d/99force-by-hash.conf
 
 msgmsg 'Fallback over hashsum errors'
 rm -f rootdir/etc/apt/sources.list rootdir/etc/apt/sources.list.d/*
@@ -47,6 +48,8 @@ copy:${TMPWORKINGDIRECTORY}/aptarchive	priority:1
 file:${TMPWORKINGDIRECTORY}/aptarchive2	priority:2
 EOF
 testsuccess apt update
+cp -a rootdir/tmp/testsuccess.output aptupdate.log
+testsuccessequal '2' grep -c -- '^Ign:' aptupdate.log
 
 rm -rf rootdir/var/lib/apt/lists
 cat > mirror.list <<EOF
@@ -61,3 +64,5 @@ file:${TMPWORKINGDIRECTORY}/aptarchive	priority:1
 file:${TMPWORKINGDIRECTORY}/aptarchive2	priority:2
 EOF
 testsuccess apt update
+cp -a rootdir/tmp/testsuccess.output aptupdate.log
+testsuccessequal '1' grep -c -- '^Ign:' aptupdate.log
diff --git a/test/integration/test-method-mirror b/test/integration/test-method-mirror
index 1784e1ecb..79c675455 100755
--- a/test/integration/test-method-mirror
+++ b/test/integration/test-method-mirror
@@ -255,7 +255,6 @@ echo "file:/nonexistent/apt/archive	priority:1
 http://localhost:${APTHTTPPORT}/redirectme
 " > aptarchive/mirror.txt
 testsuccessequal "Get:1 foo+file:${APTARCHIVE}/mirror.txt Mirrorlist [$(stat -c%s 'aptarchive/mirror.txt') B]
-Get:2 foo+file:/nonexistent/apt/archive unstable InRelease
 Ign:2 foo+file:/nonexistent/apt/archive unstable InRelease
   File not found - /nonexistent/apt/archive/dists/unstable/InRelease (2: No such file or directory)
 Hit:2 foo+http://localhost:${APTHTTPPORT}/redirectme unstable InRelease