url encode manifest templates when serializing into json

otherwise it is not a valid uri
ClusterlessHQ · Jul 19, 2023 · cd34040 · cd34040
1 parent 5f41f59
commit cd34040
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 3 deletions.
diff --git a/clusterless-common/src/main/java/clusterless/util/URIs.java b/clusterless-common/src/main/java/clusterless/util/URIs.java
@@ -14,10 +14,13 @@
 import javax.annotation.Nullable;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.net.URLEncoder;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.regex.Pattern;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 /**
  *
  */
@@ -200,4 +203,16 @@ public static URI fromTo(URI fromBase, URI from, URI toBase) {
 
         return copyAppend(toBase, fromPath.substring(fromBasePath.length()));
     }
+
+    public static String encodeOnly(String chars, String path) {
+        if (chars.isEmpty()) {
+            return path;
+        }
+
+        for (String c : chars.split("")) {
+            String encode = URLEncoder.encode(c, UTF_8);
+            path = path.replace(c, encode);
+        }
+        return path;
+    }
 }
diff --git a/clusterless-common/src/test/java/clusterless/util/URIsTest.java b/clusterless-common/src/test/java/clusterless/util/URIsTest.java
@@ -178,4 +178,11 @@ public void fromTo(String fromBaseString, String fromString, String toBaseString
 
         Assertions.assertEquals(expectedTo, actualTo);
     }
+
+    @Test
+    public void selectEncode() {
+        Assertions.assertEquals("foo/bar", URIs.encodeOnly("", "foo/bar"));
+        Assertions.assertEquals("foo/%7Bbar%7D", URIs.encodeOnly("{}", "foo/{bar}"));
+        Assertions.assertEquals("foo/%7Bb+a+r%7D", URIs.encodeOnly("{} ", "foo/{b a r}"));
+    }
 }
diff --git a/clusterless-substrate/src/main/java/clusterless/substrate/uri/ManifestURI.java b/clusterless-substrate/src/main/java/clusterless/substrate/uri/ManifestURI.java
@@ -14,6 +14,7 @@
 import clusterless.naming.Partition;
 import clusterless.substrate.store.StateStore;
 import clusterless.util.Lazy;
+import clusterless.util.URIs;
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.core.JacksonException;
 import com.fasterxml.jackson.core.JsonGenerator;
@@ -28,11 +29,13 @@
 
 import java.io.IOException;
 import java.net.URI;
+import java.net.URLDecoder;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Supplier;
 
 import static clusterless.util.Optionals.optional;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 /**
  * Identifier
@@ -58,7 +61,7 @@ protected Serializer() {
 
         @Override
         public void serialize(ManifestURI value, JsonGenerator gen, SerializerProvider provider) throws IOException {
-            gen.writeString(value.template());
+            gen.writeString(value.templateEncoded());
         }
     }
 
@@ -117,6 +120,8 @@ public static ManifestURI parse(URI uri) {
     public static ManifestURI parse(String template) {
         Objects.requireNonNull(template, "template is null");
 
+        template = URLDecoder.decode(template, UTF_8);
+
         // {provider-service}://{manifest-store}/datasets/{dataset-name}/{dataset-version}/{lot}/{state}[/{attempt}]/manifest.{ext}
         String[] split = template.split("/");
 
@@ -181,7 +186,19 @@ public URI uri() {
     @Override
     public String template() {
         // {provider-service}://{manifest-store}/datasets/{dataset-name}/{dataset-version}/{lot}/{state}{/attempt*}]/manifest.{ext}
+        String path = templatePath();
+
+        return String.format("s3://%s/%s/%s", storeName.get(), DATASETS, path);
+    }
+
+    public String templateEncoded() {
+        // {provider-service}://{manifest-store}/datasets/{dataset-name}/{dataset-version}/{lot}/{state}{/attempt*}]/manifest.{ext}
+        String path = URIs.encodeOnly("{} ", templatePath());
 
+        return String.format("s3://%s/%s/%s", storeName.get(), DATASETS, path);
+    }
+
+    protected String templatePath() {
         // bypass the named partition and store directly
         Partition manifest = Optional.ofNullable(state).map(s -> (Partition) s).orElse(Partition.namedOf("state", "{state}"));
 
@@ -198,8 +215,7 @@ public String template() {
                 .withNamed("lot", Optional.ofNullable(lotId).orElse("{lot}")) // retain case
                 .with(manifest)
                 .partition();
-
-        return String.format("s3://%s/%s/%s", storeName.get(), DATASETS, path);
+        return path;
     }
 
     @JsonIgnore