-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: Allow NVD data feed metadata downloads to fail on 1st Jan while logging correct errors #8205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Allow NVD data feed metadata downloads to fail on 1st Jan while logging correct errors #8205
Conversation
…he future Signed-off-by: Chad Wilson <[email protected]>
…t error Also refactors the codebase to avoid such issues and simplify the URL handling Signed-off-by: Chad Wilson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes exception handling for NVD data feed downloads by improving error reporting and handling edge cases around January 1st when new year feed files may not yet be available.
Key Changes:
- Improves exception handling to properly report download failures instead of swallowing errors and logging misleading messages about cache.properties failures
- Implements timezone-aware logic to make feed files mandatory only after January 2nd everywhere on earth, avoiding failures on January 1st when NVD feed files may not yet be published
- Refactors URL parsing and formatting logic into a dedicated
FeedUrlclass with improved API
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| core/src/test/resources/suppressions_1_4.xml | Updates test expiration date from 2026 to 2046 to prevent date-dependent test failure |
| core/src/test/java/org/owasp/dependencycheck/xml/suppression/SuppressionParserTest.java | Updates expected date in test to match the suppressions file change |
| core/src/test/java/org/owasp/dependencycheck/data/update/NvdApiDataSourceTest.java | Adds comprehensive test coverage for URL parsing, mandatory year logic, and metadata retrieval with timezone edge cases |
| core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java | Refactors URL extraction to FeedUrl class, improves exception handling in metadata download fallback, implements timezone-aware mandatory year logic, and updates error messages to reference "Cache / Data Feed" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Show resolved
Hide resolved
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Show resolved
Hide resolved
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Show resolved
Hide resolved
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Show resolved
Hide resolved
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Outdated
Show resolved
Hide resolved
64399d1 to
4e9d846
Compare
…il it is January 2nd everywhere This avoids failures on January 1st given feed upload times are not guaranteed to be at any particular time of day in any particular timezone. We assume the metadaa and feed should exist once it is January 2nd in the "earliest" TZ on earth. Signed-off-by: Chad Wilson <[email protected]>
4e9d846 to
7c26b70
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Show resolved
Hide resolved
core/src/test/java/org/owasp/dependencycheck/data/update/NvdApiDataSourceTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Show resolved
Hide resolved
core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Show resolved
Hide resolved
…iDataSourceTest.java Co-authored-by: Copilot <[email protected]>
jeremylong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description of Change
As noted in #8204 currently the code logs a misleading exception when the data feed metadata downloads fail; because it swallows the exception and logs the original
cache.propertiesfailure. That might be OK when using your own proxy or a cache produced by vulnz, but is misleading when using the NVD 2.0 data feeds, especially swallowing the original error (exvsex1).DependencyCheck/core/src/main/java/org/owasp/dependencycheck/data/update/NvdApiDataSource.java
Lines 637 to 638 in 48c4595
When pointing to the NVD 2.0 data feeds, the
cache.propertiesdoesn't exist (I think this is a vulnz invention?), so the metadata files are always needed to be checked remotely.Furthermore, this also changes the feed file for a given year to be considered "mandatory" ONLY once it is 2nd Jan (or later) everywhere on earth. This way the code tries its best to find metadata on January 1st, without failing - so avoids us coupling logic to when NVD happen to publish their feed (currently 03:00 Jan 1st UTC-05:00). Seeking feedback on this - this change is in a separate subsequent commit.
Minor related changes
Related issues
nvdDatafeedstopped working (feed 2.0) #8204Have test cases been added to cover the new functionality?
yes