feat: try .zip if .gz missing or mismatches expected checksum for NVD (#5673)#5679
Open
feat: try .zip if .gz missing or mismatches expected checksum for NVD (#5673)#5679
Conversation
This allows to not have to care about cleaning up after yourself in an NVD test. We anyway didn't make use of shared cachedir in this test class as only test_01_cache_update was something that was making use of the cache. Being the only test populating and making use of the cache, this doesn't change anything in behavior for now (aside from creating an additional NVD_Source() object and associated temporary directory). This will be useful in the next commit where more tests are added and files from one test will share the same name as in other tests. This will allow to avoid test poisoning. Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
…ossf#5673) This adds support for downloading and using local .zip NVD year databases instead of being limited to .gz. It's happened that the mirror lacked a .gz file for a year but had the .zip version available. Unfortunately, because we didn't try the .zip file, cve-bin-tool would simply fail when failing to find the .gz file. This adds a fallback to a .zip file both for local and remote files. According to the current zip files in the mirror, it's currently zlib compressed. If the .gz file is missing, .zip will be attempted. If the .gz file is corrupted/mismatching the expected checksum, the .zip will be attempted. The zipfile.ZipFile interface is quite awkward compared to gzip's as you need to create an archive first, then write content to a file within that archive by providing the path within the archive and the content. Similarly, .read() doesn't read a given amount of data, but the whole content of the path passed as argument. So, buffered reading is left for later, if need be. Fixes ossf#5673. Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
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
1 participant
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.
This adds support for downloading and using local .zip NVD year
databases instead of being limited to .gz.
It's happened that the mirror lacked a .gz file for a year but had the
.zip version available. Unfortunately, because we didn't try the .zip
file, cve-bin-tool would simply fail when failing to find the .gz file.
This adds a fallback to a .zip file both for local and remote files.
According to the current zip files in the mirror, it's currently zlib
compressed. If the .gz file is missing, .zip will be attempted. If the
.gz file is corrupted/mismatching the expected checksum, the .zip will
be attempted.
The zipfile.ZipFile interface is quite awkward compared to gzip's as you
need to create an archive first, then write content to a file within
that archive by providing the path within the archive and the content.
Similarly, .read() doesn't read a given amount of data, but the whole
content of the path passed as argument. So, buffered reading is left for
later, if need be.
Fixes #5673
cc @mmind
I have some doubts on test/test_json.py. I'm quite confused by how this works, is it doing a request to NVD? If so, should there be something to adapt to possibly downloading a .zip now?
The diff looks a bit ugly, but looking at it with
git log --ignore-all-spacehelps a bit :)MR check list:
Yes, they don't work even without my patches. I've run the tests I believe were impacted by changes (
EXTERNAL_SYSTEM=1 LONG_TESTS=1 python -m pytest -v test/test_database_defaults.py test/test_source_nvd.py). I'll check what the CI says.Yes, they also don't pass without my patches. Whatever was automatically modified by pre-commit was squashed in the patches.
Yes. Though I would have liked to fake a corrupted gzip we "download" but couldn't figure out how to mock
aiohttp.ClientSession(it's an async context manager for which we want a side-effect for the first call but not the second).No, does this need to be documented? I'm not sure where the appropriate place would be, any hint?
I hope I've done it correctly :)
fixed)Yup, it's there in both PR description and commit log.