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?