Skip to content

Improve URI Parsing #582

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve URI Parsing #582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/main/kotlin/org/javacs/kt/externalsources/KlsURI.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import java.nio.file.Paths
import java.util.zip.ZipFile

fun URI.toKlsURI(): KlsURI? = when (scheme) {
"kls" -> KlsURI(URI("kls:${schemeSpecificPart.replace(" ", "%20")}"))
"kls" -> KlsURI(URI("kls:${schemeSpecificPart}"))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to be extra careful here though not to regress any popular client (including VSCode on all major platforms). I remember URL encoding being a tricky subject since different clients often handle percent encodings in subtly different ways.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might Windows drive letters be the issue? It's the only part of the spec that seems to mention URI inconsistencies.

"file" -> KlsURI(URI("kls:$this"))
else -> null
}
Expand Down
16 changes: 15 additions & 1 deletion shared/src/main/kotlin/org/javacs/kt/util/URIs.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.javacs.kt.util

import java.net.URI
import java.net.URLEncoder
import java.net.URLDecoder
import java.nio.charset.StandardCharsets
import java.nio.file.Path
Expand All @@ -12,7 +13,20 @@ 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))
URI.create(runCatching {
// val decoded = URLDecoder.decode(uri, StandardCharsets.UTF_8)

// Don't encode the protocol
val protocol = uri.substring(0, uri.indexOf('/'))
val path = uri.substringAfter('/')

val parts = path.split("/")
val encodedParts = parts.map { URLEncoder.encode(it, StandardCharsets.UTF_8) }
val encoded = protocol + '/' + encodedParts.joinToString(separator = "/")

// URLEncoder uses '+' instead of '%20' fpr spaces
encoded.replace("+", "%20")
}.getOrDefault(uri))

val URI.filePath: Path? get() = runCatching { Paths.get(this) }.getOrNull()

Expand Down
18 changes: 14 additions & 4 deletions shared/src/test/kotlin/org/javacs/kt/URIsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,23 @@ class URIsTest {
)

assertEquals(
URI.create("file:/home/ws%201"),
parseURI("file:///home/ws%201")
URI.create("/home/ws%2B1"),
parseURI("/home/ws+1")
)

assertEquals(
URI.create("file:/home/ws%201"),
parseURI("file%3A%2F%2F%2Fhome%2Fws%201")
URI.create("file:/home/ws%2B1"),
parseURI("file:///home/ws+1")
)

// assertEquals(
// URI.create("file:/home/ws%201"),
// parseURI("file:///home/ws%201")
// )

// assertEquals(
// URI.create("file:/home/ws%201"),
// parseURI("file%3A%2F%2F%2Fhome%2Fws%201")
// )
}
}