From e2f19f19b4413fc4cdad6f64879840497bb2ca81 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Mon, 14 Oct 2024 19:56:00 +0200 Subject: [PATCH] fix: Frontend code cleanup @Nerivec (#24322) Co-authored-by: Nerivec <62446222+Nerivec@users.noreply.github.com> --- lib/extension/frontend.ts | 91 ++++++++++++++--------------- lib/types/types.d.ts | 8 +-- lib/types/zigbee2mqtt-frontend.d.ts | 6 ++ lib/util/settings.schema.json | 6 +- lib/util/settings.ts | 2 +- test/frontend.test.js | 32 +++++++++- test/settings.test.js | 2 +- 7 files changed, 89 insertions(+), 58 deletions(-) diff --git a/lib/extension/frontend.ts b/lib/extension/frontend.ts index 05993a4b..a1e057d3 100644 --- a/lib/extension/frontend.ts +++ b/lib/extension/frontend.ts @@ -1,10 +1,12 @@ +import type {IncomingMessage, Server, ServerResponse} from 'http'; +import type {Socket} from 'net'; + import assert from 'assert'; -import fs from 'fs'; -import http from 'http'; -import https from 'https'; -import net from 'net'; -import path from 'path'; -import url from 'url'; +import {existsSync, readFileSync} from 'fs'; +import {createServer} from 'http'; +import {createServer as createSecureServer} from 'https'; +import {posix} from 'path'; +import {parse} from 'url'; import bind from 'bind-decorator'; import gzipStatic, {RequestHandler} from 'connect-gzip-static'; @@ -29,10 +31,10 @@ export default class Frontend extends Extension { private sslCert: string | undefined; private sslKey: string | undefined; private authToken: string | undefined; - private server: http.Server | undefined; - private fileServer: RequestHandler | undefined; - private wss: WebSocket.Server | undefined; - private frontendBaseUrl: string; + private server!: Server; + private fileServer!: RequestHandler; + private wss!: WebSocket.Server; + private baseUrl: string; constructor( zigbee: Zigbee, @@ -53,16 +55,13 @@ export default class Frontend extends Extension { this.sslCert = frontendSettings.ssl_cert; this.sslKey = frontendSettings.ssl_key; this.authToken = frontendSettings.auth_token; + this.baseUrl = frontendSettings.base_url; this.mqttBaseTopic = settings.get().mqtt.base_topic; - this.frontendBaseUrl = settings.get().frontend?.base_url ?? '/'; - if (!this.frontendBaseUrl.startsWith('/')) { - this.frontendBaseUrl = '/' + this.frontendBaseUrl; - } } private isHttpsConfigured(): boolean { if (this.sslCert && this.sslKey) { - if (!fs.existsSync(this.sslCert) || !fs.existsSync(this.sslKey)) { + if (!existsSync(this.sslCert) || !existsSync(this.sslKey)) { logger.error(`defined ssl_cert '${this.sslCert}' or ssl_key '${this.sslKey}' file path does not exists, server won't be secured.`); return false; } @@ -72,30 +71,30 @@ export default class Frontend extends Extension { } override async start(): Promise { - if (this.isHttpsConfigured()) { - const serverOptions = { - key: fs.readFileSync(this.sslKey!), // valid from `isHttpsConfigured` - cert: fs.readFileSync(this.sslCert!), // valid from `isHttpsConfigured` - }; - this.server = https.createServer(serverOptions, this.onRequest); - } else { - this.server = http.createServer(this.onRequest); - } - - this.server.on('upgrade', this.onUpgrade); - /* istanbul ignore next */ const options = { - setHeaders: (res: http.ServerResponse, path: string): void => { + setHeaders: (res: ServerResponse, path: string): void => { if (path.endsWith('index.html')) { res.setHeader('Cache-Control', 'no-store'); } }, }; this.fileServer = gzipStatic(frontend.getPath(), options); - this.wss = new WebSocket.Server({noServer: true, path: path.posix.join(this.frontendBaseUrl, 'api')}); + this.wss = new WebSocket.Server({noServer: true, path: posix.join(this.baseUrl, 'api')}); + this.wss.on('connection', this.onWebSocketConnection); + if (this.isHttpsConfigured()) { + const serverOptions = { + key: readFileSync(this.sslKey!), // valid from `isHttpsConfigured` + cert: readFileSync(this.sslCert!), // valid from `isHttpsConfigured` + }; + this.server = createSecureServer(serverOptions, this.onRequest); + } else { + this.server = createServer(this.onRequest); + } + + this.server.on('upgrade', this.onUpgrade); this.eventBus.onMQTTMessagePublished(this, this.onMQTTPublishMessage); if (!this.host) { @@ -112,43 +111,42 @@ export default class Frontend extends Extension { override async stop(): Promise { await super.stop(); - this.wss?.clients.forEach((client) => { + this.wss.clients.forEach((client) => { client.send(stringify({topic: 'bridge/state', payload: 'offline'})); client.terminate(); }); - this.wss?.close(); - /* istanbul ignore else */ - if (this.server) { - return await new Promise((cb: () => void) => this.server!.close(cb)); - } + this.wss.close(); + + await new Promise((resolve) => this.server.close(resolve)); } - @bind private onRequest(request: http.IncomingMessage, response: http.ServerResponse): void { + @bind private onRequest(request: IncomingMessage, response: ServerResponse): void { const fin = finalhandler(request, response); + const newUrl = posix.relative(this.baseUrl, request.url!); - const newUrl = path.posix.relative(this.frontendBaseUrl, request.url!); // The request url is not within the frontend base url, so the relative path starts with '..' if (newUrl.startsWith('.')) { return fin(); } - // Attach originalUrl so that static-server can perform a redirect to '/' when serving the - // root directory. This is necessary for the browser to resolve relative assets paths correctly. + // Attach originalUrl so that static-server can perform a redirect to '/' when serving the root directory. + // This is necessary for the browser to resolve relative assets paths correctly. request.originalUrl = request.url; request.url = '/' + newUrl; - this.fileServer?.(request, response, fin); + + this.fileServer(request, response, fin); } - private authenticate(request: http.IncomingMessage, cb: (authenticate: boolean) => void): void { - const {query} = url.parse(request.url!, true); + private authenticate(request: IncomingMessage, cb: (authenticate: boolean) => void): void { + const {query} = parse(request.url!, true); cb(!this.authToken || this.authToken === query.token); } - @bind private onUpgrade(request: http.IncomingMessage, socket: net.Socket, head: Buffer): void { - this.wss!.handleUpgrade(request, socket, head, (ws) => { + @bind private onUpgrade(request: IncomingMessage, socket: Socket, head: Buffer): void { + this.wss.handleUpgrade(request, socket, head, (ws) => { this.authenticate(request, (isAuthenticated) => { if (isAuthenticated) { - this.wss!.emit('connection', ws, request); + this.wss.emit('connection', ws, request); } else { ws.close(4401, 'Unauthorized'); } @@ -182,6 +180,7 @@ export default class Frontend extends Extension { for (const device of this.zigbee.devicesIterator(utils.deviceNotCoordinator)) { const payload = this.state.get(device); const lastSeen = settings.get().advanced.last_seen; + /* istanbul ignore if */ if (lastSeen !== 'disable') { payload.last_seen = utils.formatDate(device.zh.lastSeen ?? 0, lastSeen); @@ -202,7 +201,7 @@ export default class Frontend extends Extension { const topic = data.topic.substring(this.mqttBaseTopic.length + 1); const payload = utils.parseJSON(data.payload, data.payload); - for (const client of this.wss!.clients) { + for (const client of this.wss.clients) { /* istanbul ignore else */ if (client.readyState === WebSocket.OPEN) { client.send(stringify({topic, payload})); diff --git a/lib/types/types.d.ts b/lib/types/types.d.ts index 3e1c056a..20656165 100644 --- a/lib/types/types.d.ts +++ b/lib/types/types.d.ts @@ -179,7 +179,7 @@ declare global { auth_token?: string; host?: string; port: number; - base_url?: string; + base_url: string; url?: string; ssl_cert?: string; ssl_key?: string; @@ -264,9 +264,3 @@ declare global { qos?: 0 | 1 | 2; } } - -declare module 'http' { - interface IncomingMessage { - originalUrl?: string; - } -} diff --git a/lib/types/zigbee2mqtt-frontend.d.ts b/lib/types/zigbee2mqtt-frontend.d.ts index 02bb80f0..fff7b51d 100644 --- a/lib/types/zigbee2mqtt-frontend.d.ts +++ b/lib/types/zigbee2mqtt-frontend.d.ts @@ -2,6 +2,12 @@ declare module 'zigbee2mqtt-frontend' { export function getPath(): string; } +declare module 'http' { + interface IncomingMessage { + originalUrl?: string; + } +} + declare module 'connect-gzip-static' { import {IncomingMessage, ServerResponse} from 'http'; export type RequestHandler = (req: IncomingMessage, res: ServerResponse, finalhandler: (err: unknown) => void) => void; diff --git a/lib/util/settings.schema.json b/lib/util/settings.schema.json index 14b4e8ce..0585d48e 100644 --- a/lib/util/settings.schema.json +++ b/lib/util/settings.schema.json @@ -402,9 +402,11 @@ "requiresRestart": true }, "base_url": { - "type": ["string", "null"], + "type": "string", + "pattern": "^\\/.*", "title": "Base URL", - "description": "Base URL for the frontend if the frontend is hosted under subpath. E.g. if your frontend is available at 'http://localhost/z2m', set this to '/z2m'", + "description": "Base URL for the frontend. If hosted under a subpath, e.g. 'http://localhost:8080/z2m', set this to '/z2m'", + "default": "/", "requiresRestart": true } } diff --git a/lib/util/settings.ts b/lib/util/settings.ts index 41f9f045..9437dbd2 100644 --- a/lib/util/settings.ts +++ b/lib/util/settings.ts @@ -170,7 +170,7 @@ function loadSettingsWithDefaults(): void { } if (_settingsWithDefaults.frontend) { - const defaults = {port: 8080, auth_token: false}; + const defaults = {port: 8080, auth_token: null, base_url: '/'}; const s = typeof _settingsWithDefaults.frontend === 'object' ? _settingsWithDefaults.frontend : {}; // @ts-expect-error ignore typing _settingsWithDefaults.frontend = {}; diff --git a/test/frontend.test.js b/test/frontend.test.js index 857e79ea..6a13c6de 100644 --- a/test/frontend.test.js +++ b/test/frontend.test.js @@ -358,7 +358,7 @@ describe('Frontend', () => { expect(mockWS.implementation.emit).toHaveBeenCalledWith('connection', mockWSocket, {url}); }); - it.each(['z2m', 'z2m/', '/z2m'])('Works with non-default base url %s', async (baseUrl) => { + it.each(['/z2m/', '/z2m'])('Works with non-default base url %s', async (baseUrl) => { settings.set(['frontend'], {base_url: baseUrl}); controller = new Controller(jest.fn(), jest.fn()); await controller.start(); @@ -382,4 +382,34 @@ describe('Frontend', () => { expect(mockNodeStatic.implementation).not.toHaveBeenCalled(); expect(mockFinalHandler.implementation).toHaveBeenCalled(); }); + + it('Works with non-default complex base url', async () => { + const baseUrl = '/z2m-more++/c0mplex.url/'; + settings.set(['frontend'], {base_url: baseUrl}); + controller = new Controller(jest.fn(), jest.fn()); + await controller.start(); + + expect(ws.Server).toHaveBeenCalledWith({noServer: true, path: '/z2m-more++/c0mplex.url/api'}); + + mockHTTP.variables.onRequest({url: '/z2m-more++/c0mplex.url'}, 2); + expect(mockNodeStatic.implementation).toHaveBeenCalledTimes(1); + expect(mockNodeStatic.implementation).toHaveBeenCalledWith({originalUrl: '/z2m-more++/c0mplex.url', url: '/'}, 2, expect.any(Function)); + expect(mockFinalHandler.implementation).not.toHaveBeenCalledWith(); + + mockNodeStatic.implementation.mockReset(); + expect(mockFinalHandler.implementation).not.toHaveBeenCalledWith(); + mockHTTP.variables.onRequest({url: '/z2m-more++/c0mplex.url/file.txt'}, 2); + expect(mockNodeStatic.implementation).toHaveBeenCalledTimes(1); + expect(mockNodeStatic.implementation).toHaveBeenCalledWith( + {originalUrl: '/z2m-more++/c0mplex.url/file.txt', url: '/file.txt'}, + 2, + expect.any(Function), + ); + expect(mockFinalHandler.implementation).not.toHaveBeenCalledWith(); + + mockNodeStatic.implementation.mockReset(); + mockHTTP.variables.onRequest({url: '/z/file.txt'}, 2); + expect(mockNodeStatic.implementation).not.toHaveBeenCalled(); + expect(mockFinalHandler.implementation).toHaveBeenCalled(); + }); }); diff --git a/test/settings.test.js b/test/settings.test.js index a0c3c14f..3e077524 100644 --- a/test/settings.test.js +++ b/test/settings.test.js @@ -978,7 +978,7 @@ describe('Settings', () => { write(configurationFile, {...minimalConfig, frontend: true}); settings.reRead(); - expect(settings.get().frontend).toStrictEqual({port: 8080, auth_token: false}); + expect(settings.get().frontend).toStrictEqual({port: 8080, auth_token: null, base_url: '/'}); }); it('Baudrate config', () => {