| Message ID | 20260928205940.14169-1-setairidis@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v5] fetch/local: verify checksums for file:// urls | expand |
Hi, My bad I assumed that Jhonata stopped working on this based on the ticket comments. I send him a personal email about this. Regards, Savvas Στις Δευ 28 Σεπ 2026 στις 10:59 μ.μ., ο/η Savvas Etairidis < setairidis@gmail.com> έγραψε: > [YOCTO #9993] > > Local.urldata_init() always cleared needdonestamp, so a checksum given > in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at > a local file://) was never verified. Keep the donestamp when the url > carries a checksum so verify_checksum runs, and make > rename_bad_checksum() a FetchMethod method that Local overrides as a > no-op, so a mismatch doesn't rename the user's source file. > > Based on a patch by Jhonata Poma-Hansen. > > Signed-off-by: Savvas Etairidis <setairidis@gmail.com> > > --- > changes in v5: > - Rebased on top of master, added changelog entry > > changes in v4: > - Rebased on top of master > > changes in v3: > - Cherry-picked the patch from Jhonata Poma-Hansen > > --- > --- > lib/bb/fetch/__init__.py | 36 ++++++++++++++++++------------------ > lib/bb/fetch/local.py | 18 ++++++++++++++++-- > lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ > 3 files changed, 64 insertions(+), 20 deletions(-) > > diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py > index 55ab710f3..a9f0add78 100644 > --- a/lib/bb/fetch/__init__.py > +++ b/lib/bb/fetch/__init__.py > @@ -742,7 +742,7 @@ def verify_donestamp(ud, d, origud=None): > 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) > + ud.method.rename_bad_checksum(ud, e.checksum) > bb.utils.remove(ud.donestamp) > return False > > @@ -775,7 +775,7 @@ def update_stamp(ud, d): > 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) > + ud.method.rename_bad_checksum(ud, e.checksum) > bb.utils.remove(ud.donestamp) > raise > > @@ -1079,20 +1079,6 @@ def build_mirroruris(origud, mirrors, ld): > > return uris, uds > > -def rename_bad_checksum(ud, suffix): > - """ > - Renames files to have suffix from parameter > - """ > - > - if ud.localpath is None: > - return > - > - new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) > - bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) > - if not bb.utils.movefile(ud.localpath, new_localpath): > - bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch > to see why" % (ud.localpath, new_localpath)) > - > - > def try_mirror_url(fetch, origud, ud, ld, check = False): > # Return of None or a value means we're finished > # False means try another url > @@ -1163,7 +1149,7 @@ def try_mirror_url(fetch, origud, ud, ld, check = > False): > logger.warning("Mirror checksum failure for url %s (original > url: %s)\nCleaning and trying again." % (ud.url, origud.url)) > logger.warning(str(e)) > if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > + ud.method.rename_bad_checksum(ud, e.checksum) > elif isinstance(e, NoChecksumError): > raise > else: > @@ -1479,6 +1465,20 @@ class FetchMethod(object): > """ > return True > > + def rename_bad_checksum(self, ud, suffix): > + """ > + Rename ud.localpath with the given suffix on checksum mismatch. > + Local overrides this to a no-op since its localpath points at the > + user's source tree. > + """ > + if ud.localpath is None: > + return > + > + new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) > + bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) > + if not bb.utils.movefile(ud.localpath, new_localpath): > + bb.warn("Renaming %s to %s failed, grep movefile in > log.do_fetch to see why" % (ud.localpath, new_localpath)) > + > def verify_donestamp(self, ud, d): > """ > Verify the donestamp file > @@ -1943,7 +1943,7 @@ class Fetch(object): > logger.warning("Checksum failure encountered > with download of %s - will attempt other sources if available" % u) > logger.debug(str(e)) > if os.path.exists(ud.localpath): > - rename_bad_checksum(ud, e.checksum) > + ud.method.rename_bad_checksum(ud, > e.checksum) > elif isinstance(e, NoChecksumError): > raise > else: > diff --git a/lib/bb/fetch/local.py b/lib/bb/fetch/local.py > index dfc5b564c..a92c0d267 100644 > --- a/lib/bb/fetch/local.py > +++ b/lib/bb/fetch/local.py > @@ -17,7 +17,7 @@ import os > import urllib.request, urllib.parse, urllib.error > import bb > import bb.utils > -from bb.fetch import FetchMethod, FetchError, ParameterError > +from bb.fetch import CHECKSUM_LIST, FetchMethod, FetchError, > ParameterError > from bb.fetch import logger > > class Local(FetchMethod): > @@ -31,11 +31,25 @@ class Local(FetchMethod): > # We don't set localfile as for this fetcher the file is already > local! > ud.basename = os.path.basename(ud.path) > ud.basepath = ud.path > - ud.needdonestamp = False > + # Honor explicit checksum params (UNINATIVE_CHECKSUM, recipe > + # ;sha256sum=) by leaving needdonestamp at FetchData's default so > + # verify_checksum runs. > + name = ud.parm.get("name") > + ud.needdonestamp = any( > + (name and "%s.%ssum" % (name, a) in ud.parm) > + or "%ssum" % a in ud.parm > + for a in CHECKSUM_LIST > + ) > if "*" in ud.path: > raise bb.fetch.ParameterError("file:// urls using globbing > are no longer supported. Please place the files in a directory and > reference that instead.", ud.url) > return > > + def rename_bad_checksum(self, ud, suffix): > + # file:// urls point at the user's source tree (recipe-shipped > files > + # under FILESPATH or an absolute path the user passed in); skip > the > + # rename so a ChecksumError surfaces without mutating their files. > + return > + > def localpath(self, urldata, d): > """ > Return the local filename of a given url assuming a successful > fetch. > diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py > index 71b0a63fe..60d356fc6 100644 > --- a/lib/bb/tests/fetch.py > +++ b/lib/bb/tests/fetch.py > @@ -876,6 +876,36 @@ class FetcherLocalTest(FetcherTest): > with self.subTest(striplevel=repr(value)): > self.assertInvalidStriplevel(value) > > + def test_local_checksum_match(self): > + # Correct sha256 must run verify_checksum to completion; donestamp > + # lands in DL_DIR. > + import hashlib > + content = b"file:// checksum match test\n" > + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: > + f.write(content) > + good = hashlib.sha256(content).hexdigest() > + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], > self.d) > + ud = fetcher.ud[fetcher.urls[0]] > + self.assertTrue(ud.needdonestamp) > + fetcher.download() > + self.assertTrue(os.path.exists(ud.donestamp)) > + > + def test_local_checksum_mismatch(self): > + # Bad sha256 raises ChecksumError without renaming the user's > + # source file to the <localpath>_bad-checksum_<sha> sibling. > + content = b"file:// checksum mismatch test\n" > + srcpath = os.path.join(self.localsrcdir, 'sumfile') > + with open(srcpath, 'wb') as f: > + f.write(content) > + bad = "0" * 64 > + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], > self.d) > + with self.assertRaises(bb.fetch.FetchError): > + fetcher.download() > + self.assertTrue(os.path.exists(srcpath)) > + with open(srcpath, 'rb') as f: > + self.assertEqual(f.read(), content) > + self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + bad)) > + > def dummyGitTest(self, suffix): > # Create dummy local Git repo > src_dir = tempfile.mkdtemp(dir=self.tempdir, > -- > 2.34.1 > >
diff --git a/lib/bb/fetch/__init__.py b/lib/bb/fetch/__init__.py index 55ab710f3..a9f0add78 100644 --- a/lib/bb/fetch/__init__.py +++ b/lib/bb/fetch/__init__.py @@ -742,7 +742,7 @@ def verify_donestamp(ud, d, origud=None): 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) + ud.method.rename_bad_checksum(ud, e.checksum) bb.utils.remove(ud.donestamp) return False @@ -775,7 +775,7 @@ def update_stamp(ud, d): 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) + ud.method.rename_bad_checksum(ud, e.checksum) bb.utils.remove(ud.donestamp) raise @@ -1079,20 +1079,6 @@ def build_mirroruris(origud, mirrors, ld): return uris, uds -def rename_bad_checksum(ud, suffix): - """ - Renames files to have suffix from parameter - """ - - if ud.localpath is None: - return - - new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) - bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) - if not bb.utils.movefile(ud.localpath, new_localpath): - bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) - - def try_mirror_url(fetch, origud, ud, ld, check = False): # Return of None or a value means we're finished # False means try another url @@ -1163,7 +1149,7 @@ def try_mirror_url(fetch, origud, ud, ld, check = False): logger.warning("Mirror checksum failure for url %s (original url: %s)\nCleaning and trying again." % (ud.url, origud.url)) logger.warning(str(e)) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) elif isinstance(e, NoChecksumError): raise else: @@ -1479,6 +1465,20 @@ class FetchMethod(object): """ return True + def rename_bad_checksum(self, ud, suffix): + """ + Rename ud.localpath with the given suffix on checksum mismatch. + Local overrides this to a no-op since its localpath points at the + user's source tree. + """ + if ud.localpath is None: + return + + new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix) + bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath)) + if not bb.utils.movefile(ud.localpath, new_localpath): + bb.warn("Renaming %s to %s failed, grep movefile in log.do_fetch to see why" % (ud.localpath, new_localpath)) + def verify_donestamp(self, ud, d): """ Verify the donestamp file @@ -1943,7 +1943,7 @@ class Fetch(object): logger.warning("Checksum failure encountered with download of %s - will attempt other sources if available" % u) logger.debug(str(e)) if os.path.exists(ud.localpath): - rename_bad_checksum(ud, e.checksum) + ud.method.rename_bad_checksum(ud, e.checksum) elif isinstance(e, NoChecksumError): raise else: diff --git a/lib/bb/fetch/local.py b/lib/bb/fetch/local.py index dfc5b564c..a92c0d267 100644 --- a/lib/bb/fetch/local.py +++ b/lib/bb/fetch/local.py @@ -17,7 +17,7 @@ import os import urllib.request, urllib.parse, urllib.error import bb import bb.utils -from bb.fetch import FetchMethod, FetchError, ParameterError +from bb.fetch import CHECKSUM_LIST, FetchMethod, FetchError, ParameterError from bb.fetch import logger class Local(FetchMethod): @@ -31,11 +31,25 @@ class Local(FetchMethod): # We don't set localfile as for this fetcher the file is already local! ud.basename = os.path.basename(ud.path) ud.basepath = ud.path - ud.needdonestamp = False + # Honor explicit checksum params (UNINATIVE_CHECKSUM, recipe + # ;sha256sum=) by leaving needdonestamp at FetchData's default so + # verify_checksum runs. + name = ud.parm.get("name") + ud.needdonestamp = any( + (name and "%s.%ssum" % (name, a) in ud.parm) + or "%ssum" % a in ud.parm + for a in CHECKSUM_LIST + ) if "*" in ud.path: raise bb.fetch.ParameterError("file:// urls using globbing are no longer supported. Please place the files in a directory and reference that instead.", ud.url) return + def rename_bad_checksum(self, ud, suffix): + # file:// urls point at the user's source tree (recipe-shipped files + # under FILESPATH or an absolute path the user passed in); skip the + # rename so a ChecksumError surfaces without mutating their files. + return + def localpath(self, urldata, d): """ Return the local filename of a given url assuming a successful fetch. diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py index 71b0a63fe..60d356fc6 100644 --- a/lib/bb/tests/fetch.py +++ b/lib/bb/tests/fetch.py @@ -876,6 +876,36 @@ class FetcherLocalTest(FetcherTest): with self.subTest(striplevel=repr(value)): self.assertInvalidStriplevel(value) + def test_local_checksum_match(self): + # Correct sha256 must run verify_checksum to completion; donestamp + # lands in DL_DIR. + import hashlib + content = b"file:// checksum match test\n" + with open(os.path.join(self.localsrcdir, 'sumfile'), 'wb') as f: + f.write(content) + good = hashlib.sha256(content).hexdigest() + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + good], self.d) + ud = fetcher.ud[fetcher.urls[0]] + self.assertTrue(ud.needdonestamp) + fetcher.download() + self.assertTrue(os.path.exists(ud.donestamp)) + + def test_local_checksum_mismatch(self): + # Bad sha256 raises ChecksumError without renaming the user's + # source file to the <localpath>_bad-checksum_<sha> sibling. + content = b"file:// checksum mismatch test\n" + srcpath = os.path.join(self.localsrcdir, 'sumfile') + with open(srcpath, 'wb') as f: + f.write(content) + bad = "0" * 64 + fetcher = bb.fetch.Fetch(['file://sumfile;sha256sum=' + bad], self.d) + with self.assertRaises(bb.fetch.FetchError): + fetcher.download() + self.assertTrue(os.path.exists(srcpath)) + with open(srcpath, 'rb') as f: + self.assertEqual(f.read(), content) + self.assertFalse(os.path.exists(srcpath + '_bad-checksum_' + bad)) + def dummyGitTest(self, suffix): # Create dummy local Git repo src_dir = tempfile.mkdtemp(dir=self.tempdir,
[YOCTO #9993] Local.urldata_init() always cleared needdonestamp, so a checksum given in a file:// url (e.g. UNINATIVE_CHECKSUM when UNINATIVE_URL points at a local file://) was never verified. Keep the donestamp when the url carries a checksum so verify_checksum runs, and make rename_bad_checksum() a FetchMethod method that Local overrides as a no-op, so a mismatch doesn't rename the user's source file. Based on a patch by Jhonata Poma-Hansen. Signed-off-by: Savvas Etairidis <setairidis@gmail.com> --- changes in v5: - Rebased on top of master, added changelog entry changes in v4: - Rebased on top of master changes in v3: - Cherry-picked the patch from Jhonata Poma-Hansen --- --- lib/bb/fetch/__init__.py | 36 ++++++++++++++++++------------------ lib/bb/fetch/local.py | 18 ++++++++++++++++-- lib/bb/tests/fetch.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 20 deletions(-)