From 91ed72ef37749d5ba847beab97a340f5e8f1500e Mon Sep 17 00:00:00 2001 From: Wouter Termont Date: Wed, 6 Apr 2022 17:09:26 +0200 Subject: [PATCH] chore: RedirectingHttpHandler with RedirectAllHttpHandler Signed-off-by: Wouter Termont --- config/http/handler/handlers/redirect.json | 5 +- src/server/RedirectingHttpHandler.ts | 93 +++++++++++----- .../server/RedirectingHttpHandler.test.ts | 105 ++++++++++++------ 3 files changed, 144 insertions(+), 59 deletions(-) diff --git a/config/http/handler/handlers/redirect.json b/config/http/handler/handlers/redirect.json index b47d98f87..4a715487b 100644 --- a/config/http/handler/handlers/redirect.json +++ b/config/http/handler/handlers/redirect.json @@ -2,7 +2,7 @@ "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^3.0.0/components/context.jsonld", "@graph": [ { - "comment": "Example handler to configure redirect patterns. Add to urn:solid-server:default:HttpHandler", + "comment": "Example handler to configure redirect patterns.", "@id": "urn:solid-server:default:RedirectHandler", "@type": "RedirectingHttpHandler", "redirects": [ @@ -11,6 +11,9 @@ "RedirectingHttpHandler:_redirects_value": "/to/$1" } ], + "baseUrl": { "@id": "urn:solid-server:default:variable:baseUrl" }, + "targetExtractor": { "@id": "urn:solid-server:default:TargetExtractor" }, + "responseWriter": { "@id": "urn:solid-server:default:ResponseWriter" }, "statusCode": "303" } ] diff --git a/src/server/RedirectingHttpHandler.ts b/src/server/RedirectingHttpHandler.ts index f922f41fb..dcdd7f160 100644 --- a/src/server/RedirectingHttpHandler.ts +++ b/src/server/RedirectingHttpHandler.ts @@ -1,65 +1,106 @@ +import type { TargetExtractor } from '../http/input/identifier/TargetExtractor'; +import { RedirectResponseDescription } from '../http/output/response/RedirectResponseDescription'; +import type { ResponseWriter } from '../http/output/ResponseWriter'; import { getLoggerFor } from '../logging/LogUtil'; +import { FoundHttpError } from '../util/errors/FoundHttpError'; +import { MovedPermanentlyHttpError } from '../util/errors/MovedPermanentlyHttpError'; import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError'; +import { PermanentRedirectHttpError } from '../util/errors/PermanentRedirectHttpError'; +import type { RedirectHttpError } from '../util/errors/RedirectHttpError'; +import { SeeOtherHttpError } from '../util/errors/SeeOtherHttpError'; +import { TemporaryRedirectHttpError } from '../util/errors/TemporaryRedirectHttpError'; +import { getRelativeUrl, joinUrl } from '../util/PathUtil'; import type { HttpHandlerInput } from './HttpHandler'; import { HttpHandler } from './HttpHandler'; +import type { HttpRequest } from './HttpRequest'; +const redirectErrorFactories: Record<301 | 302 | 303 | 307 | 308, (location: string) => RedirectHttpError> = { + 301: (location: string): RedirectHttpError => new MovedPermanentlyHttpError(location), + 302: (location: string): RedirectHttpError => new FoundHttpError(location), + 303: (location: string): RedirectHttpError => new SeeOtherHttpError(location), + 307: (location: string): RedirectHttpError => new TemporaryRedirectHttpError(location), + 308: (location: string): RedirectHttpError => new PermanentRedirectHttpError(location), +}; + +/** + * Handler that redirects paths matching given patterns + * to their corresponding URL, substituting selected groups. + */ export class RedirectingHttpHandler extends HttpHandler { private readonly logger = getLoggerFor(this); - private readonly matcher: RegExp; - private readonly redirects: { pattern: RegExp; redirect: string }[] = []; + private readonly redirects: { + regex: RegExp; + redirectPattern: string; + }[]; /** * Creates a handler for the provided redirects. * @param redirects - A mapping between URL patterns. */ - public constructor(redirects: Record, private readonly statusCode: number = 308) { + public constructor( + redirects: Record, + private readonly baseUrl: string, + private readonly targetExtractor: TargetExtractor, + private readonly responseWriter: ResponseWriter, + private readonly statusCode: 301 | 302 | 303 | 307 | 308 = 308, + ) { super(); - const patterns = Object.keys(redirects); - - // Create a single regexp for quick checks - this.matcher = new RegExp(`^${patterns.join('|')}$`, 'u'); // Create an array of (regexp, redirect) pairs - this.redirects = patterns.map( - (pattern): { pattern: RegExp; redirect: string } => ({ - pattern: new RegExp(pattern, 'u'), - redirect: redirects[pattern], + this.redirects = Object.keys(redirects).map( + (pattern): { regex: RegExp; redirectPattern: string } => ({ + regex: new RegExp(pattern, 'u'), + redirectPattern: redirects[pattern], }), ); } public async canHandle({ request }: HttpHandlerInput): Promise { - // Only return if a redirect is configured for the requested URL - if (!this.matcher.test(request.url ?? '')) { - throw new NotImplementedHttpError(`No redirect configured for ${request.url}`); - } + // Try to find redirect for target URL + await this.findRedirect(request); } public async handle({ request, response }: HttpHandlerInput): Promise { - const { url } = request; + // Try to find redirect for target URL + const redirect = await this.findRedirect(request); + + // Send redirect response + this.logger.info(`Redirecting ${request.url} to ${redirect}`); + const result = new RedirectResponseDescription(redirectErrorFactories[this.statusCode](redirect)); + await this.responseWriter.handleSafe({ response, result }); + } + + private async findRedirect(request: HttpRequest): Promise { + // Retrieve target relative to base URL + const target = await getRelativeUrl(this.baseUrl, request, this.targetExtractor); // Get groups and redirect of first matching pattern let result; - for (const { pattern, redirect } of this.redirects) { - const match = pattern.exec(url ?? ''); + for (const { regex, redirectPattern } of this.redirects) { + const match = regex.exec(target); if (match) { - result = { match, redirect }; + result = { match, redirectPattern }; break; } } // Only return if a redirect is configured for the requested URL if (!result) { - throw new NotImplementedHttpError(`No redirect configured for ${url}`); + throw new NotImplementedHttpError(`No redirect configured for ${target}`); } // Build redirect URL from regexp result - const { match, redirect } = result; - const redirectUrl = match.reduce((prev, param, index): string => prev.replace(`$${index}`, param), redirect); + const { match, redirectPattern } = result; + const redirect = match.reduce( + (prev, param, index): string => prev.replace(`$${index}`, param), + redirectPattern, + ); - // Send redirect response - this.logger.info(`Redirecting ${url} to ${redirectUrl}`); - response.writeHead(this.statusCode, { location: redirectUrl }); - response.end(); + // Don't redirect if target is already correct + if (redirect === target) { + throw new NotImplementedHttpError('Target is already correct.'); + } + + return /^(?:[a-z]+:)?\/\//ui.test(redirect) ? redirect : joinUrl(this.baseUrl, redirect); } } diff --git a/test/unit/server/RedirectingHttpHandler.test.ts b/test/unit/server/RedirectingHttpHandler.test.ts index 5dd8df158..585bf7e28 100644 --- a/test/unit/server/RedirectingHttpHandler.test.ts +++ b/test/unit/server/RedirectingHttpHandler.test.ts @@ -1,56 +1,97 @@ -import { EventEmitter } from 'events'; -import { createResponse } from 'node-mocks-http'; +import type { TargetExtractor } from '../../../src/http/input/identifier/TargetExtractor'; +import type { ResponseWriter } from '../../../src/http/output/ResponseWriter'; +import type { ResourceIdentifier } from '../../../src/http/representation/ResourceIdentifier'; +import type { HttpRequest } from '../../../src/server/HttpRequest'; +import type { HttpResponse } from '../../../src/server/HttpResponse'; import { RedirectingHttpHandler } from '../../../src/server/RedirectingHttpHandler'; +import { joinUrl } from '../../../src/util/PathUtil'; +import { SOLID_HTTP } from '../../../src/util/Vocabularies'; describe('A RedirectingHttpHandler', (): void => { - const handler = new RedirectingHttpHandler({ - '/one': '/two', - '/from/(.*)': '/to/$1', - '/f([aeiou]+)/b([aeiou]+)r': '/f$2/b$1r', + const baseUrl = 'http://test.com/'; + const request = { method: 'GET' } as HttpRequest; + const response = {} as HttpResponse; + let targetExtractor: jest.Mocked; + let responseWriter: jest.Mocked; + let handler: RedirectingHttpHandler; + + beforeEach(async(): Promise => { + targetExtractor = { + handleSafe: jest.fn(({ request: req }): ResourceIdentifier => ({ path: joinUrl(baseUrl, req.url!) })), + } as any; + + responseWriter = { handleSafe: jest.fn() } as any; + + handler = new RedirectingHttpHandler({ + '/one': '/two', + '/from/(.*)': 'http://to/t$1', + '/f([aeiou]+)/b([aeiou]+)r': '/f$2/b$1r', + '/s(.)me': '/s$1me', + }, baseUrl, targetExtractor, responseWriter); }); afterEach(jest.clearAllMocks); it('does not handle requests without URL.', async(): Promise => { - const request = { method: 'GET' }; - await expect(handler.canHandle({ request } as any)).rejects - .toThrow('No redirect configured for undefined'); - await expect(handler.handle({ request } as any)).rejects - .toThrow('No redirect configured for undefined'); + await expect(handler.canHandle({ request, response })) + .rejects.toThrow('Url must be a string. Received undefined'); + await expect(handler.handle({ request, response })) + .rejects.toThrow('Url must be a string. Received undefined'); }); it('does not handle requests with unconfigured URLs.', async(): Promise => { - const request = { method: 'GET', url: '/other' }; - await expect(handler.canHandle({ request } as any)).rejects - .toThrow('No redirect configured for /other'); - await expect(handler.handle({ request } as any)).rejects - .toThrow('No redirect configured for /other'); + request.url = '/other'; + await expect(handler.canHandle({ request, response })) + .rejects.toThrow('No redirect configured for /other'); + await expect(handler.handle({ request, response })) + .rejects.toThrow('No redirect configured for /other'); + }); + + it('does not handle requests redirecting to their own target URL.', async(): Promise => { + request.url = '/same'; + await expect(handler.canHandle({ request, response })) + .rejects.toThrow('Target is already correct.'); + await expect(handler.handle({ request, response })) + .rejects.toThrow('Target is already correct.'); }); it('handles requests to a known URL.', async(): Promise => { - const request = { method: 'GET', url: '/one' }; - const response = createResponse({ eventEmitter: EventEmitter }); - await handler.handleSafe({ request, response } as any); + request.url = '/one'; - expect(response.statusCode).toBe(308); - expect(response.getHeaders()).toHaveProperty('location', '/two'); + await expect(handler.handle({ request, response })).resolves.toBeUndefined(); + expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1); + expect(responseWriter.handleSafe).toHaveBeenLastCalledWith({ + response, + result: expect.objectContaining({ statusCode: 308 }), + }); + const { metadata } = responseWriter.handleSafe.mock.calls[0][0].result; + expect(metadata?.get(SOLID_HTTP.terms.location)?.value).toBe(joinUrl(baseUrl, '/two')); }); it('handles correctly substitutes group patterns.', async(): Promise => { - const request = { method: 'GET', url: '/fa/boor' }; - const response = createResponse({ eventEmitter: EventEmitter }); - await handler.handleSafe({ request, response } as any); + request.url = '/fa/boor'; - expect(response.getHeaders()).toHaveProperty('location', '/foo/bar'); + await handler.handle({ request, response }); + const { metadata } = responseWriter.handleSafe.mock.calls[0][0].result; + expect(metadata?.get(SOLID_HTTP.terms.location)?.value).toBe(joinUrl(baseUrl, '/foo/bar')); }); - it('redirects with the provided status code.', async(): Promise => { - const seeOtherHandler = new RedirectingHttpHandler({ '/one': '/two' }, 303); - const request = { method: 'GET', url: '/one' }; - const response = createResponse({ eventEmitter: EventEmitter }); - await seeOtherHandler.handleSafe({ request, response } as any); + it('redirects to an absolute url if provided.', async(): Promise => { + request.url = '/from/here'; - expect(response.statusCode).toBe(303); - expect(response.getHeaders()).toHaveProperty('location', '/two'); + await handler.handle({ request, response }); + const { metadata } = responseWriter.handleSafe.mock.calls[0][0].result; + expect(metadata?.get(SOLID_HTTP.terms.location)?.value).toBe('http://to/there'); + }); + + it.each([ 301, 302, 303, 307, 308 ])('redirects with the provided status code: %i.', async(code): Promise => { + request.url = '/one'; + (handler as any).statusCode = code; + + await handler.handle({ request, response }); + expect(responseWriter.handleSafe).toHaveBeenLastCalledWith({ + response, + result: expect.objectContaining({ statusCode: code }), + }); }); });