diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/PluginContext.java b/smithy-build/src/main/java/software/amazon/smithy/build/PluginContext.java index 0497197692e..bff5131d095 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/PluginContext.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/PluginContext.java @@ -5,6 +5,7 @@ package software.amazon.smithy.build; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -37,6 +38,7 @@ public final class PluginContext implements ToSmithyBuilder { private final ClassLoader pluginClassLoader; private final Set sources; private final String artifactName; + private final List sourceUriPaths; private Model nonTraitsModel; private PluginContext(Builder builder) { @@ -50,6 +52,15 @@ private PluginContext(Builder builder) { settings = builder.settings; pluginClassLoader = builder.pluginClassLoader; sources = builder.sources.copy(); + // Normalize the source paths into URI paths, which are URI encoded and use '/' separators, + // which is the format of paths in 'jar:file:/' filenames, so they can be compared in the + // 'isSource*' methods. + // This is done preemptively so we don't have to do the same work repeatedly, and the number + // of configured sources is typically very low. + sourceUriPaths = new ArrayList<>(sources.size()); + for (Path source : sources) { + sourceUriPaths.add(source.toUri().getRawPath()); + } } /** @@ -223,6 +234,18 @@ private boolean isSource(FromSourceLocation sourceLocation) { String location = sourceLocation.getSourceLocation().getFilename(); int offsetFromStart = findOffsetFromStart(location); + if (offsetFromStart > 0) { + // Offset means the filename is a URI, so compare to the URI paths to account for encoding and windows. + for (String sourceUriPath : sourceUriPaths) { + if (location.regionMatches(offsetFromStart, sourceUriPath, 0, sourceUriPath.length())) { + return true; + } + } + + return false; + } + + // Filename is a regular path, so we can compare to sources directly. for (Path path : sources) { String pathString = path.toString(); int offsetFromStartInSource = findOffsetFromStart(pathString); diff --git a/smithy-build/src/test/java/software/amazon/smithy/build/plugins/SourcesPluginTest.java b/smithy-build/src/test/java/software/amazon/smithy/build/plugins/SourcesPluginTest.java index 36a0eb8efc4..2b97d42071d 100644 --- a/smithy-build/src/test/java/software/amazon/smithy/build/plugins/SourcesPluginTest.java +++ b/smithy-build/src/test/java/software/amazon/smithy/build/plugins/SourcesPluginTest.java @@ -10,9 +10,16 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import java.io.File; +import java.io.IOException; import java.net.URISyntaxException; +import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Comparator; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import software.amazon.smithy.build.MockManifest; import software.amazon.smithy.build.PluginContext; @@ -26,6 +33,19 @@ import software.amazon.smithy.utils.ListUtils; public class SourcesPluginTest { + + private Path tempDirectory; + + @BeforeEach + public void before() throws IOException { + tempDirectory = Files.createTempDirectory(getClass().getSimpleName()); + } + + @AfterEach + public void after() throws IOException { + Files.walk(tempDirectory).sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete); + } + @Test public void copiesFilesForSourceProjection() throws URISyntaxException { Model model = Model.assembler() @@ -236,4 +256,69 @@ public void omitsUnsupportedFilesFromManifest() throws URISyntaxException { assertThat(manifestString, containsString("a.smithy\n")); assertThat(manifestString, not(containsString("foo.md"))); } + + @Test + public void copiesModelFromJarWithEncodedPathWithSourceProjection() + throws IOException, URISyntaxException { + Path source = Paths.get(getClass().getResource("sources/jar-import.jar").toURI()); + Path jarPath = tempDirectory.resolve(Paths.get("special%45path", "special.jar")); + + Files.createDirectories(jarPath.getParent()); + Files.copy(source, jarPath); + + Model model = Model.assembler() + .addImport(jarPath) + .addImport(getClass().getResource("notsources/d.smithy")) + .assemble() + .unwrap(); + MockManifest manifest = new MockManifest(); + PluginContext context = PluginContext.builder() + .fileManifest(manifest) + .model(model) + .originalModel(model) + .sources(ListUtils.of(jarPath)) + .build(); + new SourcesPlugin().execute(context); + String manifestString = manifest.getFileString("manifest").get(); + + assertThat(manifestString, containsString("special/a.smithy\n")); + assertThat(manifestString, containsString("special/b/b.smithy\n")); + assertThat(manifestString, containsString("special/b/c/c.json\n")); + assertThat(manifestString, not(containsString("jar-import/d.json"))); + assertThat(manifest.getFileString("special/a.smithy").get(), containsString("string A")); + assertThat(manifest.getFileString("special/b/b.smithy").get(), containsString("string B")); + assertThat(manifest.getFileString("special/b/c/c.json").get(), containsString("\"foo.baz#C\"")); + } + + @Test + public void copiesModelFromJarWithEncodedPathWithNonSourceProjection() + throws IOException, URISyntaxException { + Path source = Paths.get(getClass().getResource("sources/jar-import.jar").toURI()); + Path jarPath = tempDirectory.resolve(Paths.get("special%45path", "special.jar")); + + Files.createDirectories(jarPath.getParent()); + Files.copy(source, jarPath); + + Model model = Model.assembler() + .addImport(jarPath) + .assemble() + .unwrap(); + MockManifest manifest = new MockManifest(); + ProjectionConfig projection = ProjectionConfig.builder().build(); + PluginContext context = PluginContext.builder() + .fileManifest(manifest) + .projection("foo", projection) + .model(model) + .originalModel(model) + .sources(ListUtils.of(jarPath)) + .build(); + new SourcesPlugin().execute(context); + String manifestString = manifest.getFileString("manifest").get(); + + assertThat(manifestString, containsString("model.json")); + assertThat(manifestString, not(containsString("special"))); + assertThat(manifest.getFileString("model.json").get(), containsString("\"foo.baz#A\"")); + assertThat(manifest.getFileString("model.json").get(), containsString("\"foo.baz#B\"")); + assertThat(manifest.getFileString("model.json").get(), containsString("\"foo.baz#C\"")); + } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelDiscovery.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelDiscovery.java index f6d7016ea02..934c2b977ff 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelDiscovery.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelDiscovery.java @@ -10,9 +10,12 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.net.URLConnection; import java.nio.charset.StandardCharsets; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Enumeration; import java.util.LinkedHashSet; @@ -185,19 +188,30 @@ public static String getSmithyModelPathFromJarUrl(URL modelUrl) { */ public static URL createSmithyJarManifestUrl(String fileOrUrl) { try { - return new URL(getFilenameWithScheme(fileOrUrl) + "!/" + MANIFEST_PATH); - } catch (IOException e) { + // This normalizes the path to the jar into the format expected by URI, + // accounting for the different string representation of a path on windows + // (otherwise all '\' path separators will be encoded). + URI jarUri = Paths.get(removeScheme(fileOrUrl)).toUri(); + String manifestPath = jarUri.getPath() + "!/" + MANIFEST_PATH; + + // Building a URI from parts (scheme + file path) will make sure the file path + // gets properly URI encoded. URL does not do this, instead expecting callers + // to encode first, and consumers to decode. Encoding it here means that when + // it is decoded by the consumer (e.g. the JDK reading the jar from the file system), + // it will point to the correct location. + return new URI("jar:file", null, manifestPath, null).toURL(); + } catch (IOException | URISyntaxException e) { throw new ModelImportException(e.getMessage(), e); } } - private static String getFilenameWithScheme(String filename) { - if (filename.startsWith("jar:")) { - return filename; - } else if (filename.startsWith("file:")) { - return "jar:" + filename; + private static String removeScheme(String fileOrUrl) { + // Index will also cover jar: part of jar:file: + int pathStart = fileOrUrl.indexOf("file:"); + if (pathStart == -1) { + return fileOrUrl; } else { - return "jar:file:" + filename; + return fileOrUrl.substring(pathStart + "file:".length()); } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java index 4a180b4e224..868db53b9e4 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java @@ -1459,4 +1459,57 @@ public void handlesSameModelWhenBuiltAndImported() throws Exception { assertTrue(combinedModel.expectShape(ShapeId.from("smithy.example#MachineData$machineId"), MemberShape.class) .hasTrait(RequiredTrait.ID)); } + + @Test + public void importsJarFromPathWithEncodedChars() throws Exception { + Path source = Paths.get(getClass().getResource("assembler-valid-jar").toURI()); + Path jarPath = outputDirectory.resolve(Paths.get("path%20with%45encoded%25chars", "test.jar")); + + Files.createDirectories(jarPath.getParent()); + Files.copy(JarUtils.createJarFromDir(source), jarPath); + + Model model = Model.assembler() + .addImport(jarPath) + .assemble() + .unwrap(); + + assertTrue(model.getShape(ShapeId.from("smithy.example#ExampleString")).isPresent()); + } + + @Test + public void importsJarFromUrlWithEncodedChars() throws Exception { + Path source = Paths.get(getClass().getResource("assembler-valid-jar").toURI()); + Path jarPath = outputDirectory.resolve(Paths.get("path%20with%45encoded%25chars", "test.jar")); + + Files.createDirectories(jarPath.getParent()); + Files.copy(JarUtils.createJarFromDir(source), jarPath); + + Model model = Model.assembler() + .addImport(jarPath.toUri().toURL()) + .assemble() + .unwrap(); + + assertTrue(model.getShape(ShapeId.from("smithy.example#ExampleString")).isPresent()); + } + + @Test + public void importedJarsWithEncodedCharsHaveCorrectFilename() throws Exception { + Path source = Paths.get(getClass().getResource("assembler-valid-jar").toURI()); + Path jarPath = outputDirectory.resolve(Paths.get("path%20with%45encoded%25chars", "test.jar")); + + Files.createDirectories(jarPath.getParent()); + Files.copy(JarUtils.createJarFromDir(source), jarPath); + + Model model = Model.assembler() + .addImport(jarPath) + .assemble() + .unwrap(); + + String filename = model.expectShape(ShapeId.from("smithy.example#ExampleString")) + .getSourceLocation() + .getFilename(); + String fileContents = IoUtils.readUtf8Url(new URL(filename)); + + assertThat(fileContents, containsString("string ExampleString")); + } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelDiscoveryTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelDiscoveryTest.java index 098eb780d56..465e51548a8 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelDiscoveryTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelDiscoveryTest.java @@ -147,4 +147,14 @@ public void createSmithyManifestUrlFromJarUrl() throws IOException { assertThat(ModelDiscovery.createSmithyJarManifestUrl("jar:file:/foo.jar"), equalTo(new URL("jar:file:/foo.jar!/META-INF/smithy/manifest"))); } + + @Test + public void encodesSmithyManifestUrl() throws IOException { + assertThat(ModelDiscovery.createSmithyJarManifestUrl("/foo bar.jar"), + equalTo(new URL("jar:file:/foo%20bar.jar!/META-INF/smithy/manifest"))); + assertThat(ModelDiscovery.createSmithyJarManifestUrl("file:/foo bar.jar"), + equalTo(new URL("jar:file:/foo%20bar.jar!/META-INF/smithy/manifest"))); + assertThat(ModelDiscovery.createSmithyJarManifestUrl("jar:file:/foo bar.jar"), + equalTo(new URL("jar:file:/foo%20bar.jar!/META-INF/smithy/manifest"))); + } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-valid-jar/META-INF/MANIFEST.MF b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-valid-jar/META-INF/MANIFEST.MF new file mode 100644 index 00000000000..3ca5721135a --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-valid-jar/META-INF/MANIFEST.MF @@ -0,0 +1,2 @@ +Manifest-Version: 1.0 +Created-By: 11.0.6 (Amazon.com Inc.) diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-valid-jar/META-INF/smithy/manifest b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-valid-jar/META-INF/smithy/manifest new file mode 100644 index 00000000000..c7f006be08b --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-valid-jar/META-INF/smithy/manifest @@ -0,0 +1 @@ +valid.smithy diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-valid-jar/META-INF/smithy/valid.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-valid-jar/META-INF/smithy/valid.smithy new file mode 100644 index 00000000000..ddcadfbbcbc --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-valid-jar/META-INF/smithy/valid.smithy @@ -0,0 +1,5 @@ +$version: "2.0" + +namespace smithy.example + +string ExampleString