From e6ee644a8d15e650c197b08b011707e87ea19c85 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Wed, 5 Jul 2023 16:44:47 -0500 Subject: [PATCH] Skip unrecognized JSON files when loading models This commit updates the model loading process so that JSON files that are not objects or that don't contain a top-level "smithy" key are skipped. This can be useful when loading directories that contain Smithy models mixed with other JSON files. JAR files that contain Smithy models are not permitted to refer to unrecognized JSON files that lack a valid "smithy" version key/value pair. This commit also only creates InputStreams for files that Smithy can actually load or attempt to load. Smithy would previously open an InputStream for all recursive files in a directory even if it was unable to load the file based on the filename. It now only creates an InputStream when the file has a .json, .smithy, or no file extension. Closes #1841 --- .../smithy/model/loader/ModelLoader.java | 65 ++++++++++----- .../amazon/smithy/model/JarUtils.java | 83 +++++++++++++++++++ .../model/loader/ModelAssemblerTest.java | 38 ++++++++- .../META-INF/MANIFEST.MF | 2 + .../META-INF/smithy/invalid-array.json | 2 + .../META-INF/smithy/manifest | 4 + .../META-INF/smithy/valid.smithy | 5 ++ .../main.smithy | 5 ++ .../test.ion | 1 + .../array.json | 1 + .../smithy.json | 8 ++ .../unrecognized.json | 3 + 12 files changed, 194 insertions(+), 23 deletions(-) create mode 100644 smithy-model/src/test/java/software/amazon/smithy/model/JarUtils.java create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/MANIFEST.MF create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/invalid-array.json create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/manifest create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/valid.smithy create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-files/main.smithy create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-files/test.ion create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/array.json create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/smithy.json create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/unrecognized.json diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java index ef575a777ce..cc189a5af29 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java @@ -55,9 +55,10 @@ private ModelLoader() {} * @param contentSupplier The supplier that provides an InputStream. The * supplied {@code InputStream} is automatically closed when the loader * has finished reading from it. + * @return Returns true if the file was loaded. Some JSON files might be ignored and return false. * @throws SourceException if there is an error reading from the contents. */ - static void load( + static boolean load( TraitFactory traitFactory, Map properties, String filename, @@ -65,42 +66,56 @@ static void load( Supplier contentSupplier, Function stringTable ) { - try (InputStream inputStream = contentSupplier.get()) { + try { if (filename.endsWith(".smithy")) { - String contents = IoUtils.toUtf8String(inputStream); - new IdlModelLoader(filename, contents, stringTable).parse(operationConsumer); + try (InputStream inputStream = contentSupplier.get()) { + String contents = IoUtils.toUtf8String(inputStream); + new IdlModelLoader(filename, contents, stringTable).parse(operationConsumer); + } + return true; } else if (filename.endsWith(".jar")) { loadJar(traitFactory, properties, filename, operationConsumer, stringTable); + return true; } else if (filename.endsWith(".json") || filename.equals(SourceLocation.NONE.getFilename())) { - // Assume it's JSON if there's a N/A filename. - loadParsedNode(Node.parse(inputStream, filename), operationConsumer); + try (InputStream inputStream = contentSupplier.get()) { + // Assume it's JSON if there's an N/A filename. + return loadParsedNode(Node.parse(inputStream, filename), operationConsumer); + } } else { - LOGGER.warning(() -> "No ModelLoader was able to load " + filename); + LOGGER.warning(() -> "Ignoring unrecognized file: " + filename); + return false; } } catch (IOException e) { throw new ModelImportException("Error loading " + filename + ": " + e.getMessage(), e); } } - // Loads all supported JSON formats. Each JSON format is expected to have - // a top-level version property that contains a string. This version - // is then used to delegate loading to different versions of the - // Smithy JSON AST format. + // Attempts to load a Smithy AST JSON model. JSON files that do not contain a top-level "smithy" key are skipped + // and false is returned. The "smithy" version is used to delegate loading to different versions of the Smithy + // JSON AST format. // // This loader supports version 1.0 and 2.0. Support for 0.5 and 0.4 was removed in 0.10. - static void loadParsedNode(Node node, Consumer operationConsumer) { - ObjectNode model = node.expectObjectNode("Smithy documents must be an object. Found {type}."); - StringNode versionNode = model.expectStringMember("smithy"); - Version version = Version.fromString(versionNode.getValue()); - - if (version != null) { - new AstModelLoader(version, model).parse(operationConsumer); - } else { - throw new ModelSyntaxException("Unsupported Smithy version number: " + versionNode.getValue(), versionNode); + static boolean loadParsedNode(Node node, Consumer operationConsumer) { + if (node.isObjectNode()) { + ObjectNode model = node.expectObjectNode(); + if (model.containsMember("smithy")) { + StringNode versionNode = model.expectStringMember("smithy"); + Version version = Version.fromString(versionNode.getValue()); + if (version == null) { + throw new ModelSyntaxException("Unsupported Smithy version number: " + versionNode.getValue(), + versionNode); + } else { + new AstModelLoader(version, model).parse(operationConsumer); + return true; + } + } } + + LOGGER.info("Ignoring unrecognized JSON file: " + node.getSourceLocation()); + return false; } - // Allows importing JAR files by discovering models inside of a JAR file. + // Allows importing JAR files by discovering models inside a JAR file. // This is similar to model discovery, but done using an explicit import. private static void loadJar( TraitFactory traitFactory, @@ -120,13 +135,19 @@ private static void loadJar( connection.setUseCaches(false); } - load(traitFactory, properties, model.toExternalForm(), operationConsumer, () -> { + boolean result = load(traitFactory, properties, model.toExternalForm(), operationConsumer, () -> { try { return connection.getInputStream(); } catch (IOException e) { throw throwIoJarException(model, e); } }, stringTable); + + // Smithy will skip unrecognized model files, including JSON files that don't contain a "smithy" + // version key/value pair. However, JAR manifests are not allowed to refer to unrecognized files. + if (!result) { + throw new ModelImportException("Invalid file referenced by Smithy JAR manifest: " + model); + } } catch (IOException e) { throw throwIoJarException(model, e); } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/JarUtils.java b/smithy-model/src/test/java/software/amazon/smithy/model/JarUtils.java new file mode 100644 index 00000000000..2186cecbe79 --- /dev/null +++ b/smithy-model/src/test/java/software/amazon/smithy/model/JarUtils.java @@ -0,0 +1,83 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.model; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.jar.Attributes; +import java.util.jar.JarEntry; +import java.util.jar.JarOutputStream; +import java.util.jar.Manifest; +import software.amazon.smithy.utils.SmithyInternalApi; + +@SmithyInternalApi +public final class JarUtils { + /** + * Creates a JAR in a temp directory on demand for test cases based on a directory. + * + *

This method is preferred over embedding JARs directly as resources when possible, because generated JARs + * don't need to be manually recreated if their contents need to change, and we don't need to commit blobs to VCS. + * + *

TODO: migrate other test cases to use this. + * + * @param source Where the files for the JAR are stored, including the required "META-INF/MANIFEST.MF" file. + * @return Returns the path to the temporary JAR file. + */ + public static Path createJarFromDir(Path source) { + try { + Path target = Files.createTempFile("temp-jar", ".jar"); + + Path relativeManifestLocation = Paths.get("META-INF").resolve("MANIFEST.MF"); + Manifest manifest; + + // Requires a manifest to be provided. + Path manifestLocation = target.resolve(relativeManifestLocation); + if (Files.isRegularFile(manifestLocation)) { + manifest = new Manifest(Files.newInputStream(manifestLocation)); + } else { + manifest = new Manifest(); + manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0"); + } + + try (JarOutputStream stream = new JarOutputStream(Files.newOutputStream(target), manifest)) { + Files.walkFileTree(source, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + Path relative = source.relativize(file); + // The manifest is added through the constructor. + if (!relative.equals(relativeManifestLocation)) { + JarEntry entry = new JarEntry(relative.toString().replace("\\", "/")); + entry.setTime(file.toFile().lastModified()); + stream.putNextEntry(entry); + Files.copy(file, stream); + } + return FileVisitResult.CONTINUE; + } + }); + } + + return target; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } +} 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 fa24b0baa82..1b5ff58d82f 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 @@ -33,6 +33,7 @@ import java.io.File; import java.io.IOException; +import java.net.URISyntaxException; import java.net.URL; import java.net.URLClassLoader; import java.nio.file.FileSystemException; @@ -48,12 +49,12 @@ import java.util.Optional; import java.util.function.Supplier; import java.util.stream.Collectors; - import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import software.amazon.smithy.model.JarUtils; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.SourceLocation; import software.amazon.smithy.model.node.Node; @@ -1211,4 +1212,39 @@ public void versionTransformsAreAlwaysApplied() { assertThat(fooBam.getAllTraits(), hasKey(BoxTrait.ID)); assertThat(fooBam.expectTrait(DefaultTrait.class).toNode(), equalTo(Node.nullNode())); } + + @Test + public void ignoresUnrecognizedFileExtensions() throws URISyntaxException { + ValidatedResult result = Model.assembler() + .addImport(Paths.get(getClass().getResource("assembler-ignore-unrecognized-files").toURI())) + .assemble(); + + assertThat(result.getValidationEvents(Severity.DANGER), empty()); + assertThat(result.getValidationEvents(Severity.ERROR), empty()); + + result.unwrap().expectShape(ShapeId.from("smithy.example#MyString")); + } + + @Test + public void ignoresUnrecognizedJsonFiles() throws URISyntaxException { + ValidatedResult result = Model.assembler() + .addImport(Paths.get(getClass().getResource("assembler-ignore-unrecognized-json").toURI())) + .assemble(); + + assertThat(result.getValidationEvents(Severity.DANGER), empty()); + assertThat(result.getValidationEvents(Severity.ERROR), empty()); + + result.unwrap().expectShape(ShapeId.from("smithy.example#MyString")); + } + + @Test + public void failsOnInvalidJarJsonFile() throws URISyntaxException, IOException { + Path jar = JarUtils.createJarFromDir(Paths.get(getClass().getResource("assembler-fail-invalid-jar").toURI())); + + ModelImportException e = Assertions.assertThrows(ModelImportException.class, () -> { + Model.assembler().addImport(jar).assemble(); + }); + + assertThat(e.getMessage(), containsString("Invalid file referenced by Smithy JAR manifest")); + } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/MANIFEST.MF b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/MANIFEST.MF new file mode 100644 index 00000000000..4d241de43a8 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-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-fail-invalid-jar/META-INF/smithy/invalid-array.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/invalid-array.json new file mode 100644 index 00000000000..6c87723be2c --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/invalid-array.json @@ -0,0 +1,2 @@ + +[] diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/manifest b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/manifest new file mode 100644 index 00000000000..262723db7f9 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/manifest @@ -0,0 +1,4 @@ +# This is loaded first and succeeds. +valid.smithy +# This fails because JARs cannot explicitly refer to unrecognized models files or JSON files. +invalid-array.json diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/valid.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/valid.smithy new file mode 100644 index 00000000000..b528584b5d0 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/valid.smithy @@ -0,0 +1,5 @@ +$version: "2.0" + +namespace smithy.example + +string MyString diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-files/main.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-files/main.smithy new file mode 100644 index 00000000000..c688b478dd3 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-files/main.smithy @@ -0,0 +1,5 @@ +$version: "2" + +namespace smithy.example + +string MyString diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-files/test.ion b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-files/test.ion new file mode 100644 index 00000000000..0967ef424bc --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-files/test.ion @@ -0,0 +1 @@ +{} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/array.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/array.json new file mode 100644 index 00000000000..b5d8bb58d9b --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/array.json @@ -0,0 +1 @@ +[1, 2, 3] diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/smithy.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/smithy.json new file mode 100644 index 00000000000..30fa52a8b5e --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/smithy.json @@ -0,0 +1,8 @@ +{ + "smithy": "2", + "shapes": { + "smithy.example#MyString": { + "type": "string" + } + } +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/unrecognized.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/unrecognized.json new file mode 100644 index 00000000000..1ca29148762 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/unrecognized.json @@ -0,0 +1,3 @@ +{ + "foo": 1 +}