From a57105be8e08f8b39bd827a56fc6cf14d4425419 Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Thu, 28 Jan 2021 22:36:26 +0100
Subject: [PATCH] change: Query string does not influence identifier.

---
 config/presets/ldp/request-parser.json          |  3 ++-
 src/ldp/http/OriginalUrlExtractor.ts            | 17 +++++++++++++++--
 test/integration/LdpHandlerWithoutAuth.test.ts  | 12 ++++++++++++
 test/unit/ldp/http/OriginalUrlExtractor.test.ts | 11 +++++++++++
 test/util/TestHelpers.ts                        |  3 +--
 test/util/Util.ts                               |  2 +-
 6 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/config/presets/ldp/request-parser.json b/config/presets/ldp/request-parser.json
index 07efda2f8..80c253aef 100644
--- a/config/presets/ldp/request-parser.json
+++ b/config/presets/ldp/request-parser.json
@@ -27,7 +27,8 @@
     },
     {
       "@id": "urn:solid-server:default:TargetExtractor",
-      "@type": "OriginalUrlExtractor"
+      "@type": "OriginalUrlExtractor",
+      "OriginalUrlExtractor:_options_includeQueryString": false
     }
   ]
 }
diff --git a/src/ldp/http/OriginalUrlExtractor.ts b/src/ldp/http/OriginalUrlExtractor.ts
index 57306aa27..ea36dde8c 100644
--- a/src/ldp/http/OriginalUrlExtractor.ts
+++ b/src/ldp/http/OriginalUrlExtractor.ts
@@ -9,6 +9,13 @@ import { TargetExtractor } from './TargetExtractor';
  * Reconstructs the original URL of an incoming {@link HttpRequest}.
  */
 export class OriginalUrlExtractor extends TargetExtractor {
+  private readonly includeQueryString: boolean;
+
+  public constructor(options: { includeQueryString?: boolean } = {}) {
+    super();
+    this.includeQueryString = options.includeQueryString ?? true;
+  }
+
   public async handle({ request: { url, connection, headers }}: { request: HttpRequest }): Promise<ResourceIdentifier> {
     if (!url) {
       throw new Error('Missing URL');
@@ -37,7 +44,13 @@ export class OriginalUrlExtractor extends TargetExtractor {
 
     // URL object applies punycode encoding to domain
     const base = `${protocol}://${host}`;
-    const path = new URL(toCanonicalUriPath(url), base).href;
-    return { path };
+    const originalUrl = new URL(toCanonicalUriPath(url), base);
+
+    // Drop the query string if requested
+    if (!this.includeQueryString) {
+      originalUrl.search = '';
+    }
+
+    return { path: originalUrl.href };
   }
 }
diff --git a/test/integration/LdpHandlerWithoutAuth.test.ts b/test/integration/LdpHandlerWithoutAuth.test.ts
index 92ecd9355..3c2abba65 100644
--- a/test/integration/LdpHandlerWithoutAuth.test.ts
+++ b/test/integration/LdpHandlerWithoutAuth.test.ts
@@ -69,6 +69,18 @@ describe.each(stores)('An LDP handler without auth using %s', (name, { storeUrn,
     expect(response.getHeaders().link).toContain(`<${BASE}/.acl>; rel="acl"`);
   });
 
+  it('can read a folder listing with a query string.', async():
+  Promise<void> => {
+    const response = await resourceHelper.getResource(`${BASE}/?abc=def&xyz`);
+    expect(response.statusCode).toBe(200);
+    expect(response.getHeaders()).toHaveProperty('content-type', 'text/turtle');
+
+    const data = response._getData().toString();
+    expect(data).toContain(`<> a ldp:Container`);
+    expect(response.getHeaders().link).toContain(`<${LDP.Container}>; rel="type"`);
+    expect(response.getHeaders().link).toContain(`<${BASE}/.acl>; rel="acl"`);
+  });
+
   it('can add a file to the store, read it and delete it.', async():
   Promise<void> => {
     // POST
diff --git a/test/unit/ldp/http/OriginalUrlExtractor.test.ts b/test/unit/ldp/http/OriginalUrlExtractor.test.ts
index 408e63925..83cdc664d 100644
--- a/test/unit/ldp/http/OriginalUrlExtractor.test.ts
+++ b/test/unit/ldp/http/OriginalUrlExtractor.test.ts
@@ -26,6 +26,17 @@ describe('A OriginalUrlExtractor', (): void => {
       .resolves.toEqual({ path: 'http://test.com/url' });
   });
 
+  it('returns an input URL with query string.', async(): Promise<void> => {
+    const noQuery = new OriginalUrlExtractor({ includeQueryString: false });
+    await expect(noQuery.handle({ request: { url: '/url?abc=def&xyz', headers: { host: 'test.com' }} as any }))
+      .resolves.toEqual({ path: 'http://test.com/url' });
+  });
+
+  it('drops the query string when includeQueryString is set to false.', async(): Promise<void> => {
+    await expect(extractor.handle({ request: { url: '/url?abc=def&xyz', headers: { host: 'test.com' }} as any }))
+      .resolves.toEqual({ path: 'http://test.com/url?abc=def&xyz' });
+  });
+
   it('supports host:port combinations.', async(): Promise<void> => {
     await expect(extractor.handle({ request: { url: 'url', headers: { host: 'localhost:3000' }} as any }))
       .resolves.toEqual({ path: 'http://localhost:3000/url' });
diff --git a/test/util/TestHelpers.ts b/test/util/TestHelpers.ts
index 606e448a4..39e3c1f9e 100644
--- a/test/util/TestHelpers.ts
+++ b/test/util/TestHelpers.ts
@@ -73,8 +73,7 @@ export class ResourceHelper {
     data: Buffer,
   ): Promise<MockResponse<any>> {
     const request = Readable.from([ data ]) as HttpRequest;
-
-    request.url = requestUrl.pathname;
+    request.url = `${requestUrl.pathname}${requestUrl.search}`;
     request.method = method;
     request.headers = headers;
     request.headers.host = requestUrl.host;
diff --git a/test/util/Util.ts b/test/util/Util.ts
index 9aa69afc8..1661d8e82 100644
--- a/test/util/Util.ts
+++ b/test/util/Util.ts
@@ -17,7 +17,7 @@ export async function performRequest(
   data: string[],
 ): Promise<MockResponse<any>> {
   const request = streamifyArray(data) as HttpRequest;
-  request.url = requestUrl.pathname;
+  request.url = `${requestUrl.pathname}${requestUrl.search}`;
   request.method = method;
   request.headers = headers;
   request.headers.host = requestUrl.host;