ngclient: add download_target_bytes() - #2992
Open
ChrisJr404 wants to merge 1 commit into
Open
Conversation
Add an Updater API that downloads and verifies a target and returns its content as bytes instead of writing it into the local cache. sigstore-python and similar callers want the verified bytes in memory and don't need the file on disk. The URL-building logic is pulled out into a shared _target_file_url() helper so both download_target() and download_target_bytes() run the exact same length/hash verification against the same downloaded stream; only the output differs (write to disk vs return bytes). Fixes theupdateframework#1556 Signed-off-by: Chris (ChrisJr404) <11917633+ChrisJr404@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
Updater.download_target_bytes(), which downloads and verifies a target and hands back the content asbytesinstead of writing it to the cache dir, so callers like sigstore-python that just want the verified bytes in memory don't have to round-trip through a file.The verification path is shared, not duplicated: I pulled the URL-building out of
download_target()into a small_target_file_url()helper, and both methods then do the same_fetcher.download_file()+targetinfo.verify_length_and_hashes()against the same downloaded stream. The only difference is what happens after verification succeeds -download_target()copies to disk,download_target_bytes()reads the stream and returns it. So the length/hash checks are byte-for-byte identical between the two.On the cache-timing concern from #1556 (re #1168): the new method deliberately does not touch the local target cache at all. It doesn't call
find_cached_target(), doesn't read an existing cached file, and doesn't persist anything. Since it never consults the cache, there's no cache-hit/miss timing to leak - it always downloads and verifies. Callers who want caching keep usingdownload_target()+find_cached_target().One thing worth flagging: because a target's hash can't be checked until it's fully downloaded, the bytes variant buffers the whole target in memory. That's fine for the small artifacts this is aimed at, but it's not suitable for very large targets - I noted that in the docstring. The streaming/iterator idea from the issue thread would be a separate, larger API and I left it out here.
Tests in
tests/test_updater_fetch_target.py, mirroring the existingdownload_targettests against the repository simulator:test_fetch_target_bytes(dataset-driven, same three target cases): returned bytes match expected content, and nothing gets written to the cache dir (find_cached_targetstaysNone).test_invalid_target_download_bytes: hash mismatch and length mismatch both still raiseRepositoryError, and nothing is persisted.tox-equivalent locally:pytest tests/test_updater_*.pygreen (updater suites 27 passed / 13 subtests),black/isortclean at line-length 80,mypyclean onupdater.py,pylint9.91 (only pre-existing__init__arg-count warnings, untouched by this change).Fixes #1556