Repository: solid/community-server
Pull request #620 (2021-02-17 14:32:00)

feat/correctly-handle-slug-in-POST-request

BelgianNoise: I noticed when running `npm run test` that `SparqlStorage.test.ts` does not run. This made the tests fail during the checks.
RubenVerborgh: > when running `npm run test` that `SparqlStorage.test.ts` does not run.

The tests are skipped by default to avoid that every developer needs to set up a Docker environment.

However, the fix worries me; the `Location` header should still be there?

joachimvh: > However, the fix worries me; the `Location` header should still be there?

PUT is being used instead of POST here to create the resources now. Currently the CSS only exposes the Location header for POST requests.
RubenVerborgh: > PUT is being used instead of POST here to create the resources now.

Shouldn't we still test both though?
joachimvh: > Shouldn't we still test both though?

We should, we should also just clean up the integration tests in general. 😄 

One problem was that there was a helper function being used called `createResource` which did a POST to a container with slug `testfolder/testfile.txt` to create both the folder and the file. This would break now since that is what we want to reject so that function had to be changed to use PUT to function correctly.

I didn't have an in-depth look at this PR yet though so I can't comment if perhaps too many things were removed.
joachimvh: As an addendum, even if all the location headers would disappear here, we still have an integration test for that specifically at https://github.com/solid/community-server/blob/main/test/integration/ServerFetch.test.ts#L71-L95

Patch:
diff --git a/bin/server.js b/bin/server.js
index 0fed11b85..67cc27c6f 100755
--- a/bin/server.js
+++ b/bin/server.js
@@ -1,4 +1,4 @@
 #!/usr/bin/env node
 // eslint-disable-next-line @typescript-eslint/naming-convention
 const { CliRunner } = require('..');
-new CliRunner().run();
+new CliRunner(process).run();
diff --git a/package-lock.json b/package-lock.json
index 8f787eadf..4531b285e 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -7649,7 +7649,6 @@
       "version": "1.2.0",
       "resolved": "https://registry.npmjs.org/rdf-isomorphic/-/rdf-isomorphic-1.2.0.tgz",
       "integrity": "sha512-Dq+iuWrVuK7q3P4/nychbWhRJ1M5yMAekNJN8f5pjarE8SH9Duy/R0JopVF0I0Q2w0poZlsVKKIBpeG+AdOSAw==",
-      "dev": true,
       "requires": {
         "rdf-string": "^1.5.0",
         "rdf-terms": "^1.6.2"
@@ -8881,13 +8880,14 @@
       "dev": true
     },
     "sparqlalgebrajs": {
-      "version": "2.4.0",
-      "resolved": "https://registry.npmjs.org/sparqlalgebrajs/-/sparqlalgebrajs-2.4.0.tgz",
-      "integrity": "sha512-6glKn1uWBsdPuQ4D+4r5m8mgWZoMfiNgip4uyblULTmgISqcbsQzrlrIhWQoZSX95QLLlWlYJufhelQAIRAWKg==",
+      "version": "2.5.0",
+      "resolved": "https://registry.npmjs.org/sparqlalgebrajs/-/sparqlalgebrajs-2.5.0.tgz",
+      "integrity": "sha512-dNJf4xUj5DFZc/9vQnDU5y9u8l4MfvOkMgx6PAefhTjAK0HHChxLZFF4n6GngWfvEZQ3/HcfmQk3cQo6sT/6bQ==",
       "requires": {
         "fast-deep-equal": "^3.1.1",
         "minimist": "^1.2.5",
         "rdf-data-factory": "^1.0.0",
+        "rdf-isomorphic": "^1.2.0",
         "rdf-string": "^1.5.0",
         "sparqljs": "^3.1.1"
       }
diff --git a/package.json b/package.json
index 6e7385686..2b5a12321 100644
--- a/package.json
+++ b/package.json
@@ -104,7 +104,7 @@
     "rdf-parse": "^1.7.0",
     "rdf-serialize": "^1.1.0",
     "rdf-terms": "^1.5.1",
-    "sparqlalgebrajs": "^2.3.1",
+    "sparqlalgebrajs": "^2.5.0",
     "sparqljs": "^3.1.2",
     "streamify-array": "^1.0.1",
     "uuid": "^8.3.0",
diff --git a/src/storage/DataAccessorBasedStore.ts b/src/storage/DataAccessorBasedStore.ts
index c28a21e82..52a08d450 100644
--- a/src/storage/DataAccessorBasedStore.ts
+++ b/src/storage/DataAccessorBasedStore.ts
@@ -22,6 +22,7 @@ import {
   isContainerIdentifier,
   isContainerPath,
   trimTrailingSlashes,
+  toCanonicalUriPath,
 } from '../util/PathUtil';
 import { parseQuads } from '../util/QuadUtil';
 import { generateResourceQuads } from '../util/ResourceUtil';
@@ -347,8 +348,22 @@ export class DataAccessorBasedStore implements ResourceStore {
    * @param slug - Slug to use for the new URI.
    */
   protected createURI(container: ResourceIdentifier, isContainer: boolean, slug?: string): ResourceIdentifier {
-    return { path:
-        `${ensureTrailingSlash(container.path)}${slug ? trimTrailingSlashes(slug) : uuid()}${isContainer ? '/' : ''}` };
+    const base = ensureTrailingSlash(container.path);
+    const name = (slug && this.cleanSlug(slug)) ?? uuid();
+    const suffix = isContainer ? '/' : '';
+    return { path: `${base}${name}${suffix}` };
+  }
+
+  /**
+   * Clean http Slug to be compatible with the server. Makes sure there are no unwanted characters
+   * e.g.: cleanslug('&%26') returns '%26%26'
+   * @param slug - the slug to clean
+   */
+  protected cleanSlug(slug: string): string {
+    if (/\/[^/]/u.test(slug)) {
+      throw new BadRequestHttpError('Slugs should not contain slashes');
+    }
+    return toCanonicalUriPath(trimTrailingSlashes(slug));
   }
 
   /**
diff --git a/src/storage/patch/SparqlUpdatePatchHandler.ts b/src/storage/patch/SparqlUpdatePatchHandler.ts
index c0671278e..455c09372 100644
--- a/src/storage/patch/SparqlUpdatePatchHandler.ts
+++ b/src/storage/patch/SparqlUpdatePatchHandler.ts
@@ -51,6 +51,14 @@ export class SparqlUpdatePatchHandler extends PatchHandler {
     return op.type === Algebra.types.COMPOSITE_UPDATE;
   }
 
+  private isBasicGraphPatternWithoutVariables(op: Algebra.Operation): op is Algebra.Bgp {
+    if (op.type !== Algebra.types.BGP) {
+      return false;
+    }
+    return !(op.patterns as BaseQuad[]).some((pattern): boolean =>
+      someTerms(pattern, (term): boolean => term.termType === 'Variable'));
+  }
+
   /**
    * Checks if the input operation is of a supported type (DELETE/INSERT or composite of those)
    */
@@ -67,7 +75,7 @@ export class SparqlUpdatePatchHandler extends PatchHandler {
 
   /**
    * Checks if the input DELETE/INSERT is supported.
-   * This means: no GRAPH statements, no DELETE WHERE.
+   * This means: no GRAPH statements, no DELETE WHERE containing terms of type Variable.
    */
   private validateDeleteInsert(op: Algebra.DeleteInsert): void {
     const def = defaultGraph();
@@ -81,8 +89,7 @@ export class SparqlUpdatePatchHandler extends PatchHandler {
       this.logger.warn('GRAPH statement in INSERT clause');
       throw new NotImplementedHttpError('GRAPH statements are not supported');
     }
-    if (op.where ?? deletes.some((pattern): boolean =>
-      someTerms(pattern, (term): boolean => term.termType === 'Variable'))) {
+    if (!(typeof op.where === 'undefined' || this.isBasicGraphPatternWithoutVariables(op.where))) {
       this.logger.warn('WHERE statements are not supported');
       throw new NotImplementedHttpError('WHERE statements are not supported');
     }


Test patch:
diff --git a/test/deploy/validate-package.sh b/test/deploy/validate-package.sh
index c70ae8611..bccf345a0 100755
--- a/test/deploy/validate-package.sh
+++ b/test/deploy/validate-package.sh
@@ -1,32 +1,37 @@
 #!/usr/bin/env bash
-# Script to validate our packed package
+# Script to validate the packaged module
 
-echo "Building and installing package"
-npm pack
-npm install -g solid-community-server-*.tgz
+TEST_NAME="Deployment test: packaged module"
+
+echo "$TEST_NAME - Building and installing package"
+npm pack --loglevel warn
+npm install -g solid-community-server-*.tgz --loglevel warn
 rm solid-community-server-*.tgz
 
-echo "Starting the server"
-community-solid-server -p 8888 &
+echo "$TEST_NAME - Starting the server"
+community-solid-server -p 8888 -l warn &
 PID=$!
 
-echo "Attempting access over HTTP"
 FAILURE=1
 if [ -z $PID ]; then
-  echo "Server failed to start"
+  echo "$TEST_NAME - Failure: Server did not start"
 else
+  echo "$TEST_NAME - Attempting HTTP access to the server"
   for i in {1..10}; do
     sleep 1
-    if curl -s -f localhost:8888; then
-      echo "Server reached"
+    if curl -s -f localhost:8888 > /dev/null; then
+      echo "$TEST_NAME - Success: server reached"
       FAILURE=0
       break
     fi
   done
+  if [ $FAILURE -eq 1 ]; then
+    echo "$TEST_NAME - Failure: Could not reach server"
+  fi
   kill -9 $PID
 fi
 
-echo "Uninstalling package"
+echo "$TEST_NAME - Cleanup"
 npm uninstall -g @solid/community-server
 
 exit $FAILURE
diff --git a/test/integration/LdpHandlerWithAuth.test.ts b/test/integration/LdpHandlerWithAuth.test.ts
index 4034a3da2..7c192084d 100644
--- a/test/integration/LdpHandlerWithAuth.test.ts
+++ b/test/integration/LdpHandlerWithAuth.test.ts
@@ -64,22 +64,23 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te
     await aclHelper.setSimpleAcl({ read: true, write: true, append: true, control: false }, 'agent');
 
     // Create file
+    const filePath = 'testfile2.txt';
+    const fileUrl = `${BASE}/${filePath}`;
     let response = await resourceHelper.createResource(
-      '../assets/testfile2.txt', 'testfile2.txt', 'text/plain',
+      '../assets/testfile2.txt', filePath, 'text/plain',
     );
-    const id = response._getHeaders().location;
 
     // Get file
-    response = await resourceHelper.getResource(id);
+    response = await resourceHelper.getResource(fileUrl);
     expect(response.statusCode).toBe(200);
     expect(response._getBuffer().toString()).toContain('TESTFILE2');
     expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
-    expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
+    expect(response.getHeaders().link).toContain(`<${fileUrl}.acl>; rel="acl"`);
     expect(response.getHeaders()['wac-allow']).toBe('user="read write append",public="read write append"');
 
     // DELETE file
-    await resourceHelper.deleteResource(id);
-    await resourceHelper.shouldNotExist(id);
+    await resourceHelper.deleteResource(fileUrl);
+    await resourceHelper.shouldNotExist(fileUrl);
   });
 
   it('can not add a file to the store if not allowed.', async(): Promise<void> => {
@@ -87,8 +88,9 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te
     await aclHelper.setSimpleAcl({ read: true, write: true, append: true, control: false }, 'authenticated');
 
     // Try to create file
+    const filePath = 'testfile2.txt';
     const response = await resourceHelper.createResource(
-      '../assets/testfile2.txt', 'testfile2.txt', 'text/plain', true,
+      '../assets/testfile2.txt', filePath, 'text/plain', true,
     );
     expect(response.statusCode).toBe(401);
   });
@@ -98,8 +100,9 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te
     await aclHelper.setSimpleAcl({ read: true, write: false, append: false, control: false }, 'agent');
 
     // Try to create file
+    const filePath = 'testfile2.txt';
     let response = await resourceHelper.createResource(
-      '../assets/testfile2.txt', 'testfile2.txt', 'text/plain', true,
+      '../assets/testfile2.txt', filePath, 'text/plain', true,
     );
     expect(response.statusCode).toBe(401);
 
@@ -120,17 +123,21 @@ describe.each(stores)('An LDP handler with auth using %s', (name, { storeUrn, te
     await aclHelper.setSimpleAcl({ read: true, write: false, append: true, control: false }, 'agent');
 
     // Add a file
-    let response = await resourceHelper.createResource(
-      '../assets/testfile2.txt', 'testfile2.txt', 'text/plain', true,
+    const filePath = 'testfile2.txt';
+    let response = await resourceHelper.performRequestWithBody(
+      new URL(`${BASE}/`),
+      'POST',
+      {
+        'content-type': 'text/plain',
+        'transfer-encoding': 'chunked',
+        slug: filePath,
+      },
+      Buffer.from('data'),
     );
     expect(response.statusCode).toBe(201);
 
-    const id = response._getHeaders().location;
-    response = await resourceHelper.performRequestWithBody(
-      new URL(id),
-      'PUT',
-      { 'content-type': 'text/plain', 'transfer-encoding': 'chunked' },
-      Buffer.from('data'),
+    response = await resourceHelper.createResource(
+      '../assets/testfile2.txt', filePath, 'text/plain', true,
     );
     expect(response.statusCode).toBe(401);
   });
diff --git a/test/integration/LdpHandlerWithoutAuth.test.ts b/test/integration/LdpHandlerWithoutAuth.test.ts
index 5f52a0e6e..be960b722 100644
--- a/test/integration/LdpHandlerWithoutAuth.test.ts
+++ b/test/integration/LdpHandlerWithoutAuth.test.ts
@@ -90,187 +90,203 @@ describe.each(stores)('An LDP handler without auth using %s', (name, { storeUrn,
 
   it('can add a file to the store, read it and delete it.', async():
   Promise<void> => {
-    // POST
+    const filePath = 'testfile0.txt';
+    const fileUrl = `${BASE}/${filePath}`;
+    // PUT
     let response = await resourceHelper.createResource(
-      '../assets/testfile0.txt', 'testfile0.txt', 'text/plain',
+      '../assets/testfile0.txt', filePath, 'text/plain',
     );
-    const id = response._getHeaders().location;
 
     // GET
-    response = await resourceHelper.getResource(id);
+    response = await resourceHelper.getResource(fileUrl);
     expect(response.statusCode).toBe(200);
     expect(response._getBuffer().toString()).toContain('TESTFILE0');
     expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
-    expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
+    expect(response.getHeaders().link).toContain(`<${fileUrl}.acl>; rel="acl"`);
     expect(response.getHeaders()['accept-patch']).toBe('application/sparql-update');
     expect(response.getHeaders()['ms-author-via']).toBe('SPARQL');
 
     // DELETE
-    await resourceHelper.deleteResource(id);
-    await resourceHelper.shouldNotExist(id);
+    await resourceHelper.deleteResource(fileUrl);
+    await resourceHelper.shouldNotExist(fileUrl);
   });
 
   it('can add and overwrite a file.', async(): Promise<void> => {
+    const filePath = 'file.txt';
+    const fileUrl = `${BASE}/${filePath}`;
+    // PUT
     let response = await resourceHelper.createResource(
-      '../assets/testfile0.txt', 'file.txt', 'text/plain',
+      '../assets/testfile0.txt', filePath, 'text/plain',
     );
-    const id = response._getHeaders().location;
 
     // GET
-    response = await resourceHelper.getResource(id);
+    response = await resourceHelper.getResource(fileUrl);
     expect(response.statusCode).toBe(200);
     expect(response._getBuffer().toString()).toContain('TESTFILE0');
     expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
-    expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
+    expect(response.getHeaders().link).toContain(`<${fileUrl}.acl>; rel="acl"`);
 
     // PUT
     response = await resourceHelper.replaceResource(
-      '../assets/testfile1.txt', id, 'text/plain',
+      '../assets/testfile1.txt', fileUrl, 'text/plain',
     );
 
     // GET
-    response = await resourceHelper.getResource(id);
+    response = await resourceHelper.getResource(fileUrl);
     expect(response.statusCode).toBe(200);
     expect(response._getBuffer().toString()).toContain('TESTFILE1');
     expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
-    expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
+    expect(response.getHeaders().link).toContain(`<${fileUrl}.acl>; rel="acl"`);
 
     // DELETE
-    await resourceHelper.deleteResource(id);
-    await resourceHelper.shouldNotExist(id);
+    await resourceHelper.deleteResource(fileUrl);
+    await resourceHelper.shouldNotExist(fileUrl);
   });
 
   it('can create a folder and delete it.', async(): Promise<void> => {
-    // POST
-    let response = await resourceHelper.createContainer('secondfolder/');
-    const id = response._getHeaders().location;
+    const containerPath = 'secondfolder/';
+    const containerUrl = `${BASE}/${containerPath}`;
+    // PUT
+    let response = await resourceHelper.createContainer(containerPath);
 
     // GET
-    response = await resourceHelper.getContainer(id);
+    response = await resourceHelper.getContainer(containerUrl);
     expect(response.statusCode).toBe(200);
     expect(response.getHeaders().link).toContain(`<${LDP.Container}>; rel="type"`);
     expect(response.getHeaders().link).toContain(`<${LDP.BasicContainer}>; rel="type"`);
     expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
-    expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
+    expect(response.getHeaders().link).toContain(`<${containerUrl}.acl>; rel="acl"`);
 
     // DELETE
-    await resourceHelper.deleteResource(id);
-    await resourceHelper.shouldNotExist(id);
+    await resourceHelper.deleteResource(containerUrl);
+    await resourceHelper.shouldNotExist(containerUrl);
   });
 
   it('can make a folder and put a file in it.', async(): Promise<void> => {
     // Create folder
-    await resourceHelper.createContainer('testfolder0/');
+    const containerPath = 'testfolder0/';
+    const containerUrl = `${BASE}/${containerPath}`;
+    await resourceHelper.createContainer(containerPath);
 
     // Create file
+    const filePath = 'testfolder0/testfile0.txt';
+    const fileUrl = `${BASE}/${filePath}`;
     let response = await resourceHelper.createResource(
-      '../assets/testfile0.txt', 'testfolder0/testfile0.txt', 'text/plain',
+      '../assets/testfile0.txt', filePath, 'text/plain',
     );
-    const id = response._getHeaders().location;
 
     // GET File
-    response = await resourceHelper.getResource(id);
+    response = await resourceHelper.getResource(fileUrl);
     expect(response.statusCode).toBe(200);
     expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
-    expect(response.getHeaders().link).toContain(`<${id}.acl>; rel="acl"`);
+    expect(response.getHeaders().link).toContain(`<${fileUrl}.acl>; rel="acl"`);
 
     // DELETE
-    await resourceHelper.deleteResource(id);
-    await resourceHelper.shouldNotExist(id);
-    await resourceHelper.deleteResource('http://test.com/testfolder0/');
-    await resourceHelper.shouldNotExist('http://test.com/testfolder0/');
+    await resourceHelper.deleteResource(fileUrl);
+    await resourceHelper.shouldNotExist(fileUrl);
+    await resourceHelper.deleteResource(containerUrl);
+    await resourceHelper.shouldNotExist(containerUrl);
   });
 
   it('cannot remove a folder when the folder contains a file.', async(): Promise<void> => {
     // Create folder
-    let response = await resourceHelper.createContainer('testfolder1/');
-    const folderId = response._getHeaders().location;
+    const containerPath = 'testfolder1/';
+    const containerUrl = `${BASE}/${containerPath}`;
+    let response = await resourceHelper.createContainer(containerPath);
 
     // Create file
+    const filePath = 'testfolder1/testfile0.txt';
+    const fileUrl = `${BASE}/${filePath}`;
     await resourceHelper.createResource(
-      '../assets/testfile0.txt', 'testfolder1/testfile0.txt', 'text/plain',
+      '../assets/testfile0.txt', filePath, 'text/plain',
     );
 
     // Try DELETE folder
-    response = await resourceHelper.performRequest(new URL(folderId), 'DELETE', {});
+    response = await resourceHelper.performRequest(new URL(containerUrl), 'DELETE', {});
     expect(response.statusCode).toBe(409);
     expect(response._getData()).toContain('ConflictHttpError: Can only delete empty containers.');
 
     // DELETE
-    await resourceHelper.deleteResource('http://test.com/testfolder1/testfile0.txt');
-    await resourceHelper.shouldNotExist('http://test.com/testfolder1/testfile0.txt');
-    await resourceHelper.deleteResource(folderId);
-    await resourceHelper.shouldNotExist(folderId);
+    await resourceHelper.deleteResource(fileUrl);
+    await resourceHelper.shouldNotExist(fileUrl);
+    await resourceHelper.deleteResource(containerUrl);
+    await resourceHelper.shouldNotExist(containerUrl);
   });
 
   it('cannot remove a folder when the folder contains a subfolder.', async(): Promise<void> => {
     // Create folder
-    let response = await resourceHelper.createContainer('testfolder2/');
-    const folderId = response._getHeaders().location;
+    const containerPath = 'testfolder2/';
+    const containerUrl = `${BASE}/${containerPath}`;
+    let response = await resourceHelper.createContainer(containerPath);
 
     // Create subfolder
-    response = await resourceHelper.createContainer('testfolder2/subfolder0');
-    const subFolderId = response._getHeaders().location;
+    const subContainerPath = `${containerPath}subfolder0/`;
+    const subContainerUrl = `${BASE}/${subContainerPath}`;
+    response = await resourceHelper.createContainer(subContainerPath);
 
     // Try DELETE folder
-    response = await resourceHelper.performRequest(new URL(folderId), 'DELETE', {});
+    response = await resourceHelper.performRequest(new URL(containerUrl), 'DELETE', {});
     expect(response.statusCode).toBe(409);
     expect(response._getData()).toContain('ConflictHttpError: Can only delete empty containers.');
 
     // DELETE
-    await resourceHelper.deleteResource(subFolderId);
-    await resourceHelper.shouldNotExist(subFolderId);
-    await resourceHelper.deleteResource(folderId);
-    await resourceHelper.shouldNotExist(folderId);
+    await resourceHelper.deleteResource(subContainerUrl);
+    await resourceHelper.shouldNotExist(subContainerUrl);
+    await resourceHelper.deleteResource(containerUrl);
+    await resourceHelper.shouldNotExist(containerUrl);
   });
 
   it('can read the contents of a folder.', async(): Promise<void> => {
     // Create folder
-    let response = await resourceHelper.createContainer('testfolder3/');
-    const folderId = response._getHeaders().location;
+    const containerPath = 'testfolder3/';
+    const containerUrl = `${BASE}/${containerPath}`;
+    let response = await resourceHelper.createContainer(containerPath);
 
     // Create subfolder
+    const subContainerPath = `${containerPath}subfolder0/`;
+    const subContainerUrl = `${BASE}/${subContainerPath}`;
     response = await resourceHelper.createContainer('testfolder3/subfolder0/');
-    const subFolderId = response._getHeaders().location;
 
     // Create file
+    const filePath = `${containerPath}testfile0.txt`;
+    const fileUrl = `${BASE}/${filePath}`;
     response = await resourceHelper.createResource(
-      '../assets/testfile0.txt', 'testfolder3/testfile0.txt', 'text/plain',
+      '../assets/testfile0.txt', filePath, 'text/plain',
     );
-    const fileId = response._getHeaders().location;
 
-    response = await resourceHelper.getContainer(folderId);
+    response = await resourceHelper.getContainer(containerUrl);
     expect(response.statusCode).toBe(200);
-    expect(response._getData()).toContain('<http://www.w3.org/ns/ldp#contains> <http://test.com/testfolder3/subfolder0/> .');
-    expect(response._getData()).toContain('<http://www.w3.org/ns/ldp#contains> <http://test.com/testfolder3/testfile0.txt> .');
+    expect(response._getData()).toContain(`<http://www.w3.org/ns/ldp#contains> <${subContainerUrl}> .`);
+    expect(response._getData()).toContain(`<http://www.w3.org/ns/ldp#contains> <${fileUrl}> .`);
     expect(response.getHeaders().link).toContain(`<${LDP.Container}>; rel="type"`);
     expect(response.getHeaders().link).toContain(`<${LDP.BasicContainer}>; rel="type"`);
     expect(response.getHeaders().link).toContain(`<${LDP.Resource}>; rel="type"`);
-    expect(response.getHeaders().link).toContain(`<${folderId}.acl>; rel="acl"`);
+    expect(response.getHeaders().link).toContain(`<${containerUrl}.acl>; rel="acl"`);
 
     // DELETE
-    await resourceHelper.deleteResource(fileId);
-    await resourceHelper.shouldNotExist(fileId);
-    await resourceHelper.deleteResource(subFolderId);
-    await resourceHelper.shouldNotExist(subFolderId);
-    await resourceHelper.deleteResource(folderId);
-    await resourceHelper.shouldNotExist(folderId);
+    await resourceHelper.deleteResource(fileUrl);
+    await resourceHelper.shouldNotExist(fileUrl);
+    await resourceHelper.deleteResource(subContainerUrl);
+    await resourceHelper.shouldNotExist(subContainerUrl);
+    await resourceHelper.deleteResource(containerUrl);
+    await resourceHelper.shouldNotExist(containerUrl);
   });
 
   it('can upload and delete a image.', async(): Promise<void> => {
+    const filePath = 'image.png';
+    const fileUrl = `${BASE}/${filePath}`;
     let response = await resourceHelper.createResource(
-      '../assets/testimage.png', 'image.png', 'image/png',
+      '../assets/testimage.png', filePath, 'image/png',
     );
-    const fileId = response._getHeaders().location;
 
     // GET
-    response = await resourceHelper.getResource(fileId);
+    response = await resourceHelper.getResource(fileUrl);
     expect(response.statusCode).toBe(200);
     expect(response._getHeaders()['content-type']).toBe('image/png');
 
     // DELETE
-    await resourceHelper.deleteResource(fileId);
-    await resourceHelper.shouldNotExist(fileId);
+    await resourceHelper.deleteResource(fileUrl);
+    await resourceHelper.shouldNotExist(fileUrl);
   });
 
   it('can create a container with a diamond identifier in the data.', async(): Promise<void> => {
diff --git a/test/integration/SparqlStorage.test.ts b/test/integration/SparqlStorage.test.ts
index 995bbc659..01ee11614 100644
--- a/test/integration/SparqlStorage.test.ts
+++ b/test/integration/SparqlStorage.test.ts
@@ -35,9 +35,8 @@ describeIf('docker', 'A server with a SPARQL endpoint as storage', (): void => {
 
   it('can add a Turtle file to the store.', async():
   Promise<void> => {
-    // POST
+    // PUT
     const response = await resourceHelper.createResource('../assets/person.ttl', 'person', 'text/turtle');
-    const id = response._getHeaders().location;
-    expect(id).toBeTruthy();
+    expect(response).toBeTruthy();
   });
 });
diff --git a/test/unit/storage/DataAccessorBasedStore.test.ts b/test/unit/storage/DataAccessorBasedStore.test.ts
index b0b40f2af..e9268df12 100644
--- a/test/unit/storage/DataAccessorBasedStore.test.ts
+++ b/test/unit/storage/DataAccessorBasedStore.test.ts
@@ -291,6 +291,24 @@ describe('A DataAccessorBasedStore', (): void => {
       });
     });
 
+    it('generates http://test.com/%26%26 when slug is &%26.', async(): Promise<void> => {
+      const resourceID = { path: root };
+      representation.metadata.removeAll(RDF.type);
+      representation.metadata.add(HTTP.slug, '&%26');
+      const result = await store.addResource(resourceID, representation);
+      expect(result).toEqual({ path: `${root}%26%26` });
+    });
+
+    it('errors if the slug contains a slash.', async(): Promise<void> => {
+      const resourceID = { path: root };
+      representation.metadata.removeAll(RDF.type);
+      representation.data = guardedStreamFrom([ `` ]);
+      representation.metadata.add(HTTP.slug, 'sla/sh/es');
+      const result = store.addResource(resourceID, representation);
+      await expect(result).rejects.toThrow(BadRequestHttpError);
+      await expect(result).rejects.toThrow('Slugs should not contain slashes');
+    });
+
     it('errors if the slug would cause an auxiliary resource URI to be generated.', async(): Promise<void> => {
       const resourceID = { path: root };
       representation.metadata.removeAll(RDF.type);
@@ -559,3 +577,4 @@ describe('A DataAccessorBasedStore', (): void => {
     });
   });
 });
+
diff --git a/test/unit/storage/patch/SparqlUpdatePatchHandler.test.ts b/test/unit/storage/patch/SparqlUpdatePatchHandler.test.ts
index 96f5381c2..67602d7e3 100644
--- a/test/unit/storage/patch/SparqlUpdatePatchHandler.test.ts
+++ b/test/unit/storage/patch/SparqlUpdatePatchHandler.test.ts
@@ -16,6 +16,7 @@ describe('A SparqlUpdatePatchHandler', (): void => {
   let handler: SparqlUpdatePatchHandler;
   let source: ResourceStore;
   let startQuads: Quad[];
+  const fullfilledDataInsert = 'INSERT DATA { :s1 :p1 :o1 . :s2 :p2 :o2 . }';
 
   beforeEach(async(): Promise<void> => {
     startQuads = [ quad(
@@ -56,6 +57,12 @@ describe('A SparqlUpdatePatchHandler', (): void => {
     return true;
   }
 
+  async function handle(query: string): Promise<void> {
+    const sparqlPrefix = 'prefix : <http://test.com/>\n';
+    return handler.handle({ identifier: { path: 'path' },
+      patch: { algebra: translate(sparqlPrefix.concat(query), { quads: true }) } as SparqlUpdatePatch });
+  }
+
   it('only accepts SPARQL updates.', async(): Promise<void> => {
     const input = { identifier: { path: 'path' },
       patch: { algebra: {}} as SparqlUpdatePatch };
@@ -65,12 +72,7 @@ describe('A SparqlUpdatePatchHandler', (): void => {
   });
 
   it('handles INSERT DATA updates.', async(): Promise<void> => {
-    await handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'INSERT DATA { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1>. ' +
-        '<http://test.com/s2> <http://test.com/p2> <http://test.com/o2> }',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
+    await handle(fullfilledDataInsert);
     expect(await basicChecks(startQuads.concat(
       [ quad(namedNode('http://test.com/s1'), namedNode('http://test.com/p1'), namedNode('http://test.com/o1')),
         quad(namedNode('http://test.com/s2'), namedNode('http://test.com/p2'), namedNode('http://test.com/o2')) ],
@@ -78,11 +80,7 @@ describe('A SparqlUpdatePatchHandler', (): void => {
   });
 
   it('handles DELETE DATA updates.', async(): Promise<void> => {
-    await handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'DELETE DATA { <http://test.com/startS1> <http://test.com/startP1> <http://test.com/startO1> }',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
+    await handle('DELETE DATA { :startS1 :startP1 :startO1 }');
     expect(await basicChecks(
       [ quad(namedNode('http://test.com/startS2'),
         namedNode('http://test.com/startP2'),
@@ -91,11 +89,8 @@ describe('A SparqlUpdatePatchHandler', (): void => {
   });
 
   it('handles DELETE WHERE updates with no variables.', async(): Promise<void> => {
-    await handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'DELETE WHERE { <http://test.com/startS1> <http://test.com/startP1> <http://test.com/startO1> }',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
+    const query = 'DELETE WHERE { :startS1 :startP1 :startO1 }';
+    await handle(query);
     expect(await basicChecks(
       [ quad(namedNode('http://test.com/startS2'),
         namedNode('http://test.com/startP2'),
@@ -104,13 +99,8 @@ describe('A SparqlUpdatePatchHandler', (): void => {
   });
 
   it('handles DELETE/INSERT updates with empty WHERE.', async(): Promise<void> => {
-    await handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'DELETE { <http://test.com/startS1> <http://test.com/startP1> <http://test.com/startO1> }\n' +
-        'INSERT { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1>. }\n' +
-        'WHERE {}',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
+    const query = 'DELETE { :startS1 :startP1 :startO1 } INSERT { :s1 :p1 :o1 . } WHERE {}';
+    await handle(query);
     expect(await basicChecks([
       quad(namedNode('http://test.com/startS2'),
         namedNode('http://test.com/startP2'),
@@ -122,14 +112,9 @@ describe('A SparqlUpdatePatchHandler', (): void => {
   });
 
   it('handles composite INSERT/DELETE updates.', async(): Promise<void> => {
-    await handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'INSERT DATA { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1>. ' +
-        '<http://test.com/s2> <http://test.com/p2> <http://test.com/o2> };' +
-        'DELETE WHERE { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1>.' +
-        '<http://test.com/startS1> <http://test.com/startP1> <http://test.com/startO1> }',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
+    const query = 'INSERT DATA { :s1 :p1 :o1 . :s2 :p2 :o2 };' +
+      'DELETE WHERE { :s1 :p1 :o1 . :startS1 :startP1 :startO1 }';
+    await handle(query);
     expect(await basicChecks([
       quad(namedNode('http://test.com/startS2'),
         namedNode('http://test.com/startP2'),
@@ -141,14 +126,9 @@ describe('A SparqlUpdatePatchHandler', (): void => {
   });
 
   it('handles composite DELETE/INSERT updates.', async(): Promise<void> => {
-    await handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'DELETE DATA { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1>.' +
-        '<http://test.com/startS1> <http://test.com/startP1> <http://test.com/startO1> };' +
-        'INSERT DATA { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1>. ' +
-        '<http://test.com/s2> <http://test.com/p2> <http://test.com/o2> }',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
+    const query = 'DELETE DATA { :s1 :p1 :o1 . :startS1 :startP1 :startO1 } ;' +
+      'INSERT DATA { :s1 :p1 :o1 . :s2 :p2 :o2 }';
+    await handle(query);
     expect(await basicChecks([
       quad(namedNode('http://test.com/startS2'),
         namedNode('http://test.com/startP2'),
@@ -163,80 +143,49 @@ describe('A SparqlUpdatePatchHandler', (): void => {
   });
 
   it('rejects GRAPH inserts.', async(): Promise<void> => {
-    const handle = handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'INSERT DATA { GRAPH <http://test.com/graph> { ' +
-          '<http://test.com/startS1> <http://test.com/startP1> <http://test.com/startO1> } }',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
-    await expect(handle).rejects.toThrow('GRAPH statements are not supported');
+    const query = 'INSERT DATA { GRAPH :graph { :s1 :p1 :o1 } }';
+    await expect(handle(query)).rejects.toThrow(NotImplementedHttpError);
   });
 
   it('rejects GRAPH deletes.', async(): Promise<void> => {
-    const handle = handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'DELETE DATA { GRAPH <http://test.com/graph> { ' +
-          '<http://test.com/startS1> <http://test.com/startP1> <http://test.com/startO1> } }',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
-    await expect(handle).rejects.toThrow('GRAPH statements are not supported');
+    const query = 'DELETE DATA { GRAPH :graph { :s1 :p1 :o1 } }';
+    await expect(handle(query)).rejects.toThrow(NotImplementedHttpError);
   });
 
   it('rejects DELETE/INSERT updates with a non-empty WHERE.', async(): Promise<void> => {
-    const handle = handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'DELETE { <http://test.com/startS1> <http://test.com/startP1> <http://test.com/startO1> }\n' +
-        'INSERT { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1>. }\n' +
-        'WHERE { ?s ?p ?o }',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
-    await expect(handle).rejects.toThrow('WHERE statements are not supported');
+    const query = 'DELETE { :s1 :p1 :o1 } INSERT { :s1 :p1 :o1 } WHERE { ?s ?p ?o }';
+    await expect(handle(query)).rejects.toThrow(NotImplementedHttpError);
+  });
+
+  it('rejects INSERT WHERE updates with a UNION.', async(): Promise<void> => {
+    const query = 'INSERT { :s1 :p1 :o1 . } WHERE { { :s1 :p1 :o1 } UNION { :s1 :p1 :o2 } }';
+    await expect(handle(query)).rejects.toThrow(NotImplementedHttpError);
   });
 
   it('rejects DELETE WHERE updates with variables.', async(): Promise<void> => {
-    const handle = handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'DELETE WHERE { ?v <http://test.com/startP1> <http://test.com/startO1> }',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
-    await expect(handle).rejects.toThrow('WHERE statements are not supported');
+    const query = 'DELETE WHERE { ?v :startP1 :startO1 }';
+    await expect(handle(query)).rejects.toThrow(NotImplementedHttpError);
   });
 
   it('rejects non-DELETE/INSERT updates.', async(): Promise<void> => {
-    const handle = handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'MOVE DEFAULT TO GRAPH <http://test.com/newGraph>',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
-    await expect(handle).rejects.toThrow('Only DELETE/INSERT SPARQL update operations are supported');
+    const query = 'MOVE DEFAULT TO GRAPH :newGraph';
+    await expect(handle(query)).rejects.toThrow(NotImplementedHttpError);
   });
 
   it('throws the error returned by the store if there is one.', async(): Promise<void> => {
     source.getRepresentation = jest.fn(async(): Promise<any> => {
       throw new Error('error');
     });
-
-    const input = { identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'INSERT DATA { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1>. ' +
-        '<http://test.com/s2> <http://test.com/p2> <http://test.com/o2> }',
-        { quads: true },
-      ) } as SparqlUpdatePatch };
-    await expect(handler.handle(input)).rejects.toThrow('error');
+    await expect(handle(fullfilledDataInsert)).rejects.toThrow('error');
   });
 
   it('creates a new resource if it does not exist yet.', async(): Promise<void> => {
-    // There is no initial data
     startQuads = [];
     source.getRepresentation = jest.fn((): any => {
       throw new NotFoundHttpError();
     });
-
-    await handler.handle({ identifier: { path: 'path' },
-      patch: { algebra: translate(
-        'INSERT DATA { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1>. }',
-        { quads: true },
-      ) } as SparqlUpdatePatch });
+    const query = 'INSERT DATA { <http://test.com/s1> <http://test.com/p1> <http://test.com/o1>. }';
+    await handle(query);
     expect(await basicChecks(startQuads.concat(
       [ quad(namedNode('http://test.com/s1'), namedNode('http://test.com/p1'), namedNode('http://test.com/o1')) ],
     ))).toBe(true);
diff --git a/test/util/TestHelpers.ts b/test/util/TestHelpers.ts
index 861efe773..4f2fd7417 100644
--- a/test/util/TestHelpers.ts
+++ b/test/util/TestHelpers.ts
@@ -2,7 +2,6 @@ import { EventEmitter } from 'events';
 import { promises as fs } from 'fs';
 import type { IncomingHttpHeaders } from 'http';
 import { Readable } from 'stream';
-import * as url from 'url';
 import type { MockResponse } from 'node-mocks-http';
 import { createResponse } from 'node-mocks-http';
 import type { ResourceStore, PermissionSet, HttpHandler, HttpRequest } from '../../src/';
@@ -93,24 +92,22 @@ export class ResourceHelper {
     return response;
   }
 
-  public async createResource(fileLocation: string, slug: string, contentType: string, mayFail = false):
+  public async createResource(fileLocation: string, path: string, contentType: string, mayFail = false):
   Promise<MockResponse<any>> {
     const fileData = await fs.readFile(
       joinFilePath(__dirname, fileLocation),
     );
 
     const response: MockResponse<any> = await this.performRequestWithBody(
-      this.baseUrl,
-      'POST',
+      new URL(path, this.baseUrl),
+      'PUT',
       { 'content-type': contentType,
-        slug,
         'transfer-encoding': 'chunked' },
       fileData,
     );
     if (!mayFail) {
-      expect(response.statusCode).toBe(201);
+      expect(response.statusCode).toBe(205);
       expect(response._getData()).toHaveLength(0);
-      expect(response._getHeaders().location).toContain(url.format(this.baseUrl));
     }
     return response;
   }
@@ -153,20 +150,18 @@ export class ResourceHelper {
     return response;
   }
 
-  public async createContainer(slug: string): Promise<MockResponse<any>> {
+  public async createContainer(path: string): Promise<MockResponse<any>> {
     const response: MockResponse<any> = await this.performRequest(
-      this.baseUrl,
-      'POST',
+      new URL(path, this.baseUrl),
+      'PUT',
       {
-        slug,
         link: '<http://www.w3.org/ns/ldp#Container>; rel="type"',
         'content-type': 'text/turtle',
         'transfer-encoding': 'chunked',
       },
     );
-    expect(response.statusCode).toBe(201);
+    expect(response.statusCode).toBe(205);
     expect(response._getData()).toHaveLength(0);
-    expect(response._getHeaders().location).toContain(url.format(this.baseUrl));
     return response;
   }