Repository: hyperledger/quilt
Pull request #463 (2020-05-11 21:56:54)

Fixes 441
Improve PaymentPointer parsing

Signed-off-by: David Fuelling <sappenin@gmail.com>


Patch:
diff --git a/spsp-parent/spsp-core/src/main/java/org/interledger/spsp/PaymentPointer.java b/spsp-parent/spsp-core/src/main/java/org/interledger/spsp/PaymentPointer.java
index f8fd7b9..b2ba8cb 100644
--- a/spsp-parent/spsp-core/src/main/java/org/interledger/spsp/PaymentPointer.java
+++ b/spsp-parent/spsp-core/src/main/java/org/interledger/spsp/PaymentPointer.java
@@ -14,36 +14,42 @@ import org.immutables.value.Value.Immutable;
  */
 public interface PaymentPointer {
 
-  public static final String WELL_KNOWN = "/.well-known/pay";
-  
+  String WELL_KNOWN = "/.well-known/pay";
+
   /**
    * Parses a payment pointer string into a @{code PaymentPointer}.
+   *
    * @param value text
+   *
    * @return payment pointer
+   *
    * @throws IllegalArgumentException if cannot be parsed
-   * @throws IllegalStateException if parsed payment pointer has invalid host and/or path values
+   * @throws IllegalStateException    if parsed payment pointer has invalid host and/or path values
    */
   static PaymentPointer of(String value) {
     if (!value.startsWith("$")) {
       throw new IllegalArgumentException("PaymentPointers must begin with $");
     }
+    if (value.contains("://")) {
+      throw new IllegalArgumentException("PaymentPointers paths must not contain a scheme");
+    }
     String address = value.substring(1);
     int pathIndex = address.indexOf("/");
     if (pathIndex >= 0) {
       return ImmutablePaymentPointer.builder()
-        .host(address.substring(0, pathIndex))
-        .path(address.substring(pathIndex))
-        .build();
+          .host(address.substring(0, pathIndex))
+          .path(address.substring(pathIndex))
+          .build();
     } else {
       return ImmutablePaymentPointer.builder()
-        .host(address)
-        .build();
+          .host(address)
+          .build();
     }
   }
 
   /**
-   * A host as defined by RFC-3986, which is generally (though not always) a registered name intended for lookup in
-   * the DNS.
+   * A host as defined by RFC-3986, which is generally (though not always) a registered name intended for lookup in the
+   * DNS.
    *
    * @return A {@link String} containing the `host` portion of this Payment Pointer.
    *
@@ -62,6 +68,7 @@ public interface PaymentPointer {
 
   @Immutable
   abstract class AbstractPaymentPointer implements PaymentPointer {
+
     @Default
     public String path() {
       return WELL_KNOWN;
@@ -77,19 +84,22 @@ public interface PaymentPointer {
       Preconditions.checkState(!host().isEmpty(), "PaymentPointers must specify a host");
       if (Strings.isNullOrEmpty(path())) {
         return ImmutablePaymentPointer.builder().from(this)
-          .path(WELL_KNOWN)
-          .build();
+            .path(WELL_KNOWN)
+            .build();
       }
       // Normalize acceptable input.
       if (path().equals("/")) {
         return ImmutablePaymentPointer.builder().from(this)
-          .path(WELL_KNOWN)
-          .build();
+            .path(WELL_KNOWN)
+            .build();
       }
       Preconditions.checkState(path().startsWith("/"), "path must start with a forward-slash");
       Preconditions.checkState(
           CharMatcher.ascii().matchesAllOf(toString()), "PaymentPointers may only contain ASCII characters");
 
+      // Exclude the userinfo, port, query, and fragment in the URL.
+      Preconditions.checkState(path().contains("://") == false, "PaymentPointers paths must not contain a scheme");
+
       return this;
     }
   }


Test patch:
diff --git a/spsp-parent/spsp-core/src/test/java/org/interledger/spsp/PaymentPointerTest.java b/spsp-parent/spsp-core/src/test/java/org/interledger/spsp/PaymentPointerTest.java
index fbf0b7f..adbbb91 100644
--- a/spsp-parent/spsp-core/src/test/java/org/interledger/spsp/PaymentPointerTest.java
+++ b/spsp-parent/spsp-core/src/test/java/org/interledger/spsp/PaymentPointerTest.java
@@ -43,8 +43,7 @@ public class PaymentPointerTest {
 
   @Test
   public void accountPath() {
-    assertThat(PaymentPointer.of("$example.com/foo").path())
-        .isEqualTo("/foo");
+    assertThat(PaymentPointer.of("$example.com/foo").path()).isEqualTo("/foo");
   }
 
   @Test
@@ -85,4 +84,10 @@ public class PaymentPointerTest {
     PaymentPointer.of("$ZA̡͊͠͝LGΌ IS̯͈͕̹̘̱ͮ TO͇̹̺ͅƝ̴ȳ̳ TH̘Ë͖́̉ ͠P̯͍̭O̚N̐Y̡");
   }
 
+  @Test
+  public void disallowScheme(){
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("PaymentPointers paths must not contain a scheme");
+    PaymentPointer.of("$https://dev.foo.example.com/someone");
+  }
 }