Skip to content
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

Language server crashed when specific unicode characters included path given #317

Open
Nyaacinth opened this issue Oct 31, 2021 · 5 comments
Labels
bug Something isn't working

Comments

@Nyaacinth
Copy link

Related:
fwcd/vscode-kotlin#62
https://www.mail-archive.com/[email protected]/msg99872.html

A workspace path includes specific unicode characters will cause the language server crashed. Launching a VM with Shift-JIS locale, the error disappeared.

Output:

Server initialization failed.
  Message: Internal error.
  Code: -32603 
java.util.concurrent.CompletionException: java.lang.IllegalArgumentException: Bad escape
	at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:314)
	at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:319)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1702)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: java.lang.IllegalArgumentException: Bad escape
	at java.base/sun.nio.fs.UnixUriUtils.fromUri(UnixUriUtils.java:87)
	at java.base/sun.nio.fs.UnixFileSystemProvider.getPath(UnixFileSystemProvider.java:103)
	at java.base/java.nio.file.Path.of(Path.java:203)
	at java.base/java.nio.file.Paths.get(Paths.java:97)
	at org.javacs.kt.KotlinLanguageServer$initialize$1.invoke(KotlinLanguageServer.kt:112)
	at org.javacs.kt.KotlinLanguageServer$initialize$1.invoke(KotlinLanguageServer.kt:71)
	at org.javacs.kt.util.AsyncExecutor.compute$lambda-2(AsyncExecutor.kt:19)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1700)
	... 3 more

Starting client failed
  Message: Internal error.
  Code: -32603 
java.util.concurrent.CompletionException: java.lang.IllegalArgumentException: Bad escape
	at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(CompletableFuture.java:314)
	at java.base/java.util.concurrent.CompletableFuture.completeThrowable(CompletableFuture.java:319)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1702)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: java.lang.IllegalArgumentException: Bad escape
	at java.base/sun.nio.fs.UnixUriUtils.fromUri(UnixUriUtils.java:87)
	at java.base/sun.nio.fs.UnixFileSystemProvider.getPath(UnixFileSystemProvider.java:103)
	at java.base/java.nio.file.Path.of(Path.java:203)
	at java.base/java.nio.file.Paths.get(Paths.java:97)
	at org.javacs.kt.KotlinLanguageServer$initialize$1.invoke(KotlinLanguageServer.kt:112)
	at org.javacs.kt.KotlinLanguageServer$initialize$1.invoke(KotlinLanguageServer.kt:71)
	at org.javacs.kt.util.AsyncExecutor.compute$lambda-2(AsyncExecutor.kt:19)
	at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1700)
	... 3 more
@fwcd
Copy link
Owner

fwcd commented Oct 31, 2021

Thanks for the bug report! Not the first issue with special characters in paths (see e.g. #200, #204) , the URI-path conversions unfortunately are still a bit wonky when it comes to unusual characters.

@Pikrass
Copy link

Pikrass commented Feb 5, 2024

I'm running into this too. Seems related: https://bugs.openjdk.org/browse/JDK-8162518
I found another project that ran into this: spring-projects/sts4#950

@Pikrass
Copy link

Pikrass commented Feb 5, 2024

I fixed it on my end.
I used the original solution proposed in #204, the one using a new dependency on org.eclipse.core.runtime. It turns out that spaces are not the only characters you need to handle: any non US-ASCII character needs to be encoded for the URI to be correct. That's why Paths.get failed.

Even after fixing parseURI there's still non-encoded URIs lying around. My solution is probably not the best but I didn't want to spend even more time investigating: I compare file paths instead of URIs in a couple of places.

With this the server doesn't crash, and completion and diagnostics seem to work with vim using coc. There's still more work needed obviously (tests, etc).

diff --git i/server/src/main/kotlin/org/javacs/kt/KotlinTextDocumentService.kt w/server/src/main/kotlin/org/javacs/kt/KotlinTextDocumentService.kt
index cbacca5..bc376c8 100644
--- i/server/src/main/kotlin/org/javacs/kt/KotlinTextDocumentService.kt
+++ w/server/src/main/kotlin/org/javacs/kt/KotlinTextDocumentService.kt
@@ -323,7 +323,7 @@ class KotlinTextDocumentService(
             else LOG.info("Ignore {} diagnostics in {} because it's not open", diagnostics.size, describeURI(uri))
         }
 
-        val noErrors = compiled - byFile.keys
+        val noErrors = compiled.filter { cu -> !byFile.keys.any { du -> cu.filePath == du.filePath} }
         for (file in noErrors) {
             clearDiagnostics(file)
 
diff --git i/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt w/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt
index d7bb849..1875bb1 100644
--- i/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt
+++ w/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt
@@ -193,7 +193,7 @@ class SourceFiles(
         LOG.info("Updated exclusions: ${exclusions.excludedPatterns}")
     }
 
-    fun isOpen(uri: URI): Boolean = (uri in open)
+    fun isOpen(uri: URI): Boolean = (open.any { it.filePath == uri.filePath })
 
     fun isIncluded(uri: URI): Boolean = exclusions.isURIIncluded(uri)
 }
diff --git i/shared/build.gradle.kts w/shared/build.gradle.kts
index 13f8156..7875812 100644
--- i/shared/build.gradle.kts
+++ w/shared/build.gradle.kts
@@ -17,6 +17,7 @@ dependencies {
     implementation(kotlin("stdlib"))
     implementation("org.jetbrains.exposed:exposed-core")
     implementation("org.jetbrains.exposed:exposed-dao")
+    implementation("org.eclipse.core.runtime:compatibility:3.1.200-v20070502")
     testImplementation("org.hamcrest:hamcrest-all")
     testImplementation("junit:junit")
 }
diff --git i/shared/src/main/kotlin/org/javacs/kt/util/URIs.kt w/shared/src/main/kotlin/org/javacs/kt/util/URIs.kt
index 2a0aac6..96e2d9f 100644
--- i/shared/src/main/kotlin/org/javacs/kt/util/URIs.kt
+++ w/shared/src/main/kotlin/org/javacs/kt/util/URIs.kt
@@ -5,6 +5,7 @@ import java.net.URLDecoder
 import java.nio.charset.StandardCharsets
 import java.nio.file.Path
 import java.nio.file.Paths
+import org.eclipse.core.runtime.URIUtil
 
 /**
  * Parse a possibly-percent-encoded URI string.
@@ -12,7 +13,7 @@ import java.nio.file.Paths
  * (including VSCode) invalidly percent-encode colons.
  */
 fun parseURI(uri: String): URI =
-    URI.create(runCatching { URLDecoder.decode(uri, StandardCharsets.UTF_8.toString()).replace(" ", "%20") }.getOrDefault(uri))
+    URIUtil.fromString(runCatching { URLDecoder.decode(uri, StandardCharsets.UTF_8.toString()) }.getOrDefault(uri))
 
 val URI.filePath: Path? get() = runCatching { Paths.get(this) }.getOrNull()

@Pikrass
Copy link

Pikrass commented Feb 6, 2024

Well, I had to dig more. After fixing the above mentioned issue I noticed a lot of overloading errors. It turns out my source file was loaded twice, because the URIs didn't match.
The culprit was the number of slashes for file URIs ("file:/path/to/file" vs "file:///path/to/file", so a null authority vs an empty authority). It even seems like the "Bad escape" error was linked to this, because modifying parseURI to restore triple slashes reintroduced it. (Edit: now that I think about it and it's not 4am, I probably did something wrong there and reintroduced non-encoded non-ascii chars in the process)

The patch below solves the duplicate source path error, but it leads to more tests failing.
Tests on main branch: 144 failed
Tests with the patch above: 21 failed
Tests with this new patch on top of the other: 27 failed

diff --git a/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt b/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt
index 1875bb1..8b89fa6 100644
--- a/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt
+++ b/server/src/main/kotlin/org/javacs/kt/SourceFiles.kt
@@ -1,5 +1,6 @@
 package org.javacs.kt
 
+import org.javacs.kt.util.toUri
 import com.intellij.openapi.util.text.StringUtil.convertLineSeparators
 import com.intellij.lang.java.JavaLanguage
 import com.intellij.lang.Language
@@ -184,7 +185,7 @@ class SourceFiles(
         return SourceExclusions(listOf(root), scriptsConfig)
             .walkIncluded()
             .filter { sourceMatcher.matches(it.fileName) }
-            .map(Path::toUri)
+            .map{toUri(it)}
             .toSet()
     }
 
diff --git a/shared/src/main/kotlin/org/javacs/kt/util/URIs.kt b/shared/src/main/kotlin/org/javacs/kt/util/URIs.kt
index 96e2d9f..a0939b7 100644
--- a/shared/src/main/kotlin/org/javacs/kt/util/URIs.kt
+++ b/shared/src/main/kotlin/org/javacs/kt/util/URIs.kt
@@ -15,6 +15,8 @@ import org.eclipse.core.runtime.URIUtil
 fun parseURI(uri: String): URI =
     URIUtil.fromString(runCatching { URLDecoder.decode(uri, StandardCharsets.UTF_8.toString()) }.getOrDefault(uri))
 
+fun toUri(path: Path): URI = URI("file", path.toString(), null)
+
 val URI.filePath: Path? get() = runCatching { Paths.get(this) }.getOrNull()
 
 /** Fetches the file extension WITHOUT the dot. */

@jlzht
Copy link

jlzht commented Mar 15, 2024

any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants