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

Improve proxy authentication preventing NPE and allow Basic Authentication (v3) #2088

Merged
merged 7 commits into from
Feb 29, 2024

Conversation

DybekK
Copy link
Contributor

@DybekK DybekK commented Feb 27, 2024

This PR addresses the problem described in #2050, where requests sent by HttpClientBackend to a proxy requiring Basic authentication were failing due to JDK's default settings, resulting in a 407 response and a null body, leading to errors. By removing Basic from jdk.http.auth.tunneling.disabledSchemes, we now allow HttpClient to use proxy authentication successfully. This behaviour has been documented to guide users on enabling Basic authentication for HTTP tunneling and inform them about security flaws.

Additionally, to avoid NPE from null response bodies, we return empty stream.

@DybekK DybekK requested a review from adamw February 27, 2024 21:13
@kciesielski
Copy link
Member

@DybekK could you add tests for these cases?

docs/conf/proxy.md Outdated Show resolved Hide resolved
@adamw
Copy link
Member

adamw commented Feb 28, 2024

@kciesielski I'm not sure if a test using proxies is doable in a reasonable amount of time, but maybe :)

@DybekK we'll also need a PR to master w/ the same changes please :)

@DybekK DybekK merged commit bec8fc5 into v3 Feb 29, 2024
16 checks passed
@DybekK DybekK deleted the fix/proxy-auth branch February 29, 2024 09:15
@DybekK DybekK changed the title Improve proxy authentication preventing NPE and allow Basic Authentication Improve proxy authentication preventing NPE and allow Basic Authentication (v3) Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants