From 30ee0f8dc6eb4bab4eae5282825a46882ca1626a Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Sun, 22 Nov 2020 21:58:19 +0100 Subject: [PATCH] chore: Clean up code related to headers. --- src/ldp/UnsecureWebSocketsProtocol.ts | 2 +- src/ldp/http/BasicTargetExtractor.ts | 16 +++++++++------- .../ldp/http/BasicTargetExtractor.test.ts | 17 ++++++++++++++++- .../metadata/LinkRelMetadataWriter.test.ts | 19 ++++--------------- .../metadata/MappedMetadataWriter.test.ts | 19 ++++--------------- 5 files changed, 34 insertions(+), 39 deletions(-) diff --git a/src/ldp/UnsecureWebSocketsProtocol.ts b/src/ldp/UnsecureWebSocketsProtocol.ts index 22ec4696e..7f0871ef8 100644 --- a/src/ldp/UnsecureWebSocketsProtocol.ts +++ b/src/ldp/UnsecureWebSocketsProtocol.ts @@ -95,7 +95,7 @@ class WebSocketListener extends EventEmitter { throw new Error(`Mismatched protocol: ${resolved.protocol} instead of ${this.protocol}`); } // Subscribe to the URL - const url = resolved.toString(); + const url = resolved.href; this.subscribedPaths.add(url); this.sendMessage('ack', url); this.logger.debug(`WebSocket subscribed to changes on ${url}`); diff --git a/src/ldp/http/BasicTargetExtractor.ts b/src/ldp/http/BasicTargetExtractor.ts index e9ca085af..84ed0cdcc 100644 --- a/src/ldp/http/BasicTargetExtractor.ts +++ b/src/ldp/http/BasicTargetExtractor.ts @@ -13,23 +13,25 @@ import { TargetExtractor } from './TargetExtractor'; export class BasicTargetExtractor extends TargetExtractor { protected readonly logger = getLoggerFor(this); - public async handle(request: HttpRequest): Promise { - if (!request.url) { + public async handle({ url, headers: { host }, connection }: HttpRequest): Promise { + if (!url) { this.logger.error('The request has no URL'); throw new Error('Missing URL'); } - if (!request.headers.host) { + if (!host) { this.logger.error('The request has no Host header'); throw new Error('Missing Host header'); } + if (/[/\\*]/u.test(host)) { + throw new Error(`The request has an invalid Host header: ${host}`); + } - const isHttps = request.connection && (request.connection as TLSSocket).encrypted; + const isHttps = (connection as TLSSocket)?.encrypted; this.logger.debug(`Request is using HTTPS: ${isHttps}`); // URL object applies punycode encoding to domain - const base = `http${isHttps ? 's' : ''}://${request.headers.host}`; - const url = toCanonicalUriPath(request.url); - const path = new URL(url, base).href; + const base = `http${isHttps ? 's' : ''}://${host}`; + const path = new URL(toCanonicalUriPath(url), base).href; return { path }; } diff --git a/test/unit/ldp/http/BasicTargetExtractor.test.ts b/test/unit/ldp/http/BasicTargetExtractor.test.ts index baea42e20..ad61f7751 100644 --- a/test/unit/ldp/http/BasicTargetExtractor.test.ts +++ b/test/unit/ldp/http/BasicTargetExtractor.test.ts @@ -15,18 +15,28 @@ describe('A BasicTargetExtractor', (): void => { await expect(extractor.handle({ url: 'url', headers: {}} as any)).rejects.toThrow('Missing Host header'); }); + it('errors if the host is invalid.', async(): Promise => { + await expect(extractor.handle({ url: 'url', headers: { host: 'test.com/forbidden' }} as any)) + .rejects.toThrow('The request has an invalid Host header: test.com/forbidden'); + }); + it('returns the input URL.', async(): Promise => { await expect(extractor.handle({ url: 'url', headers: { host: 'test.com' }} as any)) .resolves.toEqual({ path: 'http://test.com/url' }); }); + it('supports host:port combinations.', async(): Promise => { + await expect(extractor.handle({ 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 => { await expect(extractor.handle( { url: 'url', headers: { host: 'test.com' }, connection: { encrypted: true } as any } as any, )).resolves.toEqual({ path: 'https://test.com/url' }); }); - it('encodes relevant characters.', async(): Promise => { + it('encodes paths.', async(): Promise => { await expect(extractor.handle({ url: '/a%20path%26/name', headers: { host: 'test.com' }} as any)) .resolves.toEqual({ path: 'http://test.com/a%20path%26/name' }); @@ -36,4 +46,9 @@ describe('A BasicTargetExtractor', (): void => { await expect(extractor.handle({ url: '/path&%26/name', headers: { host: 'test.com' }} as any)) .resolves.toEqual({ path: 'http://test.com/path%26%26/name' }); }); + + it('encodes hosts.', async(): Promise => { + await expect(extractor.handle({ url: '/', headers: { host: '點看' }} as any)) + .resolves.toEqual({ path: 'http://xn--c1yn36f/' }); + }); }); diff --git a/test/unit/ldp/http/metadata/LinkRelMetadataWriter.test.ts b/test/unit/ldp/http/metadata/LinkRelMetadataWriter.test.ts index 302edd297..d33dcb340 100644 --- a/test/unit/ldp/http/metadata/LinkRelMetadataWriter.test.ts +++ b/test/unit/ldp/http/metadata/LinkRelMetadataWriter.test.ts @@ -1,27 +1,16 @@ +import { createResponse } from 'node-mocks-http'; import { LinkRelMetadataWriter } from '../../../../../src/ldp/http/metadata/LinkRelMetadataWriter'; import { RepresentationMetadata } from '../../../../../src/ldp/representation/RepresentationMetadata'; -import * as util from '../../../../../src/util/HeaderUtil'; import { LDP, RDF } from '../../../../../src/util/UriConstants'; import { toNamedNode } from '../../../../../src/util/UriUtil'; describe('A LinkRelMetadataWriter', (): void => { const writer = new LinkRelMetadataWriter({ [RDF.type]: 'type', dummy: 'dummy' }); - let mock: jest.SpyInstance; - let addHeaderMock: jest.Mock; - - beforeEach(async(): Promise => { - addHeaderMock = jest.fn(); - mock = jest.spyOn(util, 'addHeader').mockImplementation(addHeaderMock); - }); - - afterEach(async(): Promise => { - mock.mockRestore(); - }); it('adds the correct link headers.', async(): Promise => { + const response = createResponse(); const metadata = new RepresentationMetadata({ [RDF.type]: toNamedNode(LDP.Resource), unused: 'text' }); - await expect(writer.handle({ response: 'response' as any, metadata })).resolves.toBeUndefined(); - expect(addHeaderMock).toHaveBeenCalledTimes(1); - expect(addHeaderMock).toHaveBeenLastCalledWith('response', 'link', [ `<${LDP.Resource}>; rel="type"` ]); + await expect(writer.handle({ response, metadata })).resolves.toBeUndefined(); + expect(response.getHeaders()).toEqual({ link: `<${LDP.Resource}>; rel="type"` }); }); }); diff --git a/test/unit/ldp/http/metadata/MappedMetadataWriter.test.ts b/test/unit/ldp/http/metadata/MappedMetadataWriter.test.ts index 99e4e2168..f8dfab794 100644 --- a/test/unit/ldp/http/metadata/MappedMetadataWriter.test.ts +++ b/test/unit/ldp/http/metadata/MappedMetadataWriter.test.ts @@ -1,26 +1,15 @@ +import { createResponse } from 'node-mocks-http'; import { MappedMetadataWriter } from '../../../../../src/ldp/http/metadata/MappedMetadataWriter'; import { RepresentationMetadata } from '../../../../../src/ldp/representation/RepresentationMetadata'; -import * as util from '../../../../../src/util/HeaderUtil'; import { CONTENT_TYPE } from '../../../../../src/util/UriConstants'; describe('A MappedMetadataWriter', (): void => { const writer = new MappedMetadataWriter({ [CONTENT_TYPE]: 'content-type', dummy: 'dummy' }); - let mock: jest.SpyInstance; - let addHeaderMock: jest.Mock; - - beforeEach(async(): Promise => { - addHeaderMock = jest.fn(); - mock = jest.spyOn(util, 'addHeader').mockImplementation(addHeaderMock); - }); - - afterEach(async(): Promise => { - mock.mockRestore(); - }); it('adds metadata to the corresponding header.', async(): Promise => { + const response = createResponse(); const metadata = new RepresentationMetadata({ [CONTENT_TYPE]: 'text/turtle', unused: 'text' }); - await expect(writer.handle({ response: 'response' as any, metadata })).resolves.toBeUndefined(); - expect(addHeaderMock).toHaveBeenCalledTimes(1); - expect(addHeaderMock).toHaveBeenLastCalledWith('response', 'content-type', [ 'text/turtle' ]); + await expect(writer.handle({ response, metadata })).resolves.toBeUndefined(); + expect(response.getHeaders()).toEqual({ 'content-type': 'text/turtle' }); }); });