Remove dependencies on org.apache.commons.jxpath

[mickaelistria]

org.apache.commons.jxpath is a very old library, full of CVE and unmaintained for 14 years.
On the other end, the JDK ships some build path implementation that is maintained in high quality just like everything in the JDK.

We should remove deps on jxpath and use JDK standard implementation.

[mickaelistria]

@merks Is there something already existing in EMF land to handle XPath without jxpath and that Platform could reuse?

[akurtakov]

Or is this bundle helpful/used in anyway? It never got to 1.0.0.

[merks]

@mickaelistria Sorry, no.

[mickaelistria]

Or is this bundle helpful/used in anyway? It never got to 1.0.0.

Unfortunately, it seems used (or at least referenced) by org.eclipse.e4.model.workbench and by org.eclipse.e4.tools which is transtively used by the model.spy.

[ptziegler]

What would be a good starting point for removing this dependency? I noticed that for the 2024-09, the platform switched to the official Maven jar. This now introduces some very nasty dependencies like jdom and commons-collections, which I'd really like to not have in our application:

Depending on how difficult this is, I might be able to get some time to work on this topic.

[laeubi]

@ptziegler the easiest way is to remove the dependency from the manifest (either the package or the bundle) and then you will see the compile errors and can decide how to replace this with standard java XPath. If that's done one can remove it from the target entirely and see if there are some "hidden" references.

Even if we can not yet remove all usages it might be worth to start bundle-by-bundle.

[ptziegler]

Hm... this will be tricky. At least for the e4 workbench, this library is used to look up elements relative to the application model, rather than the current fragment:

With it only used being here:

eclipse.platform.ui/bundles/org.eclipse.e4.ui.model.workbench/src/org/eclipse/e4/ui/model/fragment/impl/StringModelFragmentImpl.java

Lines 355 to 393 in a43889d

	private void mergeXPath(MApplication application, List<MApplicationElement> ret, String xPath) {
	List<MApplicationElement> targetElements;
	if ("/".equals(xPath)) {
	targetElements = Collections.singletonList(application);
	} else {
	XPathContextFactory<EObject> f = EcoreXPathContextFactory.newInstance();
	XPathContext xpathContext = f.newContext((EObject) application);
	Iterator<Object> i = xpathContext.iterate(xPath);
	targetElements = new ArrayList<>();
	try {
	while (i.hasNext()) {
	Object obj = i.next();
	if (obj instanceof MApplicationElement) {
	MApplicationElement o = (MApplicationElement) obj;
	targetElements.add(o);
	}
	}
	} catch (Exception ex) {
	// custom xpath functions will throw exceptions
	ex.printStackTrace();
	}
	}
	for (MApplicationElement targetElement : targetElements) {
	EStructuralFeature feature = ((EObject) targetElement).eClass().getEStructuralFeature(getFeaturename());
	if (feature != null) {
	List<MApplicationElement> elements;
	elements = new ArrayList<>();
	for (MApplicationElement element : getElements()) {
	elements.add((MApplicationElement) EcoreUtil.copy((EObject) element));
	}
	if (!elements.isEmpty()) {
	ret.addAll(ModelUtils.merge(targetElement, feature, elements, getPositionInList()));
	}
	}
	}
	}
	} //StringModelFragmentImpl

In order to switch to the Java XPath API, one would need to keep track of the XML structure and then go:

Application model (Java) -> Application node (XML) - XPath -> Application Element node (XML) -> Aplication Element model (Java)

[laeubi]

An alternative of course is to modernize / adapt / copy / ... the library itself.

[mickaelistria]

One possible iteration would be to extract the part that process XPath in model fragments in a dedicated plugin and have a StringModelFragmentImpl.java use an extension point or a service registry. Then the xpath stuff will be more isolated and we could even consider fully removing it from default Platform/SDK/IDE packages as it doesn't seem to be used internally.

[vogella]

Allowing to extend via xpath the application model is a very important part of the Eclipse RCP story.

[ptziegler]

An alternative of course is to modernize / adapt / copy / ... the library itself.

There are already solutions for the usage of BeanUtils (2017)
apache/commons-jxpath#13

and jdom (2022)
apache/commons-jxpath#27

But there hasn't been much activity to get either merged (let alone the last release being from 2008). Even if those are fixed, it is only a matter of time before more issues pop up.

One possible iteration would be to extract the part that process XPath in model fragments in a dedicated plugin and have a StringModelFragmentImpl.java use an extension point or a service registry.

That would certainly help in our case, as we are still primarily working with the E3 model and therefore don't use xpaths. If the usage of JXPath is hidden behind a service, it would also allow for an easier migration to a "different" implementation (whatever that may look like) at some point in the future.

[laeubi]

But there hasn't been much activity to get either merged (let alone the last release being from 2008). Even if those are fixed, it is only a matter of time before more issues pop up.

It would be possible to merge the relevant code into eclipse and maintain it here. For mee it seems we are mainly interested in parsing the XPath Expression into something that is then usefull to traverse the model so one could hopefully strip of most parts of it.

Making it a service/extension does not sound very useful path see @vogella as this is an integral part of E4.

[mickaelistria]

Making it a service/extension does not sound very useful path see @vogella as this is an integral part of E4.

If it allows RCP providers who don't need it to avoid deprecated dependencies and CVEs, it's useful to make it optional.
But sure, the ideal story would be that some people who build on top of this feature invest in its maintenance. But if this doesn't happen, we need to consider alternatives.

[vogella]

Making it a service/extension does not sound very useful path see @vogella as this is an integral part of E4.

If it allows RCP providers who don't need it to avoid deprecated dependencies and CVEs, it's useful to make it optional. But sure, the ideal story would be that some people who build on top of this feature invest in its maintenance. But if this doesn't happen, we need to consider alternatives.

IIRC the PDE spies and wizards also use it.

[HannesWell]

On the other end, the JDK ships some build path implementation that is maintained in high quality just like everything in the JDK.

About a year ago I looked a bit into implementing some kind of adapter to provide a XML DOM view of an Ecore/EObject model so that the XPath facility built into the JDK can be used instead.
I didn't succeed at that time and therefore it stalled, but since the Ecore/EObject model is read from an XML I don't see a conceptual reason why this shouldn't be possible and therefore think that would be the right way forward.

@ptziegler, if you are interested in continuing that work I can push the current (incomplete) state in a draft PR this evening and you can try to complete it?

[laeubi]

If it allows RCP providers who don't need it to avoid deprecated dependencies and CVEs

Please define "don't need it" it is easy to clam that "I don't need it" because I never used it or heard about it, still it is an officially support feature of the E4 model, making it "optional" will break things without an easy way to detect it e.g. these are the places where platform using the feature:

and one can only guess if / how / where it is used in external plugins in the Eclipse IDE eco-system.

Also the UI offers this:

so one has to somehow also handle it there (what should a wizard suggest? Directly require bundle? Modify all product launches) ... so just because most Eclipse Platform is stuck at E3 it is quite unrealistic to make it really "optional" again and every E3 RCP is actually using E4 (or require it).

[laeubi]

@HannesWell @ptziegler actually this already is abstracted with org.eclipse.e4.emf.xpath so I would just look there how one can get rid of jxpath by e.g. just copy the used classes in this bundle for example.

[mickaelistria]

still it is an officially support feature of the E4 model,

Support is not Beetlejuice, you don't have to call it 3 times for it to appear. Support comes with manpower being invested in keeping the piece of software safe and efficient. Here, jxpath is the entry-point of a chain of CVE-full libraries and jxpath itself doesn't seem supported either.Tthis piece of code is working and shipped but it is not supported, that's a major difference. And IMO, longevity of the project comes with its ability to stop "officially" supporting things that are not practically supported.
But I would love to be proven wrong here and to see someone continuing @HannesWell 's work and keeping xpath: to work with safer dependencies, so we can tell that xpath: in model fragments is actually supported.

[laeubi]

It works and making it optional is clearly a breaking change.

Anyways I would suggest to first migrate org.eclipse.e4.emf.xpath from require bundle -> import package, then anyone who is concerned about " chain of CVE-full libraries" could provide an own bundle that exports the required packages as a drop-in replacement.

This would also make more clear what parts of jxpath are actually used...

[ptziegler]

@ptziegler, if you are interested in continuing that work I can push the current (incomplete) state in a draft PR this evening and you can try to complete it?

@HannesWell If you still have the prototype I could build upon, then that would already help out a lot to get me started.

Straight out removing XPath support is obviously a no-go. But if it's somehow possible to separate the JXPath implementation from the interface, then everyone would benefit from it. Especially, if it can be done in a way that fits into the constraints of the classes/interfaces in `org.eclipse.e4.emf.xpath``.

Though I'll likely won't be able to work on it tomorrow or even next week, but hopefully I can at least get started at the start of next month.

then anyone who is concerned about " chain of CVE-full libraries" could provide an own bundle that exports the required packages as a drop-in replacement.

But wouldn't that result in effectively the same thing? Whoever wants to avoid this dependency has to implement a "not-JXPath" variant, except that it is then likely not contributed back.

[laeubi]

Your inital report included this

I noticed that for the 2024-09, the platform switched to the official Maven jar. This now introduces some very nasty dependencies like jdom and commons-collections

So for this case you can pack whatever seems suitableof a different variant than what is shipped by default (e.g. a non official build one as well).

[merks]

For what it's worth, the project builds easily and produces this bundle with purely optional dependencies:

[laeubi]

That's why I suggest taking smaller steps, require bundle is the strongest dependency as the symbolic name must match, using import package can loosen the dependency a bit more allowing other bundles name and versions more easy, if then one even would try to build a stripped top the bare minimum version it could be easily contributed (we just need a CQ then...).

But abstracting the abstraction to allow alternative implementations does not seem a good fit for this case...

[HannesWell]

If we say it's supported or not is IMHO not so important but currently it's working and removing it entirely would be a significant decrease in functionality of E4, so I hope we make progress in moving forward :)

As promised I have pushed the current state of my work to

• [WIP] [E4 Xpath] Replace apache.commons.jxpath by javax.xml.xpath #2290

And also extracted some other minor improvements that can be applied now already, which includes the suggestion to use Import-Package instead of Require-Bundle:

• Various minor improvements for E4 EMF Xpath #2289