sanitize id paths via base64 encoding

projects/batfish-common-protocol/src/main/java/org/batfish/identifiers/FileBasedIdResolver.java

@@ -1,7 +1,10 @@
 package org.batfish.identifiers;
 
 import static java.util.Optional.ofNullable;
+import static org.batfish.storage.FileBasedStorage.fromBase64;
+import static org.batfish.storage.FileBasedStorage.toBase64;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
@@ -17,6 +20,7 @@
 import javax.annotation.Nullable;
 import org.batfish.common.BatfishException;
 import org.batfish.common.util.CommonUtil;
+import org.batfish.storage.FileBasedStorage;
 import org.batfish.storage.FileBasedStorageDirectoryProvider;
 
 /**
@@ -46,15 +50,24 @@ public class FileBasedIdResolver implements IdResolver {
     return Hashing.murmur3_128().hashString(input, StandardCharsets.UTF_8).toString();
   }
 
-  private static @Nonnull Set<String> listResolvableNames(Path idsDir) {
+  @VisibleForTesting
+  static @Nonnull Set<String> listResolvableNames(Path idsDir) {
     if (!Files.exists(idsDir)) {
       return ImmutableSet.of();
     }
     try (Stream<Path> files = Files.list(idsDir)) {
       return files
-          .filter(path -> path.toString().endsWith(ID_EXTENSION))
+          .filter(
+              path -> {
+                try {
+                  return fromBase64(path.getFileName().toString()).endsWith(ID_EXTENSION);
+                } catch (IllegalArgumentException e) {
+                  return false;
+                }
+              })
           .map(Path::getFileName)
           .map(Path::toString)
+          .map(FileBasedStorage::fromBase64)
           .map(
               nameWithExtension ->
                   nameWithExtension.substring(
@@ -81,7 +94,8 @@ public FileBasedIdResolver(Path storageBase) {
   }
 
   protected @Nonnull Path getAnalysisIdPath(String analysis, NetworkId networkId) {
-    return getAnalysisIdsDir(networkId).resolve(String.format("%s%s", analysis, ID_EXTENSION));
+    return getAnalysisIdsDir(networkId)
+        .resolve(toBase64(String.format("%s%s", analysis, ID_EXTENSION)));
   }
 
   protected @Nonnull Path getAnalysisIdsDir(NetworkId networkId) {
@@ -134,7 +148,7 @@ public FileBasedIdResolver(Path storageBase) {
 
   protected @Nonnull Path getIssueSettingsIdPath(String majorIssueType, NetworkId networkId) {
     return getIssueSettingsIdsDir(networkId)
-        .resolve(String.format("%s%s", majorIssueType, ID_EXTENSION));
+        .resolve(toBase64(String.format("%s%s", majorIssueType, ID_EXTENSION)));
   }
 
   protected @Nonnull Path getIssueSettingsIdsDir(NetworkId networkId) {
@@ -151,7 +165,7 @@ public FileBasedIdResolver(Path storageBase) {
   }
 
   protected @Nonnull Path getNetworkIdPath(String network) {
-    return getNetworkIdsDir().resolve(String.format("%s%s", network, ID_EXTENSION));
+    return getNetworkIdsDir().resolve(toBase64(String.format("%s%s", network, ID_EXTENSION)));
   }
 
   protected @Nonnull Path getNetworkIdsDir() {
@@ -183,7 +197,7 @@ public NodeRolesId getNetworkNodeRolesId(NetworkId networkId) {
   protected @Nonnull Path getQuestionIdPath(
       String question, NetworkId networkId, @Nullable AnalysisId analysisId) {
     return getQuestionIdsDir(networkId, analysisId)
-        .resolve(String.format("%s%s", question, ID_EXTENSION));
+        .resolve(toBase64(String.format("%s%s", question, ID_EXTENSION)));
   }
 
   protected @Nonnull Path getQuestionIdsDir(NetworkId networkId, @Nullable AnalysisId analysisId) {
@@ -209,7 +223,7 @@ public NodeRolesId getNetworkNodeRolesId(NetworkId networkId) {
 
   protected @Nonnull Path getQuestionSettingsIdPath(String questionClassId, NetworkId networkId) {
     return getQuestionSettingsIdsDir(networkId)
-        .resolve(String.format("%s%s", questionClassId, ID_EXTENSION));
+        .resolve(toBase64(String.format("%s%s", questionClassId, ID_EXTENSION)));
   }
 
   protected @Nonnull Path getQuestionSettingsIdsDir(NetworkId networkId) {
@@ -226,7 +240,8 @@ public NodeRolesId getNetworkNodeRolesId(NetworkId networkId) {
   }
 
   protected @Nonnull Path getSnapshotIdPath(String snapshot, NetworkId networkId) {
-    return getSnapshotIdsDir(networkId).resolve(String.format("%s%s", snapshot, ID_EXTENSION));
+    return getSnapshotIdsDir(networkId)
+        .resolve(toBase64(String.format("%s%s", snapshot, ID_EXTENSION)));
   }
 
   protected @Nonnull Path getSnapshotIdsDir(NetworkId networkId) {

projects/batfish-common-protocol/src/main/java/org/batfish/storage/FileBasedStorage.java

@@ -841,11 +841,11 @@ public void deleteNetworkObject(NetworkId networkId, String key)
     return _d.getSnapshotObjectsDir(networkId, snapshotId).resolve(encodedKey);
   }
 
-  private static @Nonnull String toBase64(String key) {
+  public static @Nonnull String toBase64(String key) {
     return Base64.getUrlEncoder().encodeToString(key.getBytes(StandardCharsets.UTF_8));
   }
 
-  private static @Nonnull String fromBase64(String key) {
+  public static @Nonnull String fromBase64(String key) {
     return new String(Base64.getUrlDecoder().decode(key), StandardCharsets.UTF_8);
   }
 

projects/batfish-common-protocol/src/test/java/org/batfish/identifiers/BUILD

@@ -0,0 +1,45 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+package(
+    default_testonly = True,
+    default_visibility = ["//visibility:public"],
+)
+
+load("@batfish//skylark:junit.bzl", "junit_tests")
+load("@batfish//skylark:pmd_test.bzl", "pmd_test")
+
+java_library(
+    name = "identifiers",
+    srcs = glob(
+        ["*.java"],
+        exclude = ["*Test.java"],
+    ),
+    deps = [
+        "//projects/batfish-common-protocol:common",
+        "@maven//:com_google_code_findbugs_jsr305",
+    ],
+)
+
+pmd_test(
+    name = "pmd",
+    lib = ":identifiers",
+)
+
+junit_tests(
+    name = "tests",
+    srcs = glob(
+        ["*.java"],
+        exclude = ["Test*.java"],
+
+    ),
+    deps = [
+        "//projects/batfish-common-protocol:common",
+        "@maven//:com_fasterxml_jackson_core_jackson_core",
+        "@maven//:com_google_code_findbugs_jsr305",
+        "@maven//:com_google_guava_guava",
+        "@maven//:junit_junit",
+        "@maven//:org_hamcrest_hamcrest",
+    ],
+)
+
+

projects/batfish-common-protocol/src/test/java/org/batfish/identifiers/FileBasedIdResolverTest.java

@@ -0,0 +1,86 @@
+package org.batfish.identifiers;
+
+import static org.batfish.identifiers.FileBasedIdResolver.listResolvableNames;
+import static org.batfish.storage.FileBasedStorage.fromBase64;
+import static org.batfish.storage.FileBasedStorage.toBase64;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+/** Test of {@link FileBasedIdResolver}. */
+public final class FileBasedIdResolverTest {
+
+  private FileBasedIdResolver _resolver;
+
+  @Rule public TemporaryFolder _folder = new TemporaryFolder();
+
+  @Before
+  public void setup() {
+    _resolver = new FileBasedIdResolver(_folder.getRoot().toPath());
+  }
+
+  private static void assertLastComponentBase64Encoded(Path idPath) {
+    assertNotNull(fromBase64(idPath.getFileName().toString()));
+  }
+
+  @Test
+  public void testListResolvableNames() throws IOException {
+    Path root = _folder.getRoot().toPath();
+    Files.createFile(root.resolve(toBase64("foo.id")));
+    Files.createFile(root.resolve(toBase64("bar.id")));
+    Files.createFile(root.resolve("baz.id")); // should be excluded because it is not base64-encoded
+    Files.createFile(
+        root.resolve(toBase64("bath"))); // should be excluded because it doesn't end in ID
+
+    assertThat(listResolvableNames(root), containsInAnyOrder("foo", "bar"));
+  }
+
+  @Test
+  public void testGetAnalysisIdPath() {
+    assertLastComponentBase64Encoded(
+        _resolver.getAnalysisIdPath("analysis1", new NetworkId("net1_id")));
+  }
+
+  @Test
+  public void testGetIssueSettingsIdPath() {
+    assertLastComponentBase64Encoded(
+        _resolver.getIssueSettingsIdPath("majorIssueType1", new NetworkId("net1_id")));
+  }
+
+  @Test
+  public void testGetNetworkIdPath() {
+    assertLastComponentBase64Encoded(_resolver.getNetworkIdPath("net1"));
+  }
+
+  @Test
+  public void testGetQuestionIdPath() {
+    // with analysis
+    assertLastComponentBase64Encoded(
+        _resolver.getQuestionIdPath(
+            "question1", new NetworkId("net1_id"), new AnalysisId("analysis1_id")));
+
+    // without analysis
+    assertLastComponentBase64Encoded(
+        _resolver.getQuestionIdPath("question1", new NetworkId("net1_id"), null));
+  }
+
+  @Test
+  public void testGetQuestionSettingsIdPath() {
+    assertLastComponentBase64Encoded(
+        _resolver.getQuestionSettingsIdPath("questionClassId1", new NetworkId("net1_id")));
+  }
+
+  @Test
+  public void testGetSnapshotIdPath() {
+    assertLastComponentBase64Encoded(
+        _resolver.getSnapshotIdPath("snapshot1", new NetworkId("net1_id")));
+  }
+}

projects/batfish/BUILD

@@ -115,6 +115,7 @@ junit_tests(
         "//projects/batfish-common-protocol/src/test/java/org/batfish/common/bdd:matchers",
         "//projects/batfish-common-protocol/src/test/java/org/batfish/common/matchers",
         "//projects/batfish-common-protocol/src/test/java/org/batfish/datamodel/matchers",
+        "//projects/batfish-common-protocol/src/test/java/org/batfish/identifiers",
         "//projects/bdd",
         "//projects/symbolic",
         "@maven//:com_google_code_findbugs_jsr305",