| Message ID | 20260305151941.835215-1-pedro.ms.ferreira@ctw.bmwgroup.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] fetch2: avoid reuse download filenames | expand |
On Thu, 2026-03-05 at 15:19 +0000, Pedro Ferreira via lists.openembedded.org wrote: > From: Pedro Ferreira <pmi183@gmail.com> > > When fetch task runs and while running checksum validation > detects that for a source file the checksum mismatches, > instead of aborting, its allowing to move aside and download again. > This might allow users to taint the source files instead of acting > as a safe mechanism to fix some issue occurred on the download stage. > > Signed-off-by: Pedro Ferreira <Pedro.MS.Ferreira@ctw.mbwgroup.com> > --- > lib/bb/fetch2/__init__.py | 11 ++++------- > lib/bb/tests/fetch.py | 4 +--- > 2 files changed, 5 insertions(+), 10 deletions(-) > > diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py > index aaefd8602..1484f5422 100644 > --- a/lib/bb/fetch2/__init__.py > +++ b/lib/bb/fetch2/__init__.py > @@ -716,13 +716,10 @@ def verify_donestamp(ud, d, origud=None): > p.dump(checksums) > return True > except ChecksumError as e: > - # Checksums failed to verify, trigger re-download and remove the > - # incorrect stamp file. > - logger.warning("Checksum mismatch for local file %s\n" > - "Cleaning and trying again." % ud.localpath) > - if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > - bb.utils.remove(ud.donestamp) > + # If there is a checksum mismatch, it is likely because the file > + # is being tainted or some corruption is occurring when downloading. > + # Download cache should be cleaned up before trying again. > + bb.fatal("Checksum mismatch for local file %s\n" % ud.localpath) > return False > > > diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py > index 74eb73472..8f371234b 100644 > --- a/lib/bb/tests/fetch.py > +++ b/lib/bb/tests/fetch.py > @@ -899,10 +899,8 @@ class FetcherNoNetworkTest(FetcherTest): > self.assertTrue(os.path.exists(os.path.join(self.dldir, "test-file.tar.gz"))) > self.assertFalse(os.path.exists(os.path.join(self.dldir, "test-file.tar.gz.done"))) > fetcher = bb.fetch.Fetch(["http://invalid.yoctoproject.org/test-file.tar.gz"], self.d) > - with self.assertRaises(bb.fetch2.NetworkAccess): > + with self.assertRaises(bb.BBHandledException): > fetcher.download() > - # the existing file should not exist or should have be moved to "bad-checksum" > - self.assertFalse(os.path.exists(os.path.join(self.dldir, "test-file.tar.gz"))) > > def test_nochecksums_missing(self): > self.assertFalse(os.path.exists(os.path.join(self.dldir, "test-file.tar.gz"))) I'm afraid this is just making the fetcher behave inconsistently. There are four places where rename_bad_checksum() can be called and you simply remove one of them as it suits your use case. I'm not keen to do that at all. Cheers, Richard
Hey Richard, Yes it is true that this changes the behaviour of one use case, but imho this has a unique context related to the other calls of this function. I do see this as a mechanism to detect that something nasty is being done. In the context that i'm using this, this will invalidate the download cache, making the source file names not being unique. Best regards, Pedro
diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py index aaefd8602..1484f5422 100644 --- a/lib/bb/fetch2/__init__.py +++ b/lib/bb/fetch2/__init__.py @@ -716,13 +716,10 @@ def verify_donestamp(ud, d, origud=None): p.dump(checksums) return True except ChecksumError as e: - # Checksums failed to verify, trigger re-download and remove the - # incorrect stamp file. - logger.warning("Checksum mismatch for local file %s\n" - "Cleaning and trying again." % ud.localpath) - if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) - bb.utils.remove(ud.donestamp) + # If there is a checksum mismatch, it is likely because the file + # is being tainted or some corruption is occurring when downloading. + # Download cache should be cleaned up before trying again. + bb.fatal("Checksum mismatch for local file %s\n" % ud.localpath) return False diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py index 74eb73472..8f371234b 100644 --- a/lib/bb/tests/fetch.py +++ b/lib/bb/tests/fetch.py @@ -899,10 +899,8 @@ class FetcherNoNetworkTest(FetcherTest): self.assertTrue(os.path.exists(os.path.join(self.dldir, "test-file.tar.gz"))) self.assertFalse(os.path.exists(os.path.join(self.dldir, "test-file.tar.gz.done"))) fetcher = bb.fetch.Fetch(["http://invalid.yoctoproject.org/test-file.tar.gz"], self.d) - with self.assertRaises(bb.fetch2.NetworkAccess): + with self.assertRaises(bb.BBHandledException): fetcher.download() - # the existing file should not exist or should have be moved to "bad-checksum" - self.assertFalse(os.path.exists(os.path.join(self.dldir, "test-file.tar.gz"))) def test_nochecksums_missing(self): self.assertFalse(os.path.exists(os.path.join(self.dldir, "test-file.tar.gz")))