-
Notifications
You must be signed in to change notification settings - Fork 14
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
org.eclipse.ecf.provider.filetransfer.httpclientjava does not support proxy authentication #76
Comments
I agree with @merks that this might fix some (or even all) of the observed proxy creds regressions. I have no way of testing whether it will help or not however, so we should line up some testing assistance and/or environment before making this fix. |
I am one of the users behind an authentification proxy. If you could provide me with a build of the eclipse sdk that contains the fix @merks suggests, I could do a test and see wether it works again - if that is of any help of course. |
Yes, my sense is that the fix can do no harm. In fact the called getCredentials method looks to me highly likely to produce NPEs when passed null as it current does so I feel we should change this regardless of there not being good automated tests... Locally for testing this technology stack, I use the installer with these projects for the workspace: That lets me change and test everything (ECF, p2, and Oomph) locally. If you wish to help with testing such a change before there is an ECF build, let's discuss that further in this Oomph issue that I just opened: Thanks again for reporting the issue... |
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
I produced this update site that you could use for testing the potential fix: |
I have looked into this in more detail and have come to the conclusion that the implementation is incomplete regarding proxy authentication. The whole code base of java 11 client was copied from apache http client 5 and adapted. However, apache 5 worked significantly different:
The Java 11 HTTP client however, does not allow request-specific credential providers or proxy selectors. It only allows a global one set on the HttpClient instance during its construction. The current ECF wrapper for Java 11 tries to imitate such a 'request context' object by introducing a custom interface and the proxy and credential information goes to the implementation of that interface: Lines 64 to 91 in 3485821
The problem is - as can be seen by the I don't think there is an easy way around this. Two options have come to mind:
|
Another thing: The Java HTTP client - by default - disallows Basic Authentication for proxies since Java 8u111:
So after solving the problem mentioned above, we would - additionally - need the following system properties to be set:
to get things working. This would need to happen probably via eclipse.ini, as these are initialized statically very early: |
@sratz Prox Auth is done on the http level, so one always could set these headers manually (we do that at Tycho already)... |
Note that final platform build is on Wednesday so it will be good to have a final repository URL for that: eclipse-platform/eclipse.platform.releng.aggregator#1390 Also for contribution to SimRel. If you let me know the URL I will do that for ECF. Thanks. |
Here is release build repo for ECF 3.14.40: https://download.eclipse.org/rt/ecf/3.14.40/site.p2/3.14.40.v20231114-1017/ |
Can you consider reopening this for planning because we have started a funded dev effort initiative to cover this issue. |
There seems to be an open question as to which approach for handling http going forward should happen. The ECF and Equinox/p2 project teams need to decide on the correct course of action so that the planning council can recommend funding this work to the IDE WG. (See this comment in gitlab) |
This is a related problem that I too encounter only with the newer provider: |
See the following p2 issue for background details:
eclipse-equinox/p2#381
It's not clear (to me) if the proxy needs to be configured by java.net.http.HttpClient.Builder.proxy(ProxySelector) as suggested here:
https://openjdk.org/groups/net/httpclient/intro.html
Note that with org.eclipse.ecf.internal.provider.filetransfer.httpclientjava.Activator.USE_SHARED_CLIENT_DEFAULT being true, I think there is only a singe HttpClient that is shared.
These two fields
ecf/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclientjava/src/org/eclipse/ecf/internal/provider/filetransfer/httpclientjava/ECFHttpClientFactory.java
Lines 35 to 38 in bbf5ffa
which are copied from here:
ecf/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclient5/src/org/eclipse/ecf/internal/provider/filetransfer/httpclient5/ECFHttpClientFactory.java
Lines 62 to 63 in bbf5ffa
are not actually used. It's not clear what, if anything, needs to be configured in this regard.
The following implementation detail is clearly incomplete:
ecf/providers/bundles/org.eclipse.ecf.provider.filetransfer.httpclientjava/src/org/eclipse/ecf/internal/provider/filetransfer/httpclientjava/HttpClientProxyCredentialProvider.java
Line 77 in bbf5ffa
Perhaps it would be correctly implemented like this:
It's possible that if this were fixed, some additional proxy configurations would "just work"...
The text was updated successfully, but these errors were encountered: