From ee85388a3bc9aaa9e9d19ba82fc6ae2afe1606a2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Thu, 14 Aug 2025 20:18:47 +0200 Subject: [PATCH] fix: avoid to leak handlers in UI server MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- src/charging-station/ocpp/OCPPServiceUtils.ts | 39 +++++-- .../ui-server/UIHttpServer.ts | 110 ++++++++++++------ .../ui-server/UIWebSocketServer.ts | 44 ++++++- src/utils/Utils.ts | 3 +- 4 files changed, 141 insertions(+), 55 deletions(-) diff --git a/src/charging-station/ocpp/OCPPServiceUtils.ts b/src/charging-station/ocpp/OCPPServiceUtils.ts index ab01c167..69a3e5d6 100644 --- a/src/charging-station/ocpp/OCPPServiceUtils.ts +++ b/src/charging-station/ocpp/OCPPServiceUtils.ts @@ -286,14 +286,29 @@ export const ajvErrorsToErrorType = (errors: ErrorObject[] | null | undefined): // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters export const convertDateToISOString = (object: T): void => { - // eslint-disable-next-line @typescript-eslint/no-for-in-array - for (const key in object) { - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion, @typescript-eslint/no-non-null-assertion - if (isDate(object![key])) { - ;(object[key] as unknown as string) = (object[key] as Date).toISOString() - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion, @typescript-eslint/no-non-null-assertion - } else if (typeof object![key] === 'object' && object[key] !== null) { - convertDateToISOString(object[key] as T) + for (const [key, value] of Object.entries(object as Record)) { + if (isDate(value)) { + try { + ;(object as Record)[key] = value.toISOString() + } catch { + // Ignore date conversion error + } + } else if (Array.isArray(value)) { + for (let i = 0; i < value.length; i++) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const item = value[i] + if (isDate(item)) { + try { + value[i] = item.toISOString() as unknown as typeof item + } catch { + // Ignore date conversion error + } + } else if (typeof item === 'object' && item !== null) { + convertDateToISOString(item as T) + } + } + } else if (typeof value === 'object' && value !== null) { + convertDateToISOString(value as T) } } } @@ -335,7 +350,7 @@ export const buildMeterValue = ( Number.parseInt(socSampledValueTemplate.value), socSampledValueTemplate.fluctuationPercent ?? Constants.DEFAULT_FLUCTUATION_PERCENT ) - : randomInt(socMinimumValue, socMaximumValue) + : randomInt(socMinimumValue, socMaximumValue + 1) meterValue.sampledValue.push( buildSampledValue(socSampledValueTemplate, socSampledValueTemplateValue) ) @@ -979,8 +994,8 @@ export const buildMeterValue = ( }: phase ${ // eslint-disable-next-line @typescript-eslint/restrict-template-expressions meterValue.sampledValue[sampledValuesPerPhaseIndex].phase - }, connector id ${connectorId.toString()}, transaction id $ - connector?.transactionId?.toString()}, value: ${connectorMinimumAmperage.toString()}/${ + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + }, connector id ${connectorId.toString()}, transaction id ${connector?.transactionId?.toString()}, value: ${connectorMinimumAmperage.toString()}/${ meterValue.sampledValue[sampledValuesPerPhaseIndex].value }/${connectorMaximumAmperage.toString()}` ) @@ -1143,7 +1158,7 @@ const getLimitFromSampledValueTemplateCustomValue = ( }, ...options, } - const parsedValue = Number.parseInt(value ?? '') + const parsedValue = Number.parseFloat(value ?? '') if (options.limitationEnabled) { return max( min( diff --git a/src/charging-station/ui-server/UIHttpServer.ts b/src/charging-station/ui-server/UIHttpServer.ts index b1357822..2004752c 100644 --- a/src/charging-station/ui-server/UIHttpServer.ts +++ b/src/charging-station/ui-server/UIHttpServer.ts @@ -16,7 +16,6 @@ import { type UIServerConfiguration, } from '../../types/index.js' import { - Constants, generateUUID, isNotEmptyString, JSONStringify, @@ -98,29 +97,52 @@ export class UIHttpServer extends AbstractUIServer { .end(`${StatusCodes.UNAUTHORIZED.toString()} Unauthorized`) res.destroy() req.destroy() + return } - }) - // Expected request URL pathname: /ui/:version/:procedureName - const [protocol, version, procedureName] = req.url?.split('/').slice(1) as [ - Protocol, - ProtocolVersion, - ProcedureName - ] - const uuid = generateUUID() - this.responseHandlers.set(uuid, res) - try { - const fullProtocol = `${protocol}${version}` - if (!isProtocolAndVersionSupported(fullProtocol)) { - throw new BaseError(`Unsupported UI protocol version: '${fullProtocol}'`) - } - this.registerProtocolVersionUIService(version) - req.on('error', error => { - logger.error( - `${this.logPrefix(moduleName, 'requestListener.req.onerror')} Error on HTTP request:`, - error - ) + + const uuid = generateUUID() + this.responseHandlers.set(uuid, res) + res.on('close', () => { + this.responseHandlers.delete(uuid) }) - if (req.method === HttpMethods.POST) { + try { + // Expected request URL pathname: /ui/:version/:procedureName + const rawUrl = req.url ?? '' + const { pathname } = new URL(rawUrl, 'http://localhost') + const parts = pathname.split('/').filter(Boolean) + if (parts.length < 3) { + throw new BaseError( + `Malformed URL path: '${pathname}' (expected /ui/:version/:procedureName)` + ) + } + const [protocol, version, procedureName] = parts as [ + Protocol, + ProtocolVersion, + ProcedureName + ] + const fullProtocol = `${protocol}${version}` + if (!isProtocolAndVersionSupported(fullProtocol)) { + throw new BaseError(`Unsupported UI protocol version: '${fullProtocol}'`) + } + this.registerProtocolVersionUIService(version) + + req.on('error', error => { + logger.error( + `${this.logPrefix(moduleName, 'requestListener.req.onerror')} Error on HTTP request:`, + error + ) + if (!res.headersSent) { + this.sendResponse(this.buildProtocolResponse(uuid, { status: ResponseStatus.FAILURE })) + } else { + this.responseHandlers.delete(uuid) + } + }) + + if (req.method !== HttpMethods.POST) { + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + throw new BaseError(`Unsupported HTTP method: '${req.method}'`) + } + const bodyBuffer: Uint8Array[] = [] req .on('data', (chunk: Uint8Array) => { @@ -140,28 +162,44 @@ export class UIHttpServer extends AbstractUIServer { ) return } - this.uiServices - .get(version) - ?.requestHandler(this.buildProtocolRequest(uuid, procedureName, requestPayload)) + const service = this.uiServices.get(version) + if (service == null || typeof service.requestHandler !== 'function') { + this.sendResponse( + this.buildProtocolResponse(uuid, { status: ResponseStatus.FAILURE }) + ) + return + } + // eslint-disable-next-line promise/no-promise-in-callback + service + .requestHandler(this.buildProtocolRequest(uuid, procedureName, requestPayload)) .then((protocolResponse?: ProtocolResponse) => { if (protocolResponse != null) { this.sendResponse(protocolResponse) + } else { + this.sendResponse( + this.buildProtocolResponse(uuid, { status: ResponseStatus.SUCCESS }) + ) } return undefined }) - .catch(Constants.EMPTY_FUNCTION) + .catch((error: unknown) => { + logger.error( + `${this.logPrefix(moduleName, 'requestListener.handler.catch')} UI service handler error:`, + error + ) + this.sendResponse( + this.buildProtocolResponse(uuid, { status: ResponseStatus.FAILURE }) + ) + }) }) - } else { - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - throw new BaseError(`Unsupported HTTP method: '${req.method}'`) + } catch (error) { + logger.error( + `${this.logPrefix(moduleName, 'requestListener')} Handle HTTP request error:`, + error + ) + this.sendResponse(this.buildProtocolResponse(uuid, { status: ResponseStatus.FAILURE })) } - } catch (error) { - logger.error( - `${this.logPrefix(moduleName, 'requestListener')} Handle HTTP request error:`, - error - ) - this.sendResponse(this.buildProtocolResponse(uuid, { status: ResponseStatus.FAILURE })) - } + }) } private responseStatusToStatusCode (status: ResponseStatus): StatusCodes { diff --git a/src/charging-station/ui-server/UIWebSocketServer.ts b/src/charging-station/ui-server/UIWebSocketServer.ts index dd57883e..0a44a799 100644 --- a/src/charging-station/ui-server/UIWebSocketServer.ts +++ b/src/charging-station/ui-server/UIWebSocketServer.ts @@ -92,16 +92,19 @@ export class UIWebSocketServer extends AbstractUIServer { public start (): void { this.webSocketServer.on('connection', (ws: WebSocket, _req: IncomingMessage): void => { - if (!isProtocolAndVersionSupported(ws.protocol)) { + const protocol = ws.protocol + const protocolAndVersion = getProtocolAndVersion(protocol) + if (protocolAndVersion == null || !isProtocolAndVersionSupported(protocol)) { logger.error( `${this.logPrefix( moduleName, 'start.server.onconnection' - )} Unsupported UI protocol version: '${ws.protocol}'` + )} Unsupported UI protocol version: '${protocol}'` ) ws.close(WebSocketCloseEventStatusCode.CLOSE_PROTOCOL_ERROR) + return } - const [, version] = getProtocolAndVersion(ws.protocol) + const [, version] = protocolAndVersion this.registerProtocolVersionUIService(version) ws.on('message', rawData => { const request = this.validateRawDataRequest(rawData) @@ -121,6 +124,9 @@ export class UIWebSocketServer extends AbstractUIServer { return undefined }) .catch(Constants.EMPTY_FUNCTION) + .finally(() => { + this.responseHandlers.delete(requestId) + }) }) ws.on('error', error => { logger.error(`${this.logPrefix(moduleName, 'start.ws.onerror')} WebSocket error:`, error) @@ -134,10 +140,15 @@ export class UIWebSocketServer extends AbstractUIServer { code )}' - '${reason.toString()}'` ) + for (const [responseId, responseHandlerWs] of this.responseHandlers) { + if (responseHandlerWs === ws) this.responseHandlers.delete(responseId) + } }) }) this.httpServer.on('connect', (req: IncomingMessage, socket: Duplex, _head: Buffer) => { - if (req.headers.connection !== 'Upgrade' || req.headers.upgrade !== 'websocket') { + const connectionHeader = req.headers.connection ?? '' + const upgradeHeader = req.headers.upgrade ?? '' + if (!/upgrade/i.test(connectionHeader) || !/^websocket$/i.test(upgradeHeader)) { socket.write(`HTTP/1.1 ${StatusCodes.BAD_REQUEST.toString()} Bad Request\r\n\r\n`) socket.destroy() } @@ -154,6 +165,7 @@ export class UIWebSocketServer extends AbstractUIServer { } socket.on('error', onSocketError) this.authenticate(req, err => { + socket.removeListener('error', onSocketError) if (err != null) { socket.write(`HTTP/1.1 ${StatusCodes.UNAUTHORIZED.toString()} Unauthorized\r\n\r\n`) socket.destroy() @@ -173,7 +185,6 @@ export class UIWebSocketServer extends AbstractUIServer { ) } }) - socket.removeListener('error', onSocketError) }) this.startHttpServer() } @@ -241,6 +252,29 @@ export class UIWebSocketServer extends AbstractUIServer { return false } + if (typeof request[1] !== 'string') { + logger.error( + `${this.logPrefix( + moduleName, + 'validateRawDataRequest' + )} UI protocol request procedure field must be a string:`, + request + ) + return false + } + + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + if (!(typeof request[2] === 'object' && request[2] !== null)) { + logger.error( + `${this.logPrefix( + moduleName, + 'validateRawDataRequest' + )} UI protocol request payload field must be an object or an array:`, + request + ) + return false + } + return request } } diff --git a/src/utils/Utils.ts b/src/utils/Utils.ts index 285927bd..3e0f59cc 100644 --- a/src/utils/Utils.ts +++ b/src/utils/Utils.ts @@ -61,7 +61,6 @@ export const has = (property: PropertyKey, object: null | object | undefined): b const type = (value: unknown): string => { if (value === null) return 'Null' if (value === undefined) return 'Undefined' - if (typeof value === 'string') return 'String' if (Number.isNaN(value)) return 'NaN' if (Array.isArray(value)) return 'Array' return Object.prototype.toString.call(value).slice(8, -1) @@ -69,7 +68,7 @@ const type = (value: unknown): string => { export const isEmpty = (value: unknown): boolean => { const valueType = type(value) - if (['NaN', 'Null', 'Number', 'Undefined'].includes(valueType)) { + if (['BigInt', 'Boolean', 'NaN', 'Null', 'Number', 'Undefined'].includes(valueType)) { return false } if (!value) return true -- 2.43.0