Fix jar uri conversion (#233)

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.

Author: milesziemer

src/main/java/software/amazon/smithy/lsp/SmithyLanguageServer.java

@@ -369,7 +369,7 @@ public CompletableFuture<String> 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)));
         }
     }
 

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;

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);
         }
     }
-
 }

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));
+    }
+}