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

Replace ECF's filetransfer.httpclient5 with filetransfer.httpclientjava #1389

Merged
merged 1 commit into from
Sep 23, 2023

Conversation

merks
Copy link
Contributor

@merks merks commented Sep 23, 2023

@merks
Copy link
Contributor Author

merks commented Sep 23, 2023

@laeubi

I'm surprised by this failure:

15:38:40  [INFO] ---------------------------[ eclipse-plugin ]---------------------------
15:38:40  [INFO] Resolving dependencies of MavenProject: org.eclipse.equinox.p2:org.eclipse.equinox.p2.examples.rcp.discovery:2.3.0-SNAPSHOT @ /home/jenkins/agent/workspace/atform.releng.aggregator_PR-1389/rt.equinox.p2/examples/org.eclipse.equinox.p2.examples.rcp.discovery/.polyglot.META-INF
15:38:40  [INFO] Maven Artifact org.eclipse.jdt.ui:org.eclipse.jdt.ui.junit.sampleproject:1.0.0-SNAPSHOT @ /home/jenkins/agent/workspace/atform.releng.aggregator_PR-1389/eclipse.jdt.ui/org.eclipse.jdt.ui.junit.sampleproject/target/org.eclipse.jdt.ui.junit.sampleproject-1.0.0-SNAPSHOT.jar is not a bundle and will be ignored, automatic wrapping of such artifacts can be enabled with <pomDependencies>wrapAsBundle</pomDependencies> in target platform configuration.
15:38:40  [INFO] {osgi.os=linux, osgi.ws=gtk, org.eclipse.update.install.features=true, osgi.arch=x86_64}
15:38:40  [ERROR] Cannot resolve project dependencies:
15:38:40  [ERROR]   Software being installed: org.eclipse.equinox.p2.examples.rcp.discovery 2.3.0.qualifier
15:38:40  [ERROR]   Missing requirement: org.eclipse.equinox.p2.examples.rcp.discovery 2.3.0.qualifier requires 'osgi.bundle; org.apache.httpcomponents.client5.httpclient5 5.0.2' but it could not be found

I noticed now there is no mention of org.apache.httpcomponents.client5.httpclient5 so just removing the feature has resulted in this being removed. I did not expect that given the slicer mode, but I guess my expectation is wrong...

This means I'll have to find and remove all uses of org.apache.httpcomponents.client5.httpclient5 and potentially other things included by that feature:

image

E.g., probably all these must go:

image

They good thing is they go without a replacement... 😄

@merks
Copy link
Contributor Author

merks commented Sep 23, 2023

I'm doing these removals first now:

eclipse-equinox/p2#332

@merks
Copy link
Contributor Author

merks commented Sep 23, 2023

The failure looks like it's caused by the new version of batik:

16:49:16  [ERROR]   Missing requirement: org.eclipse.e4.rcp.feature.group 4.30.0.v20230922-1304 requires 'org.eclipse.equinox.p2.iu; org.apache.batik.css [1.16.0.v20221027-0840,1.16.0.v20221027-0840]' but it could not be found

I'm not sure why that didn't fail when the new version of batik was committed....

I'm also not sure if the batik include could be removed...

This is now pending:

eclipse-platform/eclipse.platform.ui#1164

@merks merks merged commit ce9f0a7 into eclipse-platform:master Sep 23, 2023
2 checks passed
@merks merks deleted the pr-httpclientjava branch September 23, 2023 16:51
@laeubi
Copy link
Contributor

laeubi commented Sep 24, 2023

@merks I'm not completely sure what is surprising here, do you mean that previously things where pulled ins by ecf.httpclient5?

This means I'll have to find and remove all uses of org.apache.httpcomponents.client5.httpclient5 and potentially other things included by that feature

Yes that's the main purpose of the migration, to get rid of these large implicit dependency chains of org.apache.httpcomponents.client5 because sadly they does not (and don't want to) support OSGi so its always a maintenance nightmare for @akurtakov if anything changes there.

They good thing is they go without a replacement...

Obviously these have never really required these bundles so its good to clean that up!

@akurtakov
Copy link
Member

@merks Batik is expressed as requirement in https://github.com/eclipse-platform/eclipse.platform.ui/blob/dde3b606e801b648b466f5ed9fe52e4915f95298/bundles/org.eclipse.e4.ui.css.core/META-INF/MANIFEST.MF#L43 so it should be autopullled in products where needed.

About httpclient5 - it's been one of the biggest CVE concerns for long time but at some point it was just impossible to keep up . E.g. httpclient5/httpcore5 shipped in september release has a good number Vulnerabilities :
CVE-2021-45046
CVE-2021-44832
CVE-2021-44228
CVE-2021-45105
CVE-2020-9488
CVE-2020-15250

Visible at:
https://mvnrepository.com/artifact/org.apache.httpcomponents.core5/httpcore5/5.1.4
https://mvnrepository.com/artifact/org.apache.httpcomponents.client5/httpclient5/5.1.3

Thus getting rid of apache httpclient entirely is the correct thing from maintenance POV as shipping content with well known CVEs is definetely not acceptable today but at the same time there is no manpower to keep maintaining things in the way they used to be.
Of course if there are issues with JVM's httpclient we will have to carefully consider whether these issues out-weight the security concerns. In both cases we need more people be involved on the topic. People doing changes to really owe them and drive the changes to decent state (thanks @merks for being an example for that!) and people facing issues to not only report issues but be the driving force in investigating and fixing the problems. If both things are not happening - the choice between two different "broken" states would never be easy.

@scottslewis
Copy link

Of course if there are issues with JVM's httpclient we will have to carefully consider whether these issues out-weight the security concerns.

I agree. I suggest that the platform team engage some Eclipse consumers and request them to test the javahttpclient (in p2/eclipse) specifically in their networking environments once in a milestone release. When changing filetransfer providers the most problematic thing is has been to get the proxy and default authentication (basic, ntlm, digest) working as well as possible across different corp environments.

For example: eclipse-equinox/p2#330

In both cases we need more people be involved on the topic. People doing changes to really owe them and drive the changes to decent state (thanks @merks for being an example for that!) and people facing issues to not only report issues but be the driving force in investigating and fixing the problems.

Agreed. I cannot test in various networking/proxy/auth environments and am limited to diagnosing and providing directional pointers in the ECF and/or provider code. I will do so as I can (as with 330), but in some cases cannot test/verify fixes (whether from me or not). That's why I would suggest asking milestone consumers in the Eclipse community to assist with testing in their environments if at all possible.

WRT the org.apache.httpcomponents bundles that ECF httpclient5 file transfer has been pulling in: Given how long these bundles have been in Eclipse isn't there some community notification/deprecation procedure that traditionally takes place For removal from eclipse/equinox/rcp/other eclipse projects/broader consumers? Of course these old providers will still be in ECF, so they can still be used if that's how people are consuming them, but if they are using Eclipse/rcp sdk to consume them (e.g. for rest-api, etc) then they could be surprised with removal.

@laeubi
Copy link
Contributor

laeubi commented Sep 25, 2023

I suggest that the platform team engage some Eclipse consumers and request them to test the javahttpclient (in p2/eclipse) specifically in their networking environments once in a milestone release. When changing filetransfer providers the most problematic thing is has been to get the proxy and default authentication (basic, ntlm, digest) working as well as possible across different corp environments.

Maybe these "corporations" that have these complicated requirements/setups should better pay the "platform team" to keep up with their requirements? I think if we can test the basic auth this is fair enough as it is the most widely used setup.

I cannot test in various networking/proxy/auth environments and am limited to diagnosing and providing directional pointers in the ECF and/or provider code.

I think this is fine, what I'm a bit confused is that it seems there is not a single test in ECF regarding authentication at all, or is P2 using ECF if a special way here?

but if they are using Eclipse/rcp sdk to consume them (e.g. for rest-api, etc) then they could be surprised with removal.

ECF is an implementation detail of P2 not an API a consumer should rely on.

@akurtakov
Copy link
Member

Request to test sent on crossproject: https://www.eclipse.org/lists/cross-project-issues-dev/msg19748.html

About notification/deprecation period - this is really implementation detail and there were no such periods when the switch from jakarta httpclient to apache httpclient 4 happened nor when the apache httpclient 4 to 5 happened.

@scottslewis
Copy link

I suggest that the platform team engage some Eclipse consumers and request them to test the javahttpclient (in p2/eclipse) specifically in their networking environments once in a milestone release. When changing filetransfer providers the most problematic thing is has been to get the proxy and default authentication (basic, ntlm, digest) working as well as possible across different corp environments.

Maybe these "corporations" that have these complicated requirements/setups should better pay the "platform team" to keep up with their requirements? I think if we can test the basic auth this is fair enough as it is the most widely used setup.

I agree that the platform team should probably receive better pay from the 'corporations' e.g. for testing, but otoh, I believe the platform team has some (I don't really know how many) dedicated (read paid) devs/testers. ECF does not/never has had such a commitment from EF or platform team or others. ECF (mostly me personally) has been supporting the platform/p2 team for 15+ years on a strictly volunteer basis. At this point, I regret that I feel it was an exercise/education in OSS exploitation.

In short, I'm sympathetic to the platform being starved of resources, but I'm more sympathetic elsewhere. :-)

I cannot test in various networking/proxy/auth environments and am limited to diagnosing and providing directional pointers in the ECF and/or provider code.

I think this is fine, what I'm a bit confused is that it seems there is not a single test in ECF regarding authentication at all, or is P2 using ECF if a special way here?

To be direct, yes P2 is using ECF in a special way here (basic auth). This was not my design choice...it was done by p2 team at the time of initial integration of ECF filetransfer with p2 (some 15+ years ago). We/I responded to their requests for support/testing of auth in p2 at the time (using a much older...i.e. 3.x version of apache httpclient I believe), and in some cases the code contributions were made by non-ECF committers.

Yes, it's a miss in the ECF test cases to not test basic auth at ECF level. auth/proxy tests are more needed at p2 level IMHO (because of the interaction between Eclipse auth/proxy UI), but contributions are welcome as usual.

but if they are using Eclipse/rcp sdk to consume them (e.g. for rest-api, etc) then they could be surprised with removal.

ECF is an implementation detail of P2 not an API a consumer should rely on.

There are non-Eclipse consumers of ECF filetransfer and or other parts of ECF (e.g. Oomph).

Yes, from Eclipse point of view only, ECF is an implementation detail of P2...so IMHO it should be P2/Equinox/platform team (whatever is appropriate) to take responsibility for testing and bug fixing of new providers (at least) for Eclipse...especially when it comes to proxy/auth (which needs cooperation from someone...i.e. the consumers with the crazy/old proxy configurations, or using basic auth with passwords in cleartext.

I believe I publicly warned everyone several times (months ago) about the testing needs wrt auth/proxy issues with a new provider impl.

For some 15 years we (mostly I) have supported Eclipse directly...for obvious reasons...with nary a thank you (and in fact a lot of what I consider grief from the platform team...e.g. imposed schedules, fixes, implicit or explicit shade about supporting p2/ecf/apache code I didn't create, etc).

To be short, I'm done with that.

With additional issues, right now I can't be proactive I was yesterday with basic auth support. I will answer technical questions when I can and do builds/releases with request one week in advance.

@merks
Copy link
Contributor Author

merks commented Sep 26, 2023

@scottslewis

Thanks.

@scottslewis
Copy link

There is a new ECF snapshot version for 3.14.40 here

https://download.eclipse.org/rt/ecf/snapshot/site.p2/3.14.40.v20231021-2127

This snapshot has merged the two prs:

eclipse/ecf#71
eclipse/ecf#72

along with associated feature and bundle version number increases

@merks
Copy link
Contributor Author

merks commented Oct 22, 2023

@scottslewis

Thanks for the heads up!

@jcompagner
Copy link

does this mean that ECF will not include http core/client anymore in a next release?

I asked this because even the latest 3.14.40 snapshot above still have the apache http clients in it.
And they are older versions (5.1 not 5.2.x) which for us has 1 or 2 bugs fixed.

Problem is we build on top of eclipse and i don't really want to ship all kind of duplicate jars. But the feature of ecf is hardcoded to that release so that will always also come along..

So is the planning to really not ship the http client anymore (not sure if there are other plugins/features also doing this?)
and if that is not in a next release yet, do you plan to at least upgrade once to the latest releases of the Apache http client?

@jcompagner
Copy link

About httpclient5 - it's been one of the biggest CVE concerns for long time but at some point it was just impossible to keep up . E.g. httpclient5/httpcore5 shipped in september release has a good number Vulnerabilities : CVE-2021-45046 CVE-2021-44832 CVE-2021-44228 CVE-2021-45105 CVE-2020-9488 CVE-2020-15250

Visible at: https://mvnrepository.com/artifact/org.apache.httpcomponents.core5/httpcore5/5.1.4 https://mvnrepository.com/artifact/org.apache.httpcomponents.client5/httpclient5/5.1.3

those CVE's are just dependency vulnerabilities right? so all log4j? which can be just resolved outside of the http plugin
Also its not that the manifest of the http plugin is enforcing anything because they have a horrible manifest... (and for some reason they think it is hard to do osgi... not sure why, at first i think they really think they have to have a different osgi jar which they had previously)

@laeubi
Copy link
Contributor

laeubi commented Oct 24, 2023

does this mean that ECF will not include http core/client anymore in a next release?

correct

I asked this because even the latest 3.14.40 snapshot above still have the apache http clients in it.

It is possible that someone still reference that so it is pulled in. @merks has usually tools to find out whats going on.

And they are older versions (5.1 not 5.2.x) which for us has 1 or 2 bugs fixed.

That's why you usually should ship whats best fit with your product.

So is the planning to really not ship the http client anymore

Yes.

do you plan to at least upgrade once to the latest releases of the Apache http client

No. But please note that ECF is a separate project so they might decide different.

@scottslewis
Copy link

So is the planning to really not ship the http client anymore

Yes.

do you plan to at least upgrade once to the latest releases of the Apache http client

No. But please note that ECF is a separate project so they might decide different.

@laeubi is correct, ECF is a separate project from Eclipse and our answer to the above two questions is different.

ECF will be shipping httpclient5 filetransfer provider as part of it's repos (alongside the javahttpclient provider used by Eclipse) for at least the near future.

Of course we will consider upgrading the httpclient5 version to incorporate fixes as long as access to httpclient5 as OSGi bundles is assured (I don't know whether httpclient5 new versions are maintaining OSGi metadata properly) and some resources are available to do testing and releng.

@laeubi
Copy link
Contributor

laeubi commented Oct 26, 2023

ECF will be shipping httpclient5 filetransfer provider as part of it's repos

To make it clear, "we" as platform will not actively include any httpclient5 but ECF as part of simrel will do (one way or the other). So one might find the bundles somewhere but not necessarily as part of the eclipse sdk.

I don't know whether httpclient5 new versions are maintaining OSGi metadata properly

The main reason for the switch to javahttp was that hey do not and are not very enthusiastic in doing so.

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