Skip unrecognized JSON files when loading models

smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelLoader.java

@@ -55,52 +55,67 @@ 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<String, Object> properties,
             String filename,
             Consumer<LoadOperation> operationConsumer,
             Supplier<InputStream> contentSupplier,
             Function<CharSequence, String> 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<LoadOperation> 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<LoadOperation> 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);
             }

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.
+     *
+     * <p>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.
+     *
+     * <p>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<Path>() {
+                    @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);
+        }
+    }
+}

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

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.)

smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-fail-invalid-jar/META-INF/smithy/invalid-array.json

@@ -0,0 +1,2 @@
+
+[]

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

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

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

smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-files/test.ion

@@ -0,0 +1 @@
+{}

smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/array.json

@@ -0,0 +1 @@
+[1, 2, 3]

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"
+        }
+    }
+}

smithy-model/src/test/resources/software/amazon/smithy/model/loader/assembler-ignore-unrecognized-json/unrecognized.json

@@ -0,0 +1,3 @@
+{
+    "foo": 1
+}