Embedd jxpath

[laeubi]

I have copied the code from Apache, then deleted all "problematic" parts (jdom and servlet and alike) and fixed one compiler error and removed the imports.

One possibly could (optionally) import the package as well if we want to support people supply their own version of jxpath but I don't see any immediate benefit here as it is al internal.

[eclipse-platform-bot]

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

bundles/org.eclipse.e4.emf.xpath/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From 3b40b566eb540a12cd58aa56739bf36cc540f248 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Tue, 14 Jan 2025 10:49:14 +0000
Subject: [PATCH] Version bump(s) for 4.35 stream


diff --git a/bundles/org.eclipse.e4.emf.xpath/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.emf.xpath/META-INF/MANIFEST.MF
index b74bf32abb..0abea75c05 100644
--- a/bundles/org.eclipse.e4.emf.xpath/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.emf.xpath/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Bundle-Name
 Bundle-SymbolicName: org.eclipse.e4.emf.xpath
-Bundle-Version: 0.5.0.qualifier
+Bundle-Version: 0.5.100.qualifier
 Bundle-RequiredExecutionEnvironment: JavaSE-17
 Require-Bundle: org.eclipse.emf.ecore;bundle-version="2.35.0",
  org.eclipse.core.runtime;bundle-version="3.29.0"
-- 
2.47.1

Further information are available in Common Build Issues - Missing version increments.

[akurtakov]

I believe this is the best path forward. As per https://www.eclipse.org/legal/committer-guidelines/#third-party - please open a CQ to get it approved by IP team.

[laeubi]

One might can delete even more code here if it is unused but thats probably something for a later cleanup.

[merks]

It's kind of the dark path and it's a lot of stuff. 😨 But at this point there seems to be little in the way of good alternatives...

[github-actions]

Test Results

 1 818 files  ±0   1 818 suites  ±0   1h 34m 13s ⏱️ - 1m 34s
 7 733 tests ±0   7 496 ✅  -  9  228 💤 ±0  0 ❌ ±0   9 🔥 + 9 
24 360 runs  ±0  23 584 ✅  - 27  749 💤 ±0  0 ❌ ±0  27 🔥 +27 

For more details on these errors, see this check.

Results for commit 15a17bd. ± Comparison against base commit 74efba4.

♻️ This comment has been updated with latest results.

[laeubi]

The build and tests now run all fine. There is some duplicate code and a lot of missing generics, I interpret that @akurtakov and @merks as positive approval that is approach will be accepted if I cleanup the warnings and get a positive CQ.

[akurtakov]

+1 from my side. If/when someone finds the time to implement better approach we can move to it.

[akurtakov]

IMO it's fine to disable the warnings in jxpath code altogether to reduce the diffs with upstream in case the project gets revived.

[mickaelistria]

We used to require a particular review for embedding 3rd party code, more thorough than what's done for shipping 3rd party classes/jars. Is it still true? Should we get in touch with IP team on that matter?

[merks]

Yes, we definitely need to create an issue and get approval:

https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/new

I think it will be very good to document here what was deleted specifically to address the CVE because that question is bound to arise in the future.

[akurtakov]

I think it will be very good to document here what was deleted specifically to address the CVE because that question is bound to arise in the future.

Actually, it would be even better if that information goes into the new org/apache/commons/jxpath/Readme.md file so one can find this info in git directly without any archeology.

[merks]

Actually, it would be even better if that information goes into the new org/apache/commons/jxpath/Readme.md file so one can find this info in git directly without any archeology.

Yes, that is a better idea!

[laeubi]

To summarize what I deleted is:

1. servlet as we don't need servlet support
2. jdom as we don't want this
3. commons-beanutils (because its optional and seems unused)
4. Then some unused classes

So nothing fancy or actually "fixing" any codes, just make the amount of code a little bit smaller.

[laeubi]

We used to require a particular review for embedding 3rd party code, more thorough than what's done for shipping 3rd party classes/jars. Is it still true? Should we get in touch with IP team on that matter?

I have created an IP-Lab issue here:

• https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/18689

[ptziegler]

To summarize what I deleted is:

1. servlet as we don't need servlet support
2. jdom as we don't want this
3. commons-beanutils (because its optional and seems unused)
4. Then some unused classes

So nothing fancy or actually "fixing" any codes, just make the amount of code a little bit smaller.

At least from my side, I'm very happy to see those dependencies gone. Especially since JDOM1/Commons-BeanUtils and Commons-Collections3 would've likely caused additional CVEs.

[HannesWell]

I believe this is the best path forward.

Sorry, but I'm totally on the opposite site. This is IMO the worst possible path forward and should really be applied as a last resort!

Adding 27,000 lines of code for such a rarely used feature is really something I don't want to support. Even if we don't actively develop that code it will again and again cause disturbance! Like different formatter settings, warnings that have to be fixed etc. This is also difficult with the embedded Felix or OSGi API sources in Equinox.

With the improvements made by @ptziegler in #2387 the solution initially proposed in #2290 looks very likely applicable.

I'll work on bringing it to completion. In the meantime I think this should not proceed further (I'm not aware of a reason why this is now more urgent than a month ago).

[merks]

It is indeed a lot of baggage, but it does appear all other efforts are stalled. These old/ancient dependencies do keep raising new questions downstream as well.

[laeubi]

It might looks like much code, but we likely never will need to change anything here, looking at the history this code itself has not changed really. One advantage would be that we are indeed able to fix bugs or delete things if required and are independent from any external resources. And we can get rid of some bad dependencies (especially jdom).

[ptziegler]

I also just want to mention that both approaches aren't mutually exclusive. I'd still like to revisit my PoC at some unspecified point in the future. But that doesn't prevent a fork of JxPath from being a intermediate solution.

Though this obviously also brings the risk of the "intermediate" solution quickly becoming the "permanent" solution, unless this alternative implementation is further pushed...

[laeubi]

Though this obviously also brings the risk of the "intermediate" solution quickly becoming the "permanent" solution, unless this alternative implementation is further pushed...

One problem with a new solution is that it can introduce new bugs, and at least the current one is proven to work.

[HannesWell]

It is indeed a lot of baggage, but it does appear all other efforts are stalled.

I'm now working on completing these other efforts.

Though this obviously also brings the risk of the "intermediate" solution quickly becoming the "permanent" solution, unless this alternative implementation is further pushed...

Absolutely. That's why I would like to put the effort on a sustainable solution.

Though this obviously also brings the risk of the "intermediate" solution quickly becoming the "permanent" solution, unless this alternative implementation is further pushed...

One problem with a new solution is that it can introduce new bugs, and at least the current one is proven to work.

Well, that's the risk of every change. Even a fix of a know bug could introduce a new bug.

[laeubi]

Well, that's the risk of every change. Even a fix of a know bug could introduce a new bug.

Fixing a bug is different from changing completely the behavior / underlying implementation! I just seen that all approaches have currently had some flaws and incompatibility.

And also if its implementation is internal it is widely used (even though some might not notice that or think its not important), so a new implementation has to be 100% compatible as otherwise maybe something will fail silently as it is only referenced in the e4xmi and very hard to debug.

[laeubi]

The code was approved now to be compatible with eclipse platform project as project content.

[mickaelistria]

The about.html file has to be modified to declare contents under the new license/copyright. I think it can also be used to capture some particular note about the code ie "this contains forked code from Apache JXPath from commit 123xyz, initially contributed under the following license and copyright...". IIRC, there are examples of the EDP document.

[HannesWell]

The code was approved now to be compatible with eclipse platform project as project content.

I want to emphasize again, that I'm actively working on #2290 and I'm making good progress. I'll push an update as soon as it becomes close to be ready.
So unless that attempt fails in its final phase I think we should not proceed here, but I'm fine awaiting the completion of #2290 before this is abandoned.

[HannesWell]

With #2290 being submitted, this is not necessary anymore.
Still thank you for your involvement in this topic.