JCL-431: Improve containment validation (#653)

Author: acoburn

solid/src/main/java/com/inrupt/client/solid/SolidContainer.java

@@ -82,10 +82,11 @@ public SolidContainer(final URI identifier, final Dataset dataset, final Metadat
      * @return the contained resources
      */
     public Set<SolidResource> getResources() {
-        final String container = normalize(getIdentifier());
         // As defined by the Solid Protocol, containers always end with a slash.
-        if (container.endsWith("/")) {
-            final Node node = new Node(rdf.createIRI(getIdentifier().toString()), getGraph());
+        final URI base = getIdentifier().normalize();
+        if (isContainer(base)) {
+            final String container = normalize(base);
+            final Node node = new Node(rdf.createIRI(base.toString()), getGraph());
             try (final Stream<Node.TypedNode> stream = node.getResources()) {
                 return stream.filter(child -> verifyContainmentIri(container, child)).map(child -> {
                     final Metadata.Builder builder = Metadata.newBuilder();
@@ -100,14 +101,15 @@ public Set<SolidResource> getResources() {
 
     @Override
     public ValidationResult validate() {
-        // Get the normalized container URI
-        final String container = normalize(getIdentifier());
         final List<String> messages = new ArrayList<>();
         // Verify that the container URI path ends with a slash
-        if (!container.endsWith("/")) {
+        final URI base = getIdentifier().normalize();
+        if (!isContainer(base)) {
             messages.add("Container URI does not end in a slash");
         }
 
+        // Get the normalized container URI
+        final String container = normalize(base);
         // Verify that all ldp:contains triples align with Solid expectations
         getGraph().stream(null, rdf.createIRI(LDP.contains.toString()), null)
             .collect(Collectors.partitioningBy(verifyContainmentTriple(container)))
@@ -121,8 +123,8 @@ public ValidationResult validate() {
         return new ValidationResult(false, messages);
     }
 
-    static String normalize(final IRI iri) {
-        return normalize(URI.create(iri.getIRIString()));
+    static boolean isContainer(final URI uri) {
+        return uri.normalize().getPath().endsWith("/");
     }
 
     static String normalize(final URI uri) {
@@ -145,22 +147,44 @@ static Predicate<Triple> verifyContainmentTriple(final String container) {
     }
 
     static boolean verifyContainmentIri(final String container, final IRI object) {
-        if (!object.getIRIString().startsWith(container)) {
-            // Out-of-domain containment triple object
+
+        // URI Structure Tests
+        final URI base = URI.create(container).normalize();
+        final URI normalized = URI.create(object.getIRIString()).normalize();
+
+        // Query strings are not allowed in subject or object URI
+        if (base.getQuery() != null || normalized.getQuery() != null) {
+            return false;
+        }
+
+        // URI fragments are not allowed in subject or object URI
+        if (base.getFragment() != null || normalized.getFragment() != null) {
             return false;
-        } else {
-            final String relativePath = object.getIRIString().substring(container.length());
-            final String normalizedPath = relativePath.endsWith("/") ?
-                relativePath.substring(0, relativePath.length() - 1) : relativePath;
-            if (normalizedPath.isEmpty()) {
-                // Containment triple subject and object cannot be the same
-                return false;
-            }
-            if (normalizedPath.contains("/")) {
-                // Containment cannot skip intermediate nodes
-                return false;
-            }
         }
+
+        // Base URI cannot equal the object URI
+        if (base.getScheme().equals(normalized.getScheme()) &&
+                base.getSchemeSpecificPart().equals(normalized.getSchemeSpecificPart())) {
+            return false;
+        }
+
+        // Relative path tests
+        final URI relative = base.relativize(normalized);
+
+        // Object URI must be relative to (contained in) the base URI
+        if (relative.isAbsolute()) {
+            return false;
+        }
+
+        final String relativePath = relative.getPath();
+        final String normalizedPath = relativePath.endsWith("/") ?
+            relativePath.substring(0, relativePath.length() - 1) : relativePath;
+
+        // Containment cannot skip intermediate nodes
+        if (normalizedPath.contains("/")) {
+            return false;
+        }
+
         return true;
     }
 

solid/src/test/java/com/inrupt/client/solid/SolidClientTest.java

@@ -231,6 +231,8 @@ void testLowLevelSolidContainer() {
         expected.add(URIBuilder.newBuilder(uri).path("newContainer/").build());
         expected.add(URIBuilder.newBuilder(uri).path("test.txt").build());
         expected.add(URIBuilder.newBuilder(uri).path("test2.txt").build());
+        expected.add(URIBuilder.newBuilder(uri).path("test3").build());
+        expected.add(URIBuilder.newBuilder(uri).path("test4").build());
 
         client.send(Request.newBuilder(uri).build(), SolidResourceHandlers.ofSolidContainer())
             .thenAccept(response -> {

solid/src/test/java/com/inrupt/client/solid/SolidRDFSourceTest.java

@@ -194,6 +194,25 @@ void testEmptyContentType() {
         }
     }
 
+    @Test
+    void testNonContainer() {
+        final URI resource = URI.create(config.get("solid_resource_uri") + "/recipe");
+        final Request request = Request.newBuilder()
+            .uri(resource)
+            .header("Accept", "text/turtle")
+            .GET()
+            .build();
+
+        final Response<SolidContainer> response = client.send(request, SolidResourceHandlers.ofSolidContainer())
+            .toCompletableFuture().join();
+
+        try (final SolidContainer container = response.body()) {
+            assertEquals(resource, container.getIdentifier());
+            assertTrue(container.getResources().isEmpty());
+            assertFalse(container.validate().isValid());
+        }
+    }
+
     @Test
     void testInvalidRdf() {
         final URI resource = URI.create(config.get("solid_resource_uri") + "/nonRDF");

solid/src/test/resources/__files/container.ttl

@@ -7,6 +7,14 @@
     a ldp:BasicContainer ;
     dct:modified "2022-11-25T10:36:36Z"^^xsd:dateTime;
     ldp:contains <newContainer/>, <test.txt>, <test2.txt> .
+<intermediate/..>
+    a ldp:BasicContainer ;
+    dct:modified "2022-11-25T10:38:12Z"^^xsd:dateTime ;
+    ldp:contains <test3> .
+<intermediate/../>
+    a ldp:BasicContainer ;
+    dct:modified "2022-11-25T10:38:47Z"^^xsd:dateTime ;
+    ldp:contains <test4> .
 <newContainer/>
     a ldp:BasicContainer ;
     dct:modified "2022-11-25T10:36:36Z"^^xsd:dateTime .
@@ -16,10 +24,18 @@
 <test2.txt>
     a pl:Resource, ldp:NonRDFSource;
     dct:modified "2022-11-25T10:37:06Z"^^xsd:dateTime .
+<test3>
+    a ldp:RDFSource ;
+    dct:modified "2022-11-25T10:37:31Z"^^xsd:dateTime .
+<test4>
+    a ldp:RDFSource ;
+    dct:modified "2022-11-25T10:39:22Z"^^xsd:dateTime .
 
 # These containment triples should not be included in a getResources response
 <>
-    ldp:contains <https://example.com/other> , <newContainer/child> , <> , <./> .
+    ldp:contains <https://example.com/other> , <newContainer/child> , <newContainer%2Fchild2> ,  <> , <./> ,
+    <?foo> , <#bar> , <?foo#bar> , <./?foo> , <./#bar> , <./?foo#bar> ,
+    <../?foo> , <../#bar> , <../?foo#bar> , <child?foo> , <child#bar> , <child?foo#bar> .
 <https://example.test/container/>
     a ldp:BasicContainer ;
     ldp:contains <https://example.test/container/external> .