Repository: solid/community-server
Pull request #344 (2020-11-22 21:02:18)

Advertise WebSocket via Updates-Via header
This is the last individual piece for WebSocket support; the final one is wiring them up in the configuration.

The second commit aligns some of the other code; I found some minor shortcomings when looking for related code that could inform the writing of the `WebSocketMetadataWriter` class.


Patch:
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<ResourceIdentifier> {
-    if (!request.url) {
+  public async handle({ url, headers: { host }, connection }: HttpRequest): Promise<ResourceIdentifier> {
+    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/src/ldp/http/metadata/WebSocketMetadataWriter.ts b/src/ldp/http/metadata/WebSocketMetadataWriter.ts
new file mode 100644
index 000000000..00392578b
--- /dev/null
+++ b/src/ldp/http/metadata/WebSocketMetadataWriter.ts
@@ -0,0 +1,25 @@
+import type { HttpResponse } from '../../../server/HttpResponse';
+import { addHeader } from '../../../util/HeaderUtil';
+import { MetadataWriter } from './MetadataWriter';
+
+/**
+ * A {@link MetadataWriter} that advertises a WebSocket through the Updates-Via header.
+ */
+export class WebSocketMetadataWriter extends MetadataWriter {
+  private readonly socketUrl: string;
+
+  public constructor({ hostname = 'localhost', port = 80, protocol = 'ws:' }:
+  { hostname?: string; port?: number; protocol?: string }) {
+    super();
+    const secure = /^(?:https|wss)/u.test(protocol);
+    const socketUrl = new URL(`${secure ? 'wss' : 'ws'}://${hostname}:${port}/`);
+    if (socketUrl.hostname !== hostname) {
+      throw new Error(`Invalid hostname: ${hostname}`);
+    }
+    this.socketUrl = socketUrl.href.slice(0, -1);
+  }
+
+  public async handle({ response }: { response: HttpResponse }): Promise<void> {
+    addHeader(response, 'updates-via', this.socketUrl);
+  }
+}


Test patch:
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<void> => {
+    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<void> => {
     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<void> => {
+    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<void> => {
     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<void> => {
+  it('encodes paths.', async(): Promise<void> => {
     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<void> => {
+    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<void> => {
-    addHeaderMock = jest.fn();
-    mock = jest.spyOn(util, 'addHeader').mockImplementation(addHeaderMock);
-  });
-
-  afterEach(async(): Promise<void> => {
-    mock.mockRestore();
-  });
 
   it('adds the correct link headers.', async(): Promise<void> => {
+    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<void> => {
-    addHeaderMock = jest.fn();
-    mock = jest.spyOn(util, 'addHeader').mockImplementation(addHeaderMock);
-  });
-
-  afterEach(async(): Promise<void> => {
-    mock.mockRestore();
-  });
 
   it('adds metadata to the corresponding header.', async(): Promise<void> => {
+    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' });
   });
 });
diff --git a/test/unit/ldp/http/metadata/WebSocketMetadataWriter.test.ts b/test/unit/ldp/http/metadata/WebSocketMetadataWriter.test.ts
new file mode 100644
index 000000000..c3fd2958c
--- /dev/null
+++ b/test/unit/ldp/http/metadata/WebSocketMetadataWriter.test.ts
@@ -0,0 +1,37 @@
+import { createResponse } from 'node-mocks-http';
+import { WebSocketMetadataWriter } from '../../../../../src/ldp/http/metadata/WebSocketMetadataWriter';
+
+describe('A WebSocketMetadataWriter', (): void => {
+  it('writes a default HTTP WebSocket.', async(): Promise<void> => {
+    const writer = new WebSocketMetadataWriter({});
+    const response = createResponse();
+    await writer.handle({ response } as any);
+    expect(response.getHeaders()).toEqual({ 'updates-via': 'ws://localhost' });
+  });
+
+  it('writes an HTTP WebSocket with port 80.', async(): Promise<void> => {
+    const writer = new WebSocketMetadataWriter({ hostname: 'test.example', port: 80, protocol: 'http' });
+    const response = createResponse();
+    await writer.handle({ response } as any);
+    expect(response.getHeaders()).toEqual({ 'updates-via': 'ws://test.example' });
+  });
+
+  it('writes an HTTP WebSocket with port 3000.', async(): Promise<void> => {
+    const writer = new WebSocketMetadataWriter({ hostname: 'test.example', port: 3000, protocol: 'http' });
+    const response = createResponse();
+    await writer.handle({ response } as any);
+    expect(response.getHeaders()).toEqual({ 'updates-via': 'ws://test.example:3000' });
+  });
+
+  it('writes an HTTPS WebSocket with port 443.', async(): Promise<void> => {
+    const writer = new WebSocketMetadataWriter({ hostname: 'test.example', port: 443, protocol: 'https' });
+    const response = createResponse();
+    await writer.handle({ response } as any);
+    expect(response.getHeaders()).toEqual({ 'updates-via': 'wss://test.example' });
+  });
+
+  it('rejects an invalid hostname.', (): void => {
+    expect((): any => new WebSocketMetadataWriter({ hostname: 'test.example/invalid' }))
+      .toThrow('Invalid hostname: test.example/invalid');
+  });
+});