-
Notifications
You must be signed in to change notification settings - Fork 93
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
Request to bump LSP4J version 0.20.1 to 0.21.0 #1546
Comments
@cherylking there is a PR for that at #1529 and I suggest you that you read comments and add your comments. The main problem with lsp4j 0.21.0 is that it requires to change java version and it is very annoying for vscode xml users even if we have binary. More I dont see the value to use this last version of lsp4j about features. It will be difficult to explain to vscode users that it requires new version of java altough it doesnt provide new features |
@angelozerr Thanks for the pointer. We are already requiring Java 17 for all of our IDE tools. So I don't think that will be a problem for us, but I will keep an eye on that PR and the various comments. |
@angelozerr It looks like the minimum Java has been bumped to Java 11 in later releases. Can this LSP4J version now be bumped from 0.20.1 to 0.21.0? The CVE related to guava is causing us issues with our security processes internally. Actually, I was looking at this draft PR and thought it was something that had already gone in. Is the minimum Java still 8 then? |
@cherylking the main problem is that we use lemminx in vscode-xml with binary (we build lemminx with graalvm):
|
@cherylking From my understanding, we do not ship the vulnerable version of Guava. We use a Maven exclusion to remove the version specified by lsp4j, and instead specify Guava 32.0.1-jre to be used, which is not vulnerable. As a side note, we cannot drop Guava as a dependency, as we use it in the lemminx codebase. @angelozerr We are building the binary with a version of GraalVM that uses Java 21, so the first point shouldn't be a problem I think it's feasible to update to Java 11 and the latest lsp4j, but it will take a bit of effort to do it. Updating to the latest LSP4J introduces only very minor API changes that are easily resolved. The main challenge with updating to the new version is, as Angelo mentioned, making sure that the binary version of lemminx still works. We need to make sure that we have all classes that will be used with reflection listed in https://github.com/eclipse-lemminx/lemminx/blob/main/org.eclipse.lemminx/src/main/resources/META-INF/native-image/reflect-config.json, otherwise there will be runtime errors. We need to do manual testing to figure out if we are missing any entries, since our test suite can't detect this. |
@datho7561 Thank you for pointing this out. I was not aware that exclusion had been added. |
The
0.21.0
version removes a dependency onorg.eclipse.xtend.lib
which thereby removes a transitive dependency onguava
. There's a new CVE related toguava
which is why I am interested in this getting changed.The text was updated successfully, but these errors were encountered: