fix: Frontend code cleanup @Nerivec (#24322)

Koenkk · Nerivec · web-flow · commit e2f19f19b441 · 2024-10-14T19:56:00.000+02:00
Co-authored-by: Nerivec &lt;62446222+Nerivec@users.noreply.github.com&gt;
diff --git 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<void> {
-        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<void> {
         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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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', () => {

Original file line number	Diff line number	Diff line change
`@@ -402,9 +402,11 @@`
`402`	`402`	`"requiresRestart": true`
`403`	`403`	`},`
`404`	`404`	`"base_url": {`
`405`		`- "type": ["string", "null"],`
	`405`	`+ "type": "string",`
	`406`	`+ "pattern": "^\\/.*",`
`406`	`407`	`"title": "Base URL",`
`407`		`- "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'",`
	`408`	`+ "description": "Base URL for the frontend. If hosted under a subpath, e.g. 'http://localhost:8080/z2m', set this to '/z2m'",`
	`409`	`+ "default": "/",`
`408`	`410`	`"requiresRestart": true`
`409`	`411`	`}`
`410`	`412`	`}`
Original file line number	Diff line number	Diff line change
`@@ -170,7 +170,7 @@ function loadSettingsWithDefaults(): void {`
`170`	`170`	`}`
`171`	`171`
`172`	`172`	`if (_settingsWithDefaults.frontend) {`
`173`		`- const defaults = {port: 8080, auth_token: false};`
	`173`	`+ const defaults = {port: 8080, auth_token: null, base_url: '/'};`
`174`	`174`	`const s = typeof _settingsWithDefaults.frontend === 'object' ? _settingsWithDefaults.frontend : {};`
`175`	`175`	`// @ts-expect-error ignore typing`
`176`	`176`	`_settingsWithDefaults.frontend = {};`