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