From c8d05418a7d0b9f13702110df1096ed0fef196d4 Mon Sep 17 00:00:00 2001 From: Miles Ziemer Date: Sun, 20 Apr 2025 16:25:28 -0400 Subject: [PATCH] Fix jar uri conversion The language server has to convert to and from LSP's URIs and the Smithy model's source location filenames. The filenames used for files in jars are actually URIs in the form `jar:file/foo.jar!/bar.smithy` - obviously there isn't an actual file path to a file within a jar. Because LSP's URIs and these Jar URIs are URIs, they're percent-encoded. When we convert from a regular file URI -> filename, `Path.of(URI).toString()` makes sure the filename ends up properly decoded, regardless of how the client encoded the URI. However, when going from jar file URI -> jar file filename (as it appears in the model), we don't want to decode the URI (because the filename is encoded in the model). This should be easy, but since some clients encode the URI differently, the URI sent by the client might not be encoded the same way it is in the model filename. In particular, VSCode is quite aggresive in its encoding, and encodes the `!` in the jar URI. To handle this, we were decoding the LSP URI, but if the URI has other special characters, like spaces, those would also be decoded. I'm pretty sure this would always be an issue on windows too, since the `:` in `C:` would be encoded. The problem hasn't come up yet, because who puts special characters in file/directory names? However, when trying to autodownload our new standalone installations in the VSCode extension, I found that extensions' storage directories are under `/Application Support/` on Mac. So we need to fix this in order to autodownload the language server. To fix this, I updated the implementation of a few methods in LspAdapter. Most notable is the new `smithyJarUriToJarModelFilename` method which takes an LSP jar URI and turns it into a Java URI that can properly `toString()` into the model filename, or `toURL()` into a URL that can be used to read the contents of the file. The method has a comment that explains how it works - it's really a hack. We really shouldn't be using strings to represent URIs and paths at all, but at some point we still need to go back and forth between LSP URI and model filename, so I'm not sure if that would fix the problem, or just move it. --- .../smithy/lsp/SmithyLanguageServer.java | 2 +- .../smithy/lsp/project/ProjectLoader.java | 2 +- .../smithy/lsp/protocol/LspAdapter.java | 94 +++++++++++++------ .../smithy/lsp/protocol/LspAdapterTest.java | 47 ++++++++++ 4 files changed, 113 insertions(+), 32 deletions(-) create mode 100644 src/test/java/software/amazon/smithy/lsp/protocol/LspAdapterTest.java diff --git a/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java b/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java index 5d2f0247..f87210bf 100644 --- a/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java +++ b/src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java @@ -369,7 +369,7 @@ public CompletableFuture jarFileContents(TextDocumentIdentifier textDocu return completedFuture(projectAndFile.file().document().copyText()); } else { // Technically this can throw if the uri is invalid - return completedFuture(IoUtils.readUtf8Url(LspAdapter.jarUrl(uri))); + return completedFuture(IoUtils.readUtf8Url(LspAdapter.smithyJarUriToReadableUrl(uri))); } } diff --git a/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java b/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java index 1c5025a0..9c948694 100644 --- a/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java +++ b/src/main/java/software/amazon/smithy/lsp/project/ProjectLoader.java @@ -211,7 +211,7 @@ private static void findOrReadDocument( // the model stores jar paths as URIs if (LspAdapter.isSmithyJarFile(filePath) || LspAdapter.isJarFile(filePath)) { // Technically this can throw - String text = IoUtils.readUtf8Url(LspAdapter.jarUrl(filePath)); + String text = IoUtils.readUtf8Url(LspAdapter.jarModelFilenameToReadableUrl(filePath)); Document document = Document.of(text); consumer.accept(filePath, text, document); return; diff --git a/src/main/java/software/amazon/smithy/lsp/protocol/LspAdapter.java b/src/main/java/software/amazon/smithy/lsp/protocol/LspAdapter.java index 41fca697..1b9188ac 100644 --- a/src/main/java/software/amazon/smithy/lsp/protocol/LspAdapter.java +++ b/src/main/java/software/amazon/smithy/lsp/protocol/LspAdapter.java @@ -5,13 +5,11 @@ package software.amazon.smithy.lsp.protocol; -import java.io.IOException; +import java.net.MalformedURLException; import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; import java.nio.file.Paths; -import java.util.logging.Logger; import org.eclipse.lsp4j.Location; import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; @@ -28,8 +26,6 @@ * 'smithyjar:' scheme we use. */ public final class LspAdapter { - private static final Logger LOGGER = Logger.getLogger(LspAdapter.class.getName()); - private LspAdapter() { } @@ -186,11 +182,11 @@ public static SourceLocation toSourceLocation(String path, Position position) { * @return A path representation of the {@code uri}, with the scheme removed */ public static String toPath(String uri) { - if (uri.startsWith("file://")) { + if (uri.startsWith("file:")) { return Paths.get(URI.create(uri)).toString(); } else if (isSmithyJarFile(uri)) { - String decoded = decode(uri); - return fixJarScheme(decoded); + URI jarUri = smithyJarUriToJarModelFilename(uri); + return jarUri.toString(); } return uri; } @@ -231,36 +227,74 @@ public static boolean isJarFile(String uri) { } /** - * Get a {@link URL} for the Jar represented by the given URI or path. + * Get a {@link URL} for the Jar represented by the given smithyjar URI. * - * @param uriOrPath LSP URI or regular path - * @return The {@link URL}, or throw if the uri/path cannot be decoded + * @param uri The smithyjar URI + * @return A URL which can be used to read the contents of the file */ - public static URL jarUrl(String uriOrPath) { + public static URL smithyJarUriToReadableUrl(String uri) { try { - String decodedUri = decode(uriOrPath); - return URI.create(fixJarScheme(decodedUri)).toURL(); - } catch (IOException e) { + URI jarUri = smithyJarUriToJarModelFilename(uri); + return jarUri.toURL(); + } catch (MalformedURLException e) { throw new RuntimeException(e); } } - private static String decode(String uriOrPath) { - // Some clients encode parts of the jar, like !/ - return URLDecoder.decode(uriOrPath, StandardCharsets.UTF_8); + /** + * Get a {@link URL} for the jar file from the filename in the model's + * source location. + * + * @param modelFilename The filename from the model's source location + * @return A URL which can be used to read the contents of the file + */ + public static URL jarModelFilenameToReadableUrl(String modelFilename) { + try { + // No need to decode these, they're already encoded + return URI.create(modelFilename).toURL(); + } catch (MalformedURLException e) { + throw new RuntimeException(e); + } } - private static String fixJarScheme(String uriOrPath) { - if (uriOrPath.startsWith("smithyjar:")) { - uriOrPath = uriOrPath.replaceFirst("smithyjar:", ""); - } - if (uriOrPath.startsWith("jar:")) { - return uriOrPath; - } else if (uriOrPath.startsWith("file:")) { - return "jar:" + uriOrPath; - } else { - return "jar:file:" + uriOrPath; + /** + * Converts a smithyjar uri that was sent from the client into a URI that + * is equivalent to what appears in the Smithy model. + * + * @param smithyJarUri smithyjar uri received from the client + * @return The converted URI + */ + private static URI smithyJarUriToJarModelFilename(String smithyJarUri) { + // Clients encode URIs differently. VSCode is particularly aggressive with + // its encoding, so the URIs it produces aren't equivalent to what we get + // from source locations in the model. + // + // For example, given a jar that lives in some directory with special characters: + // /path with spaces/foo.jar + // The model will have a source location with a filename like: + // jar:file:/path%20with%20spaces/foo.jar!/baz.smithy + // When sending requests/notifications for this file, VSCode will encode the URI like: + // smithyjar:/path%20with%20spaces/foo.jar%21/baz.smithy + // Note the ! is encoded. + // + // If we just used URI.create().toString(), we will end up with the exact same + // URI that VSCode sent, because URI.create() (and its equivalent ctor) keep + // the original input string to use for the toString() call. + // + // Instead, we use getSchemeSpecificPart() to fully decode everything after the + // smithyjar: part, to get: + // /path with spaces/foo.jar!/baz.smithy + // Then, we reconstruct the URI from parts, using a different ctor that performs + // encoding. The resulting URI.toString() call will give us what we want: + // jar:file:/path%20with%20spaces/foo.jar!/baz.smithy + + URI encodedUri = URI.create(smithyJarUri); + String decodedPath = encodedUri.getSchemeSpecificPart(); + + try { + return new URI("jar", "file:" + decodedPath, null); + } catch (URISyntaxException e) { + throw new RuntimeException(e); } } - } diff --git a/src/test/java/software/amazon/smithy/lsp/protocol/LspAdapterTest.java b/src/test/java/software/amazon/smithy/lsp/protocol/LspAdapterTest.java new file mode 100644 index 00000000..9358edbb --- /dev/null +++ b/src/test/java/software/amazon/smithy/lsp/protocol/LspAdapterTest.java @@ -0,0 +1,47 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +package software.amazon.smithy.lsp.protocol; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +import org.junit.jupiter.api.Test; + +public class LspAdapterTest { + @Test + public void jarModelFilenameRoundTrip() { + String jarModelFilename = "jar:file:/path%20with%20spaces/foo.jar!/bar.smithy"; + String jarUri = LspAdapter.toUri(jarModelFilename); + + assertThat(jarUri, equalTo("smithyjar:/path%20with%20spaces/foo.jar!/bar.smithy")); + assertThat(LspAdapter.toPath(jarUri), equalTo(jarModelFilename)); + } + + @Test + public void smithyjarRoundTrip() { + String jarUri = "smithyjar:/path%20with%20spaces/foo.jar!/bar.smithy"; + String jarModelFilename = LspAdapter.toPath(jarUri); + + assertThat(jarModelFilename, equalTo("jar:file:/path%20with%20spaces/foo.jar!/bar.smithy")); + assertThat(LspAdapter.toUri(jarModelFilename), equalTo(jarUri)); + } + + @Test + public void aggressivelyEncodedSmithyjarRoundTrip() { + String encodedJarUri = "smithyjar:/path%20with%20spaces/foo.jar%21/bar.smithy"; + String jarModelFilename = LspAdapter.toPath(encodedJarUri); + + assertThat(jarModelFilename, equalTo("jar:file:/path%20with%20spaces/foo.jar!/bar.smithy")); + assertThat(LspAdapter.toUri(jarModelFilename), equalTo("smithyjar:/path%20with%20spaces/foo.jar!/bar.smithy")); + } + + @Test + public void aggressivelyEncodedSmithyJarToUrl() { + String encodedJarUri = "smithyjar:/path%20with%20spaces/foo.jar%21/bar.smithy"; + + assertDoesNotThrow(() -> LspAdapter.smithyJarUriToReadableUrl(encodedJarUri)); + } +}