fix: Throw error when accessing URLs out of scope

* feat: allow OriginalUrlExtractor to be configured with an identifierStrategy. Included the existing parameter 'includeQueryString' in the OriginalUrlExtractorArgs type.

* test: fixed OriginalUrlExtractor instantiation in OriginalUrlExtractor and RequestParser tests

* fix: Server no longer warns when accessing a URL out of scope #1148

* test: updated tests broken by #1148 fix

* test: ensuring 100% coverage

Co-authored-by: Wannes Kerckhove <wannes.kerckhove@ugent.be>

config/ldp/authentication/dpop-bearer.json

@@ -12,7 +12,8 @@
             {
               "@type": "DPoPWebIdExtractor",
               "originalUrlExtractor": {
-                "@type": "OriginalUrlExtractor"
+                "@type": "OriginalUrlExtractor",
+                "args_identifierStrategy": { "@id": "urn:solid-server:default:IdentifierStrategy" }
               }
             },
             { "@type": "BearerWebIdExtractor" }

config/ldp/handler/components/request-parser.json

@@ -8,7 +8,8 @@
       "args_targetExtractor": {
         "@id": "urn:solid-server:default:TargetExtractor",
         "@type": "OriginalUrlExtractor",
-        "options_includeQueryString": false
+        "args_identifierStrategy": { "@id": "urn:solid-server:default:IdentifierStrategy" },
+        "args_includeQueryString": false
       },
       "args_preferenceParser": { "@type": "AcceptPreferenceParser" },
       "args_metadataParser": { "@id": "urn:solid-server:default:MetadataParser" },

src/http/input/identifier/OriginalUrlExtractor.ts

@@ -3,19 +3,34 @@ import type { HttpRequest } from '../../../server/HttpRequest';
 import { BadRequestHttpError } from '../../../util/errors/BadRequestHttpError';
 import { InternalServerError } from '../../../util/errors/InternalServerError';
 import { parseForwarded } from '../../../util/HeaderUtil';
+import type { IdentifierStrategy } from '../../../util/identifiers/IdentifierStrategy';
 import { toCanonicalUriPath } from '../../../util/PathUtil';
 import type { ResourceIdentifier } from '../../representation/ResourceIdentifier';
 import { TargetExtractor } from './TargetExtractor';
 
+export interface OriginalUrlExtractorArgs {
+  /**
+   * The IdentifierStrategy to use for checking the scope of the request
+   */
+  identifierStrategy: IdentifierStrategy;
+
+  /**
+   * Specify wether the OriginalUrlExtractor should include the request query string.
+   */
+  includeQueryString?: boolean;
+}
+
 /**
  * Reconstructs the original URL of an incoming {@link HttpRequest}.
  */
 export class OriginalUrlExtractor extends TargetExtractor {
+  private readonly identifierStrategy: IdentifierStrategy;
   private readonly includeQueryString: boolean;
 
-  public constructor(options: { includeQueryString?: boolean } = {}) {
+  public constructor(args: OriginalUrlExtractorArgs) {
     super();
-    this.includeQueryString = options.includeQueryString ?? true;
+    this.identifierStrategy = args.identifierStrategy;
+    this.includeQueryString = args.includeQueryString ?? true;
   }
 
   public async handle({ request: { url, connection, headers }}: { request: HttpRequest }): Promise<ResourceIdentifier> {
@@ -52,6 +67,15 @@ export class OriginalUrlExtractor extends TargetExtractor {
       originalUrl.search = search;
     }
 
-    return { path: originalUrl.href };
+    // Create ResourceIdentifier instance
+    const identifier = { path: originalUrl.href };
+
+    // Check if the configured IdentifierStrategy supports the identifier
+    if (!this.identifierStrategy.supportsIdentifier(identifier)) {
+      throw new InternalServerError(`The identifier ${identifier.path} is outside the configured identifier space.`,
+        { errorCode: 'E0001', details: { path: identifier.path }});
+    }
+
+    return identifier;
   }
 }

test/integration/RequestParser.test.ts

@@ -1,5 +1,6 @@
 import { Readable } from 'stream';
 import arrayifyStream from 'arrayify-stream';
+import { SingleRootIdentifierStrategy } from '../../src';
 import { BasicRequestParser } from '../../src/http/input/BasicRequestParser';
 import { RawBodyParser } from '../../src/http/input/body/RawBodyParser';
 import { BasicConditionsParser } from '../../src/http/input/conditions/BasicConditionsParser';
@@ -12,7 +13,8 @@ import { BasicConditions } from '../../src/storage/BasicConditions';
 import { guardedStreamFrom } from '../../src/util/StreamUtil';
 
 describe('A BasicRequestParser with simple input parsers', (): void => {
-  const targetExtractor = new OriginalUrlExtractor();
+  const identifierStrategy = new SingleRootIdentifierStrategy('http://test.com/');
+  const targetExtractor = new OriginalUrlExtractor({ identifierStrategy });
   const preferenceParser = new AcceptPreferenceParser();
   const metadataParser = new ContentTypeParser();
   const conditionsParser = new BasicConditionsParser();

test/unit/http/input/identifier/OriginalUrlExtractor.test.ts

@@ -1,7 +1,22 @@
+import { SingleRootIdentifierStrategy } from '../../../../../src';
 import { OriginalUrlExtractor } from '../../../../../src/http/input/identifier/OriginalUrlExtractor';
 
+// Utility interface for defining the createExtractor utility method arguments
+interface CreateExtractorArgs {
+  baseUrl?: string;
+  includeQueryString?: boolean;
+}
+
+// Helper function for instantiating an OriginalUrlExtractor
+function createExtractor(args: CreateExtractorArgs = { }): OriginalUrlExtractor {
+  const identifierStrategy = new SingleRootIdentifierStrategy(args.baseUrl ?? 'http://test.com');
+  const extractor = new OriginalUrlExtractor({ identifierStrategy, includeQueryString: args.includeQueryString });
+  return extractor;
+}
+
 describe('A OriginalUrlExtractor', (): void => {
-  const extractor = new OriginalUrlExtractor();
+  // Default extractor to use, some test cases may specify an alternative extractor
+  const extractor = createExtractor();
 
   it('can handle any input.', async(): Promise<void> => {
     await expect(extractor.canHandle({} as any)).resolves.toBeUndefined();
@@ -21,19 +36,24 @@ describe('A OriginalUrlExtractor', (): void => {
       .rejects.toThrow('The request has an invalid Host header: test.com/forbidden');
   });
 
+  it('errors if the request URL base does not match the configured baseUrl.', async(): Promise<void> => {
+    await expect(extractor.handle({ request: { url: 'url', headers: { host: 'example.com' }} as any }))
+      .rejects.toThrow(`The identifier http://example.com/url is outside the configured identifier space.`);
+  });
+
   it('returns the input URL.', async(): Promise<void> => {
     await expect(extractor.handle({ request: { url: 'url', headers: { host: 'test.com' }} as any }))
       .resolves.toEqual({ path: 'http://test.com/url' });
   });
 
   it('returns an input URL with query string.', async(): Promise<void> => {
-    const noQuery = new OriginalUrlExtractor({ includeQueryString: false });
+    const noQuery = createExtractor({ 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('returns an input URL with multiple leading slashes.', async(): Promise<void> => {
-    const noQuery = new OriginalUrlExtractor({ includeQueryString: true });
+    const noQuery = createExtractor({ includeQueryString: true });
     await expect(noQuery.handle({ request: { url: '///url?abc=def&xyz', headers: { host: 'test.com' }} as any }))
       .resolves.toEqual({ path: 'http://test.com///url?abc=def&xyz' });
   });
@@ -44,12 +64,14 @@ describe('A OriginalUrlExtractor', (): void => {
   });
 
   it('supports host:port combinations.', async(): Promise<void> => {
-    await expect(extractor.handle({ request: { url: 'url', headers: { host: 'localhost:3000' }} as any }))
+    const altExtractor = createExtractor({ baseUrl: 'http://localhost:3000/' });
+    await expect(altExtractor.handle({ request: { url: 'url', headers: { host: 'localhost:3000' }} as any }))
       .resolves.toEqual({ path: 'http://localhost:3000/url' });
   });
 
   it('uses https protocol if the connection is secure.', async(): Promise<void> => {
-    await expect(extractor.handle(
+    const altExtractor = createExtractor({ baseUrl: 'https://test.com/' });
+    await expect(altExtractor.handle(
       { request: { url: 'url', headers: { host: 'test.com' }, connection: { encrypted: true } as any } as any },
     )).resolves.toEqual({ path: 'https://test.com/url' });
   });
@@ -66,7 +88,8 @@ describe('A OriginalUrlExtractor', (): void => {
   });
 
   it('encodes hosts.', async(): Promise<void> => {
-    await expect(extractor.handle({ request: { url: '/', headers: { host: '點看' }} as any }))
+    const altExtractor = createExtractor({ baseUrl: 'http://xn--c1yn36f/' });
+    await expect(altExtractor.handle({ request: { url: '/', headers: { host: '點看' }} as any }))
       .resolves.toEqual({ path: 'http://xn--c1yn36f/' });
   });
 
@@ -80,60 +103,66 @@ describe('A OriginalUrlExtractor', (): void => {
   });
 
   it('takes the Forwarded header into account.', async(): Promise<void> => {
+    const altExtractor = createExtractor({ baseUrl: 'https://pod.example/' });
     const headers = {
       host: 'test.com',
       forwarded: 'proto=https;host=pod.example',
     };
-    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+    await expect(altExtractor.handle({ request: { url: '/foo/bar', headers } as any }))
       .resolves.toEqual({ path: 'https://pod.example/foo/bar' });
   });
 
   it('should fallback to x-fowarded-* headers.', async(): Promise<void> => {
+    const altExtractor = createExtractor({ baseUrl: 'https://pod.example/' });
     const headers = {
       host: 'test.com',
       'x-forwarded-host': 'pod.example',
       'x-forwarded-proto': 'https',
     };
-    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+    await expect(altExtractor.handle({ request: { url: '/foo/bar', headers } as any }))
       .resolves.toEqual({ path: 'https://pod.example/foo/bar' });
   });
 
   it('should just take x-forwarded-host if provided.', async(): Promise<void> => {
+    const altExtractor = createExtractor({ baseUrl: 'http://pod.example/' });
     const headers = {
       host: 'test.com',
       'x-forwarded-host': 'pod.example',
     };
-    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+    await expect(altExtractor.handle({ request: { url: '/foo/bar', headers } as any }))
       .resolves.toEqual({ path: 'http://pod.example/foo/bar' });
   });
 
   it('should just take x-forwarded-protocol if provided.', async(): Promise<void> => {
+    const altExtractor = createExtractor({ baseUrl: 'https://test.com/' });
     const headers = {
       host: 'test.com',
       'x-forwarded-proto': 'https',
     };
-    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+    await expect(altExtractor.handle({ request: { url: '/foo/bar', headers } as any }))
       .resolves.toEqual({ path: 'https://test.com/foo/bar' });
   });
 
   it('should prefer forwarded header to x-forwarded-* headers.', async(): Promise<void> => {
+    const altExtractor = createExtractor({ baseUrl: 'http://pod.example/' });
     const headers = {
       host: 'test.com',
       forwarded: 'proto=http;host=pod.example',
       'x-forwarded-proto': 'https',
       'x-forwarded-host': 'anotherpod.example',
     };
-    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+    await expect(altExtractor.handle({ request: { url: '/foo/bar', headers } as any }))
       .resolves.toEqual({ path: 'http://pod.example/foo/bar' });
   });
 
   it('should just take the first x-forwarded-* value.', async(): Promise<void> => {
+    const altExtractor = createExtractor({ baseUrl: 'http://pod.example/' });
     const headers = {
       host: 'test.com',
       'x-forwarded-host': 'pod.example, another.domain',
       'x-forwarded-proto': 'http,https',
     };
-    await expect(extractor.handle({ request: { url: '/foo/bar', headers } as any }))
+    await expect(altExtractor.handle({ request: { url: '/foo/bar', headers } as any }))
       .resolves.toEqual({ path: 'http://pod.example/foo/bar' });
   });
 });