Update M2E to 2.0.0 (Vulnerability in Apache Lucene 8.9) #2096

snjeza · 2022-05-20T23:58:10Z

Fixes #2085

Test vsix: https://github.com/snjeza/vscode-test/raw/master/java-1.6.2.vsix

Signed-off-by: Snjezana Peco snjezana.peco@redhat.com

snjeza · 2022-05-21T02:04:11Z

test this please

rgrunber

Overall size is reduced by ~5MB (46MB -> 41MB) (compressed & about the same uncompressed). I can also confirm the space taken by the extension in globalStorage is reduced by ~10MB (22MB -> 12MB).

So org.apache.commons.codec_1.14.0.v20200818-1422.jar is new (m2e-core switched from guava to commons.codec), org.eclipse.m2e.maven.indexer_1.18.1.20211011-2139.jar is gone, and org.eclipse.m2e.apt.core_2.0.0.20220518-0743.jar replaces org.jboss.tools.maven.apt.core_1.5.4.202201260914.jar.

However, when I open a simple maven project, the initialization+building doesn't seem to happen, and there is no notification (eg. for VS Code) to indicate the the projects were imported. I notice the syntax+standard language servers are both in the background and both remain up. If I open an actual Java source file, the build progress spins forever. I don't see any errors in the client or server logs. I tested this in a completely cleaned instance with only a generated vsix.

testforstephen · 2022-05-27T03:59:29Z

I remember "Maven for Java" extension has a feature to fix unresolved type by querying Maven indexer, @Eskibear could you pls double check if Maven extension is compatible with the latest m2e 2.0?

Eskibear · 2022-05-27T05:27:25Z

Verified that Maven for Java still works.

But an error related to lifecycle mappings occurs...not sure if it's related. I'm using spring-petclinic as sample project.

[{
	"resource": "/C:/Users/admin/projects/spring-petclinic-main/pom.xml",
	"owner": "_generated_diagnostic_collection_name_#3",
	"code": "0",
	"severity": 8,
	"message": "Lifecycle mapping \"org.eclipse.m2e.jdt.JarLifecycleMapping\" is not available. To enable full functionality, install the lifecycle mapping and run Maven->Update Project Configuration.",
	"source": "Java",
	"startLineNumber": 4,
	"startColumn": 95,
	"endLineNumber": 4,
	"endColumn": 103
}]

UPDATE
Not sure if it was related to some outdated cached workspace states. However, after I clone the project into another location, there's no such error when I open it.

snjeza · 2022-05-28T01:23:35Z

@rgrunber @testforstephen @Eskibear Could you, please, check https://github.com/snjeza/vscode-test/raw/master/java-1.6.6.vsix

snjeza · 2022-05-28T04:34:01Z

test this please

Eskibear · 2022-05-30T02:41:15Z

Verified that "fix unresolved type" feature in Maven for Java still works.

rgrunber

(wrong review, sorry, still looking at this)

approved wrong PR

rgrunber · 2022-05-30T19:02:06Z

I'm currently having some issues with this change. @snjeza , not sure if you're able to reproduce but if I make sure to build my vsix with a jre/ folder. (ie. npx gulp download_jre, Java 17), then the project import doesn't happen.

Looking and comparing the server logs I see that the Workspace initialized in .. message never appears, nor does the initialized to indicate the client made the request and the server responded. I can try looking into this.

Update : The client sent the initialized request but the server never responded.

snjeza · 2022-05-30T19:43:33Z

... I can try looking into this.

checking...

org.eclipse.jdt.ls.core/logback.xml

snjeza · 2022-05-30T21:34:04Z

Looking and comparing the server logs I see that the Workspace initialized in .. message never appears, nor does the initialized to indicate the client made the request and the server responded. I can try looking into this.

The issue has been introduced with laeubi/m2e-core@5f1ae0c
This commit causes the m2e.core plugin to start when starting the o.e.core.resources plugin before the java.ls plugin.
In this way, the logback.configurationFile property is set too late
@rgrunber you can try the following

create logback.xml

<configuration scan="true">
</configuration>

add -Dlogback.configurationFile=<path_to_logback.xml> to java.jdt.ls.vmargs
restart VS Code
run Clean Java Language Server workspace

I will try to make a workaround.

The best solution would be to remove the ConsoleAdapter adapter from m2e's logback.xml - https://github.com/eclipse-m2e/m2e-core/blob/master/org.eclipse.m2e.logback.configuration/defaultLogbackConfiguration/logback.xml

cc @fbricon @mickaelistria @HannesWell @laeubi

laeubi · 2022-05-31T04:14:05Z

This commit causes the m2e.core plugin to start when starting the o.e.core.resources plugin before the java.ls plugin.

Then you plugin has a start-order dependency and should probably be marked to start at a higher start-level (e.g. 2 or 3) to start as soon as possible, but relying on setting a system property sounds fragile anyway, so probably a better aproach would be to refresh the Logback config instead.

m2e's logback.xml

the m2e logback is an internal implementation detail, so probably it would be good to describe what one likes to archive with setting the logback config.xml, e.g. do you actually want to redirect the maven logs? The m2e logs? or do you like to configure your own logging?

laeubi · 2022-05-31T04:27:36Z

@snjeza I read your comment about STD-out creating confusion, why not remove the org.eclipse.m2e.logback.configuration from your project and provide an own fragment with configuration suitable for jdt.ls?

snjeza · 2022-05-31T04:38:10Z

@laeubi Thanks for your comment.
I have fixed the issue with a new plugin that configures logback.xml before starting the m2e.core or java.ls plugins.
It doesn't require any changes in m2e.
@rgrunber @fbricon could you, please, review.

laeubi · 2022-05-31T04:41:32Z

Alright, still I think it would be the best to omit the org.eclipse.m2e.logback.configuration then as it seem not valuable for your use case and as I said is an implementation detail of m2e, so it can change without any notice.

snjeza · 2022-05-31T05:08:38Z

Alright, still I think it would be the best to omit the org.eclipse.m2e.logback.configuration then as it seem not valuable for your use case and as I said is an implementation detail of m2e, so it can change without any notice.

Actually, the ch.qos.logback.classic plugin breaks our stdout. We can remove org.eclipse.m2e.logback.configuration. It just adds ch.qos.logback.* dependencies to Java LS. Java LS doesn't use logback.configuration.
However, we still need the ch.qos.logback.* and our new org.eclipse.jdt.ls.start plugins.

laeubi · 2022-05-31T05:10:37Z

Actually, the ch.qos.logback.classic plugin breaks our stdout. We can remove org.eclipse.m2e.logback.configuration. It just adds ch.qos.logback.* dependencies to Java LS. Java LS doesn't use it.

It should be possible to use the slf4j.noop binding then you don't need logback nor the logback.configuration.

snjeza · 2022-05-31T05:27:41Z

It should be possible to use the slf4j.noop binding then you don't need logback nor the logback.configuration.

It will not help. We face the issue when there are m2e.core and ch.qos.logback.classic plugins. The issue happens before starting our o.e.jdt.ls.core plugin. I have to introduce a new plugin that starts before all of them.

laeubi · 2022-05-31T05:49:28Z

when there are m2e.core and ch.qos.logback.classic plugins

But why include ch.qos.logback.classic at all?

snjeza · 2022-05-31T06:43:07Z

But why include ch.qos.logback.classic at all?

We want a logging.
See #1589. It works in m2e < 2.0.

laeubi · 2022-05-31T06:52:35Z

Sorry but this all sounds a bit contradictory to me... first you wrote:

The best solution would be to remove the ConsoleAdapter adapter from m2e's logback.xml

and i suggested to provide a custom fragment instead to make sure your custom config is always used ... but then

Actually, the ch.qos.logback.classic plugin breaks our stdout.

(while logback itself do not print to std out if not configured so) and

Java LS doesn't use logback.configuration

so i suggested to remove logback at all and now you claim it is required...

So again, why not provide an own config instead of reusing the (internal!) config from m2e at all?

It works in m2e < 2.0.

It do not work and has never worked, it was only a side-effect of start order that is seems to work, but as you see here breaks as soon as m2e changes something, or bundles are started in a different order.

So you can add another hack here or fix it in the first place by either:

do not use m2e logback config and provide your own as either a fragment or a commandline parameter
do not use logback but configure an alternative slf4j backend (e.g. one might use slf4josgi instead)

rgrunber · 2022-05-31T15:36:24Z

If we removed the logback.configuration bundle and all associated logic, and just left it up to client to pass in the configuration file (eg. -Dlogback.configurationFile=..., would that work ? Let's just stabilize this change first and maybe deal with logging options later.

snjeza · 2022-05-31T16:47:30Z

Sorry but this all sounds a bit contradictory to me... first you wrote:
The best solution would be to remove the ConsoleAdapter adapter from m2e's logback.xml

I was wrong. It is crossed out. Sorry.

and i suggested to provide a custom fragment instead to make sure your custom config is always used ... but then
Actually, the ch.qos.logback.classic plugin breaks our stdout.
(while logback itself do not print to std out if not configured so) and
Java LS doesn't use logback.configuration
so i suggested to remove logback at all and now you claim it is required...

Java LS doesn't use logback.configuration than only set -Dlogback.configurationFile. It requires the ch.qos.logback.* plugins.
We can remove all of them. It would work, but we would lose the logging.

So again, why not provide an own config instead of reusing the (internal!) config from m2e at all?

Agree.

It works in m2e < 2.0.
It do not work and has never worked, it was only a side-effect of start order that is seems to work, but as you see here breaks as soon as m2e changes something, or bundles are started in a different order.

Right.

snjeza · 2022-05-31T16:53:25Z

If we removed the logback.configuration bundle and all associated logic, and just left it up to client to pass in the configuration file (eg. -Dlogback.configurationFile=..., would that work

if we remove the logback.configuration and its dependencies (the ch.qos.logback.* plugins) it would work, but -Dlogback.configurationFile=... would not work.
if we remove the logback.configuration plugin and keep ch.qos.logback.*, -Dlogback.configurationFile=... would work.
However, VS Code would break if -Dlogback.configurationFile=... is not set or it shows an invalid file.

@rgrunber do you want me to remove the logback.configuration bundle? It will remove ch.qos.logback.* and disable -Dlogback.configurationFile=...

The current PR works in the following way:

it starts by default before the Java LS, m2.core and ch.qos.logback plugins
if set logback.configurationFile is not set, the plugin set it. We can define a default logback.xml
it starts the Java LS plugin

snjeza · 2022-05-31T20:04:03Z

@rgrunber I have updated the PR. The logback plugins have been removed.

snjeza · 2022-05-31T20:16:02Z

test this please

rgrunber

In the *.launch files, you just need to remove the 4 *logback* bundles and add org.apache.commons.codec to ensure the launch can be used.

The change is working well. We'll address the logback changes in a separate PR.

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

snjeza · 2022-05-31T21:00:00Z

In the *.launch files, you just need to remove the 4 logback bundles and add org.apache.commons.codec to ensure the launch can be used.

Fixed. I have also added org.apache.commons.codec to languageServer.product

rgrunber

The launcher works for me now. Everything else looks fine to me.

org.eclipse.m2e.logback.feature.feature.group is also still in the target platform, but for now we can keep it as it isn't a harmful change.

HannesWell

Sorry for being late, it looks like you already found a solution. Thank you Christoph for taking care.

Since you mentioned that you have to adjust requirements for your launch configuration you might be interested in the possibility to Include requirements automatically when launching an application or to Include requirements automatically when launching a product of the upcomming Eclipse 2022-06 release. If you are using the latest milestones/release-candidates you can use that already.

HannesWell · 2022-06-01T06:48:21Z

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/MavenProjectImporter.java

@@ -232,8 +234,14 @@ public void importToWorkspace(IProgressMonitor monitor) throws CoreException, Op
 	}

 	private long getLastWorkspaceStateModified() {
-		File workspaceStateFile = MavenPluginActivator.getDefault().getMavenProjectManager().getWorkspaceStateFile();
-		return workspaceStateFile.lastModified();
+		Bundle bundle = Platform.getBundle(IMavenConstants.PLUGIN_ID);


FrameworkUtil.getBundle(IMaven.class) should be a bit quicker to obtain the o.e.m2e.core bundle.

snjeza added the in progress label May 20, 2022

snjeza requested review from gorkem, fbricon, rgrunber, jdneo and testforstephen May 20, 2022 23:58

rgrunber requested changes May 26, 2022

View reviewed changes

snjeza force-pushed the issue-2085 branch from 224b06b to 1488ac5 Compare May 28, 2022 01:11

snjeza force-pushed the issue-2085 branch from 1488ac5 to ad75950 Compare May 28, 2022 04:04

rgrunber previously approved these changes May 30, 2022

View reviewed changes

rgrunber reviewed May 30, 2022

View reviewed changes

org.eclipse.jdt.ls.core/logback.xml Show resolved Hide resolved

snjeza changed the title ~~Update M2E to 2.0.0 (Vulnerability in Apache Lucene 8.9)~~ [WIP] Update M2E to 2.0.0 (Vulnerability in Apache Lucene 8.9) May 31, 2022

snjeza force-pushed the issue-2085 branch from ad75950 to 269e58f Compare May 31, 2022 04:21

snjeza changed the title ~~[WIP] Update M2E to 2.0.0 (Vulnerability in Apache Lucene 8.9)~~ Update M2E to 2.0.0 (Vulnerability in Apache Lucene 8.9) May 31, 2022

snjeza force-pushed the issue-2085 branch from 269e58f to 8f2401d Compare May 31, 2022 20:02

snjeza mentioned this pull request May 31, 2022

Add logback tracing to Java LS #2108

Closed

rgrunber reviewed May 31, 2022

View reviewed changes

Update M2E to 2.0.0 (Vulnerability in Apache Lucene 8.9)

8b66318

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>

snjeza force-pushed the issue-2085 branch from 8f2401d to 8b66318 Compare May 31, 2022 20:58

rgrunber approved these changes Jun 1, 2022

View reviewed changes

rgrunber merged commit 6c32243 into eclipse-jdtls:master Jun 1, 2022

HannesWell reviewed Jun 1, 2022

View reviewed changes

snjeza mentioned this pull request Jun 6, 2022

Add logback tracing to Java LS #2116

Merged

rgrunber mentioned this pull request Jan 29, 2024

Eclipse #3032

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update M2E to 2.0.0 (Vulnerability in Apache Lucene 8.9) #2096

Update M2E to 2.0.0 (Vulnerability in Apache Lucene 8.9) #2096

snjeza commented May 20, 2022

snjeza commented May 21, 2022

rgrunber left a comment •

edited

Loading

testforstephen commented May 27, 2022

Eskibear commented May 27, 2022 •

edited

Loading

snjeza commented May 28, 2022

snjeza commented May 28, 2022

Eskibear commented May 30, 2022

rgrunber left a comment

rgrunber commented May 30, 2022 •

edited

Loading

snjeza commented May 30, 2022

snjeza commented May 30, 2022 •

edited

Loading

laeubi commented May 31, 2022

laeubi commented May 31, 2022

snjeza commented May 31, 2022

laeubi commented May 31, 2022

snjeza commented May 31, 2022 •

edited

Loading

laeubi commented May 31, 2022

snjeza commented May 31, 2022

laeubi commented May 31, 2022

snjeza commented May 31, 2022

laeubi commented May 31, 2022

rgrunber commented May 31, 2022

snjeza commented May 31, 2022

snjeza commented May 31, 2022

snjeza commented May 31, 2022

snjeza commented May 31, 2022

rgrunber left a comment

snjeza commented May 31, 2022

rgrunber left a comment •

edited

Loading

HannesWell left a comment

HannesWell Jun 1, 2022

Update M2E to 2.0.0 (Vulnerability in Apache Lucene 8.9) #2096

Update M2E to 2.0.0 (Vulnerability in Apache Lucene 8.9) #2096

Conversation

snjeza commented May 20, 2022

snjeza commented May 21, 2022

rgrunber left a comment • edited Loading

Choose a reason for hiding this comment

testforstephen commented May 27, 2022

Eskibear commented May 27, 2022 • edited Loading

snjeza commented May 28, 2022

snjeza commented May 28, 2022

Eskibear commented May 30, 2022

rgrunber left a comment

Choose a reason for hiding this comment

rgrunber commented May 30, 2022 • edited Loading

snjeza commented May 30, 2022

snjeza commented May 30, 2022 • edited Loading

laeubi commented May 31, 2022

laeubi commented May 31, 2022

snjeza commented May 31, 2022

laeubi commented May 31, 2022

snjeza commented May 31, 2022 • edited Loading

laeubi commented May 31, 2022

snjeza commented May 31, 2022

laeubi commented May 31, 2022

snjeza commented May 31, 2022

laeubi commented May 31, 2022

rgrunber commented May 31, 2022

snjeza commented May 31, 2022

snjeza commented May 31, 2022

snjeza commented May 31, 2022

snjeza commented May 31, 2022

rgrunber left a comment

Choose a reason for hiding this comment

snjeza commented May 31, 2022

rgrunber left a comment • edited Loading

Choose a reason for hiding this comment

HannesWell left a comment

Choose a reason for hiding this comment

HannesWell Jun 1, 2022

Choose a reason for hiding this comment

rgrunber left a comment •

edited

Loading

Eskibear commented May 27, 2022 •

edited

Loading

rgrunber commented May 30, 2022 •

edited

Loading

snjeza commented May 30, 2022 •

edited

Loading

snjeza commented May 31, 2022 •

edited

Loading

rgrunber left a comment •

edited

Loading