Skip to content

fix: Make HC5 Downloader honor the connection- and readTimeout settings that the old URLConnectionFactory based downloads observed#7437

Merged
jeremylong merged 1 commit intomainfrom
fix/downloadTimeout
Feb 21, 2025
Merged

fix: Make HC5 Downloader honor the connection- and readTimeout settings that the old URLConnectionFactory based downloads observed#7437
jeremylong merged 1 commit intomainfrom
fix/downloadTimeout

Conversation

@aikebah
Copy link
Copy Markdown
Collaborator

@aikebah aikebah commented Feb 19, 2025

Description of Change

Restores the timeouts (read and connect), including their configurability, that applied to the old UrlConnectionFactory based downloads also for the traffic that now uses the apache HttpClient5 based http clients.

Related issues

Have test cases been added to cover the new functionality?

no

…gs that the old URLConnectionFactory based downloads observed
@boring-cyborg boring-cyborg bot added the utils changes to utils label Feb 19, 2025
@aikebah aikebah added this to the 12.1.1 milestone Feb 19, 2025
Copy link
Copy Markdown
Collaborator

@jeremylong jeremylong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danshome
Copy link
Copy Markdown

danshome commented Feb 20, 2025

@jeremylong When do you expect to release 12.1.1? This fix helps should relieve some of the pressure on the NVD API since their servers aren't timing out the connection either. I'm surprised NIST servers will let a connection stay open forever. Yesterday, I had a build where the same connection was stuck for 24 hours, and they never timed out the connection. @aikebah I will sync the change locally and test and see if I can complete a download with this change and report back.

@danshome
Copy link
Copy Markdown

@aikebah @jeremylong

I'm testing with the changes, and it's not helping the situation. The download has been hung at 43% now for over 30 minutes. I expected the connection to timeout after 60 seconds and to see a retry, but that hasn't happened.

[INFO] --- dependency-check:12.1.1-SNAPSHOT:check (default) @ myapp ---
[INFO] Checking for updates
[INFO] NVD API has 281,906 records in this update
[WARNING] Retrying request /rest/json/cves/2.0?resultsPerPage=2000&startIndex=4000 : 3rd time
[INFO] Downloaded 10,000/281,906 (4%)
[INFO] Downloaded 20,000/281,906 (7%)
[WARNING] Retrying request /rest/json/cves/2.0?resultsPerPage=2000&startIndex=32000 : 3rd time
[INFO] Downloaded 30,000/281,906 (11%)
[INFO] Downloaded 40,000/281,906 (14%)
[INFO] Downloaded 50,000/281,906 (18%)
[INFO] Downloaded 60,000/281,906 (21%)
[INFO] Downloaded 70,000/281,906 (25%)
[INFO] Downloaded 80,000/281,906 (28%)
[INFO] Downloaded 90,000/281,906 (32%)
[INFO] Downloaded 100,000/281,906 (35%)
[INFO] Downloaded 110,000/281,906 (39%)
[INFO] Downloaded 120,000/281,906 (43%). <<<---- Hung here for 30 minutes.

I'm going to see if I can attach a debugger and see exactly where things are hanging up. If you have any suggestions of where I might want to set a breakpoint, that might save me some time getting to the bottom of this.

@danshome
Copy link
Copy Markdown

danshome commented Feb 20, 2025

@aikebah @jeremylong I uploaded a patch file to #7418 that does seem to address the problems with the downloads hanging. It's not a properly integrated fix because I wasn't sure how to externalize the configuration parameters so they could be configured as plugin properties. I just uploaded a patch vs an incomplete pull request. I figured someone that knows the code better could integrate it better.

@matthiaso
Copy link
Copy Markdown

Is there already a timeline when this pull request might be merged/the next version (12.1.1) will be released?

Copy link
Copy Markdown
Collaborator

@jeremylong jeremylong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jeremylong
Copy link
Copy Markdown
Collaborator

@matthiaso I'm hoping we will release the next version this weekend. I'm working on something else that will help.

@jeremylong jeremylong merged commit 8ad4541 into main Feb 21, 2025
@jeremylong jeremylong deleted the fix/downloadTimeout branch February 21, 2025 11:26
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

utils changes to utils

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants