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

Fix HttpClientProxyCredentialProvider.getPasswordAuthentication #78

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

merks
Copy link
Contributor

@merks merks commented Nov 14, 2023

Ensure that httpclientjava's
HttpClientProxyCredentialProvider.getCredentials(AuthScope) is called with a correct AuthScope from getPasswordAuthentication.

Also update some target platform dependencies for the 2022-03 target to use the most recently released versions of client5/core5. Focus particularly on what is required by the httpclient5 feature because that's pulled into p2 and hence almost all installations, e.g., it does not need to include org.apache.commons.logging and should use the latest org.apache.commons.commons-codec rather than org.apache.commons.codec.

#76
#77

Ensure that httpclientjava's
HttpClientProxyCredentialProvider.getCredentials(AuthScope) is called
with a correct AuthScope from getPasswordAuthentication.

Also update some target platform dependencies for the 2022-03 target to
use the most recently released versions of client5/core5.  Focus
particularly on what is required by the httpclient5 feature because
that's pulled into p2 and hence almost all installations, e.g., it does
not need to include org.apache.commons.logging and should use the latest
org.apache.commons.commons-codec rather than org.apache.commons.codec.

eclipse#76
eclipse#77
@eclipse-ecf-bot
Copy link

Can one of the admins verify this patch?

@merks
Copy link
Contributor Author

merks commented Nov 14, 2023

FYI, I also did a test build in EMF's ci instance to produce this repository with signed content:

https://ci.eclipse.org/emf/job/test-ecf/lastSuccessfulBuild/artifact/releng/org.eclipse.ecf.releng.repository/target/repository/

This also lets me verify that with the above build, the Platform will be able to consume only the latest dependencies:

image


<plugin
id="org.apache.commons.logging"
id="org.apache.commons.commons-codec"
Copy link
Contributor

Choose a reason for hiding this comment

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

Did org.apache.commons.logging get dropped on purpose here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because I checked and there are no requirements on this by any of the http5 stuff. I suspect it was just copied from the 4.x stuff. Here is SimRel after you contributed httpclient5:

image

Note the image from the previous comment resolves using planner mode, so if something actually required it, even only transitively, it would still be present in the target for that location...

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks Ed for confirming.

@scottslewis scottslewis merged commit d5a81be into eclipse:master Nov 14, 2023
2 checks passed
@scottslewis
Copy link
Contributor

I have merged this commit and built a new snapshot that includes it here:

https://download.eclipse.org/rt/ecf/snapshot/site.p2/3.14.40.v20231114-1017

@merks merks deleted the issue-76 branch November 15, 2023 08:52
@scottslewis
Copy link
Contributor

Hi Ed. Does this comment

eclipse-equinox/p2#381 (comment)

indicate that the fix for this issue doesn't work as desired?

@sratz
Copy link
Contributor

sratz commented Nov 16, 2023

Hi Ed. Does this comment

eclipse-equinox/p2#381 (comment)

indicate that the fix for this issue doesn't work as desired?

Indeed, it's not enough :(
See #76 (comment) + #76 (comment)

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.

5 participants