kotlin-language-server icon indicating copy to clipboard operation
kotlin-language-server copied to clipboard

Language server crashed when specific unicode characters included path given

Open Nyaacinth opened this issue 2 years ago • 5 comments

Related: https://github.com/fwcd/vscode-kotlin/issues/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

Nyaacinth avatar Oct 31 '21 06:10 Nyaacinth

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.

fwcd avatar Oct 31 '21 23:10 fwcd

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

Pikrass avatar Feb 05 '24 17:02 Pikrass

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 avatar Feb 05 '24 21:02 Pikrass

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. */

Pikrass avatar Feb 06 '24 03:02 Pikrass

any update on this?

jlzht avatar Mar 15 '24 17:03 jlzht