Skip to content
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

feat: Use Apache HTTPClient for downloads of public resources #6949

Merged
merged 9 commits into from
Sep 18, 2024
Merged

Conversation

aikebah
Copy link
Collaborator

@aikebah aikebah commented Sep 7, 2024

Fixes Issue #6800

Description of Change

Swap out use of RAW JDK http(s) handling by Apache HTTP-Client in order to also take into account the https.proxyUser and https.proxyPassword JAVA_TOOL_OPTIONs

Add a resources to document and support manual testing of HTTP-Basic Authenticating proxy usage.

Also lays the groundwork for resolving #5387 by adding Settings-keys for the credentials of a mirrored HostedSuppressions- and KnownExploitedVulnerabilities-file. CLI instances initialized from a custom properties file could already exploit this, but activation for all integrations is left as an excercise in the context of #5387.

For now only a draft, as there is still some direct raw usage left behind in the codebase that I also intend to replace by HTTPClient. However I think now is a good time to already collect feedback on the initial coding for the replacement of the Downloader class

Have test cases been added to cover the new functionality?

Not yet, intend to evaluate and potentially extend the existing testcases that already partially test the functionality

@boring-cyborg boring-cyborg bot added ant changes to ant cli changes to the cli core changes to core maven changes to the maven plugin tests test cases utils changes to utils labels Sep 7, 2024
@aikebah aikebah added this to the 11.0.0 milestone Sep 7, 2024
@jeremylong
Copy link
Owner

@aikebah tests are passing - should this still be a draft PR?

@aikebah
Copy link
Collaborator Author

aikebah commented Sep 17, 2024

@jeremylong All code as committed I consider final, so if you're eager to get a release out it would be fine to remove draft status and get 'most usages replaced' as feature for 11.0

The draft status is not-yet removed as there are a few analyzers that don't use the HTTPClient yet - Nexus and Artifactory IIRC, which could also be postponed for a later feature-update I guess.

@aikebah
Copy link
Collaborator Author

aikebah commented Sep 17, 2024

Confirmed... only NexusSearch and ArtifactorySearch are pending. As both are typically solutions within the enterprise datacenter not requiring proxy access I think it would be fine to finish up this part and leave the remainder phase-out of URLConnectionFactory for a later feature-release in the 11.x series.

@aikebah aikebah marked this pull request as ready for review September 17, 2024 17:39
Copy link
Owner

@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

@aikebah aikebah changed the title feat: Use Apache HTTPClient for all downloads feat: Use Apache HTTPClient for downloads of oublic resources Sep 18, 2024
@aikebah aikebah changed the title feat: Use Apache HTTPClient for downloads of oublic resources feat: Use Apache HTTPClient for downloads of public resources Sep 18, 2024
@aikebah aikebah merged commit 865e39e into main Sep 18, 2024
8 checks passed
@jeremylong
Copy link
Owner

Thanks for the work on this PR! It looks like you have completed the groundwork for the remaining two analyzers (as seen in the CentralAnalyzer). Really appreciate all the work you put into the project!

@jeremylong jeremylong deleted the issue-6800 branch September 18, 2024 21:23
@aikebah
Copy link
Collaborator Author

aikebah commented Sep 18, 2024

Was a nice exercise for part of my holiday. NexusSearch currently cooking locally on lower velocity besides my daytime job.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ant changes to ant cli changes to the cli core changes to core maven changes to the maven plugin tests test cases utils changes to utils
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants