fix(bzlmod): throw on json parse exception

[Fixes](bazelbuild#14437) Get rid of new lines in `bazel_registry.json` and return an `Optional.empty()` aka registry descriptor without mirrors and throw an IOException for malformed JSON to stick to official json.org specs Closes bazelbuild#15109. PiperOrigin-RevId: 436992309
fweikert · Mar 24, 2022 · ac21910 · ac21910
1 parent 45b225d
commit ac21910
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 1 deletion.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java
@@ -25,6 +25,7 @@
 import com.google.gson.FieldNamingPolicy;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
+import com.google.gson.JsonParseException;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.net.MalformedURLException;
@@ -115,7 +116,14 @@ private <T> Optional<T> grabJson(String url, Class<T> klass, ExtendedEventHandle
       return Optional.empty();
     }
     String jsonString = new String(bytes.get(), UTF_8);
-    return Optional.of(gson.fromJson(jsonString, klass));
+    if (jsonString.strip().isEmpty()) {
+      return Optional.empty();
+    }
+    try {
+      return Optional.of(gson.fromJson(jsonString, klass));
+    } catch (JsonParseException e) {
+      throw new IOException(String.format("Unable to parse json at url %s", url), e);
+    }
   }
 
   @Override

diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java
@@ -19,13 +19,15 @@
 import static com.google.common.truth.Truth8.assertThat;
 import static com.google.devtools.build.lib.bazel.bzlmod.BzlmodTestUtil.createModuleKey;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.base.Suppliers;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
 import com.google.devtools.build.lib.testutil.FoundationTestCase;
 import java.io.File;
+import java.io.IOException;
 import java.io.Writer;
 import java.nio.file.Files;
 import org.junit.Before;
@@ -140,4 +142,54 @@ public void testGetRepoSpec() throws Exception {
                 .setRemotePatchStrip(3)
                 .build());
   }
+
+  @Test
+  public void testGetRepoInvalidRegistryJsonSpec() throws Exception {
+    server.serve("/bazel_registry.json", "", "", "", "");
+    server.start();
+    server.serve(
+        "/modules/foo/1.0/source.json",
+        "{",
+        "  \"url\": \"http://mysite.com/thing.zip\",",
+        "  \"integrity\": \"sha256-blah\",",
+        "  \"strip_prefix\": \"pref\"",
+        "}");
+
+    Registry registry = registryFactory.getRegistryWithUrl(server.getUrl());
+    assertThat(registry.getRepoSpec(createModuleKey("foo", "1.0"), "foorepo", reporter))
+        .isEqualTo(
+            new ArchiveRepoSpecBuilder()
+                .setRepoName("foorepo")
+                .setUrls(ImmutableList.of("http://mysite.com/thing.zip"))
+                .setIntegrity("sha256-blah")
+                .setStripPrefix("pref")
+                .setRemotePatches(ImmutableMap.of())
+                .setRemotePatchStrip(0)
+                .build());
+  }
+
+  @Test
+  public void testGetRepoInvalidModuleJsonSpec() throws Exception {
+    server.serve(
+        "/bazel_registry.json",
+        "{",
+        "  \"mirrors\": [",
+        "    \"https://mirror.bazel.build/\",",
+        "    \"file:///home/bazel/mymirror/\"",
+        "  ]",
+        "}");
+    server.serve(
+        "/modules/foo/1.0/source.json",
+        "{",
+        "  \"url\": \"http://mysite.com/thing.zip\",",
+        "  \"integrity\": \"sha256-blah\",",
+        "  \"strip_prefix\": \"pref\",",
+        "}");
+    server.start();
+
+    Registry registry = registryFactory.getRegistryWithUrl(server.getUrl());
+    assertThrows(
+        IOException.class,
+        () -> registry.getRepoSpec(createModuleKey("foo", "1.0"), "foorepo", reporter));
+  }
 }