From cb8a959d58b232a14bd0023d83aa5c95a17d28d1 Mon Sep 17 00:00:00 2001 From: Reginald Alfret V Date: Thu, 8 Oct 2026 02:17:59 +0530 Subject: [PATCH] fix(charging-station): throw BaseError on invalid supervision URL and revert state on set failure (#2136) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * fix(charging-station): throw BaseError on invalid supervision URL and revert state on set failure Signed-off-by: reginaldalfret * [autofix.ci] apply automated fixes * fix(charging-station): guard every supervision URL parsing path The invalid-supervision-URL fix only covered the stationInfo branch of setSupervisionUrl and passed a cause option BaseError does not accept, so the file did not typecheck. Route every URL construction through a single parseSupervisionUrl helper that raises a BaseError naming the offending field, covering the template/config path, the OCPP-configuration key branch and the wsConnectionUrl getter, and add the regression tests the issue requires. Signed-off-by: Jérôme Benoit * test(charging-station): cover every supervision URL guard and isolate the suite The getter guard added by the previous commit had no test: reverting it left the suite green. The three supervision URL tests also leaked a temp template directory each, because a station built with the default persistence writes its configuration after the synchronous afterEach ran cleanup. Move them into a dedicated ChargingStation-SupervisionUrl suite with an async teardown that drains the pending write, disable persistence on every station they build, add the missing getter regression, drop two inert creation options and pin the exact BaseError message and name. Name the charging station in the diagnostic and report the configured base URL instead of the derived one, so the error matches the issue acceptance criteria and no longer quotes a value the operator never configured. Signed-off-by: Jérôme Benoit * fix(charging-station): normalize supervision URL before persisting Signed-off-by: Jérôme Benoit * refactor: reuse test helpers and keep README user-focused Signed-off-by: Jérôme Benoit --------- Signed-off-by: reginaldalfret Signed-off-by: Jérôme Benoit Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jérôme Benoit Co-authored-by: Jérôme Benoit --- cspell.config.yaml | 1 + src/charging-station/ChargingStation.ts | 83 ++++-- .../ChargingStation-Configuration.test.ts | 16 -- .../ChargingStation-SupervisionUrl.test.ts | 270 ++++++++++++++++++ .../fixtures/supervisionUrlWorker.mjs | 84 ++++++ 5 files changed, 418 insertions(+), 36 deletions(-) create mode 100644 tests/charging-station/ChargingStation-SupervisionUrl.test.ts create mode 100644 tests/charging-station/fixtures/supervisionUrlWorker.mjs diff --git a/cspell.config.yaml b/cspell.config.yaml index 124140f9..7735b6b6 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -107,3 +107,4 @@ words: - HSTS - Hsts - hsts + - ocppcentraladdress diff --git a/src/charging-station/ChargingStation.ts b/src/charging-station/ChargingStation.ts index a4333563..9d4b7b2d 100644 --- a/src/charging-station/ChargingStation.ts +++ b/src/charging-station/ChargingStation.ts @@ -253,19 +253,32 @@ export class ChargingStation extends EventEmitter { } public get wsConnectionUrl (): URL { - const wsConnectionBaseUrlStr = `${ - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + const supervisionUrlOcppKey = this.stationInfo?.supervisionUrlOcppConfiguration === true && - isNotEmptyString(this.stationInfo.supervisionUrlOcppKey) && - isNotEmptyString(getConfigurationKey(this, this.stationInfo.supervisionUrlOcppKey)?.value) - ? getConfigurationKey(this, this.stationInfo.supervisionUrlOcppKey)?.value - : this.configuredSupervisionUrl.href - }` - return new URL( + isNotEmptyString(this.stationInfo.supervisionUrlOcppKey) + ? this.stationInfo.supervisionUrlOcppKey + : undefined + const configuredSupervisionUrl = + supervisionUrlOcppKey != null + ? getConfigurationKey(this, supervisionUrlOcppKey)?.value + : undefined + const wsConnectionBaseUrlStr = isNotEmptyString(configuredSupervisionUrl) + ? configuredSupervisionUrl + : this.configuredSupervisionUrl.href + // Only the OCPP configuration key is labelled as such when it actually + // supplied the value; an absent or empty key falls back to the configured + // supervision URL, so the diagnostic must not name the OCPP key. + const supervisionUrlField = + supervisionUrlOcppKey != null && isNotEmptyString(configuredSupervisionUrl) + ? supervisionUrlOcppKey + : 'supervisionUrls' + return this.parseSupervisionUrl( `${wsConnectionBaseUrlStr}${ !wsConnectionBaseUrlStr.endsWith('/') ? '/' : '' // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - }${this.stationInfo?.chargingStationId}` + }${this.stationInfo?.chargingStationId}`, + supervisionUrlField, + wsConnectionBaseUrlStr ) } @@ -1851,21 +1864,23 @@ export class ChargingStation extends EventEmitter { this.stationInfo?.supervisionUrlOcppConfiguration === true && isNotEmptyString(this.stationInfo.supervisionUrlOcppKey) ) { - setConfigurationKeyValue(this, this.stationInfo.supervisionUrlOcppKey, url) + const supervisionUrl = this.parseSupervisionUrl(url, this.stationInfo.supervisionUrlOcppKey) + setConfigurationKeyValue(this, this.stationInfo.supervisionUrlOcppKey, supervisionUrl.href) } else if (this.stationInfo != null) { + const prevSupervisionUrls = this.stationInfo.supervisionUrls this.stationInfo.supervisionUrls = url - this.configuredSupervisionUrl = this.getConfiguredSupervisionUrl() + try { + this.configuredSupervisionUrl = this.getConfiguredSupervisionUrl() + } catch (error) { + this.stationInfo.supervisionUrls = prevSupervisionUrls + throw error + } } if (this.stationInfo != null) { applyCredentials(this.stationInfo) - // Mirror the update into the retained creation options so a later reset() or - // template reload re-applies this URL, not the creation-time one — for BOTH - // branches above. On a cache-cold reinitialization (reload) a non-persistent - // station's supervisionUrlOcppKey is absent and re-seeded from - // configuredSupervisionUrl (i.e. stationInfo.supervisionUrls, written from - // these options), so without the mirror the reload would restore the old URL. - // (A warm reset() reuses the cached template whose in-place key mutation - // already carries it.) In-memory only: a full restart reverts to the originals. + // Retain the caller's URL and credentials for non-persistent resets and + // template reloads, which rebuild OCPP configuration from these options. + // getConfiguredSupervisionUrl normalizes the URL again before reseeding the key. if (this.creationOptions != null) { this.creationOptions.supervisionUrls = url applyCredentials(this.creationOptions) @@ -2471,7 +2486,7 @@ export class ChargingStation extends EventEmitter { configuredSupervisionUrl = supervisionUrls } if (isNotEmptyString(configuredSupervisionUrl)) { - return new URL(configuredSupervisionUrl) + return this.parseSupervisionUrl(configuredSupervisionUrl, 'supervisionUrls') } const errorMsg = 'No supervision url(s) configured' logger.error(`${this.logPrefix()} ${moduleName}.getConfiguredSupervisionUrl: ${errorMsg}`) @@ -3733,6 +3748,34 @@ export class ChargingStation extends EventEmitter { ) } + /** + * Parses a supervision URL, raising a structured configuration error naming + * the offending field and station instead of leaking the `TypeError` thrown + * by `new URL()`. The original error is logged with its stack. + * @param supervisionUrl - Supervision URL string to parse. + * @param field - Configuration field or OCPP configuration key holding it. + * @param reportedUrl - Configured value quoted in the diagnostic; defaults to + * `supervisionUrl`. The `wsConnectionUrl` getter passes the configured base + * URL because it parses a derived URL (base plus charging station identity). + * @returns The parsed supervision URL. + */ + private parseSupervisionUrl ( + supervisionUrl: string, + field: string, + reportedUrl = supervisionUrl + ): URL { + try { + return new URL(supervisionUrl) + } catch (error) { + const chargingStationId = this.stationInfo?.chargingStationId + const errorMsg = `${ + isNotEmptyString(chargingStationId) ? `${chargingStationId}: ` : '' + }Invalid supervision url '${reportedUrl}' configured in '${field}'` + logger.error(`${this.logPrefix()} ${moduleName}.parseSupervisionUrl: ${errorMsg}`, error) + throw new BaseError(errorMsg) + } + } + private async performDelete (): Promise { const stopPromise = this.started || this.stopPromise != null ? this.stop() : undefined if (stopPromise != null) { diff --git a/tests/charging-station/ChargingStation-Configuration.test.ts b/tests/charging-station/ChargingStation-Configuration.test.ts index 03a45241..421e908f 100644 --- a/tests/charging-station/ChargingStation-Configuration.test.ts +++ b/tests/charging-station/ChargingStation-Configuration.test.ts @@ -357,22 +357,6 @@ await describe('ChargingStation Configuration Management', async () => { cleanupChargingStation(station2) }) - await it('should support setSupervisionUrl method if available', () => { - // Arrange - const result = createMockChargingStation() - station = result.station - - // Act & Assert - setSupervisionUrl should be a function if available - if ('setSupervisionUrl' in station && typeof station.setSupervisionUrl === 'function') { - assert.doesNotThrow(() => { - station?.setSupervisionUrl('ws://new-server:8080') - }) - } else { - // Mock station may not have setSupervisionUrl, which is expected - assert.notStrictEqual(station, undefined) - } - }) - // === Configuration Loading & Persistence === await it('should have template file reference', () => { diff --git a/tests/charging-station/ChargingStation-SupervisionUrl.test.ts b/tests/charging-station/ChargingStation-SupervisionUrl.test.ts new file mode 100644 index 00000000..74eb6322 --- /dev/null +++ b/tests/charging-station/ChargingStation-SupervisionUrl.test.ts @@ -0,0 +1,270 @@ +/** + * @file Tests that a charging station rejects a malformed supervision URL with a structured configuration error. + * @description A relative or malformed supervision URL must surface a `BaseError` + * naming the offending field and the station instead of leaking the `TypeError` + * from `new URL()`, on every entry path: template/configuration loading, + * `setSupervisionUrl` (both branches) and the `wsConnectionUrl` getter. + */ +import assert from 'node:assert/strict' +import { afterEach, describe, it } from 'node:test' +import { Worker } from 'node:worker_threads' + +import type { ChargingStation } from '../../src/charging-station/ChargingStation.js' + +import { BaseError } from '../../src/exception/BaseError.js' +import { + type ChargingStationConfiguration, + type ChargingStationData, + type ChargingStationOptions, + type ChargingStationWorkerMessage, + ChargingStationWorkerMessageEvents, +} from '../../src/types/index.js' +import { flushMicrotasks, standardCleanup } from '../helpers/TestLifecycleHelpers.js' +import { TEST_SUPERVISION_URL_ALT_PORT } from '../utils/TestNetworkConstants.js' +import { TEST_CHARGING_STATION_BASE_NAME } from './ChargingStationTestConstants.js' +import { + cleanupStationTemplates, + copyStationTemplate, + createStationFromTemplate, +} from './helpers/StationHelpers.realStation.js' + +// Shipped templates use this custom key instead of the default ConnectionUrl; +// retain it to verify diagnostics name the configured key, not the default. +const OCPP_SUPERVISION_URL_KEY = 'ocppcentraladdress' +const MALFORMED_SUPERVISION_URL = 'not a url' + +const stationInternalsOf = ( + station: ChargingStation +): { creationOptions?: ChargingStationOptions } => + station as unknown as { creationOptions?: ChargingStationOptions } + +type SupervisionUrlWorkerMessage = + | ChargingStationWorkerMessage + | { data: Pick; event: 'supervisionUrlFailure' } + | { data: SupervisionUrlWorkerScenario[]; event: 'supervisionUrlResult' } + +interface SupervisionUrlWorkerScenario { + accepted: SupervisionUrlWorkerState + persistedAfter: string + persistedBefore: string + rejected: SupervisionUrlWorkerState + rejection?: Pick + supervisionUrlOcppConfiguration: boolean +} + +interface SupervisionUrlWorkerState { + connectionUrl: string + creationOptions?: ChargingStationOptions + ocppUrl?: string + stationInfo: Pick< + ChargingStationOptions, + 'supervisionPassword' | 'supervisionUrls' | 'supervisionUser' + > +} +await describe('ChargingStation supervision URL validation', async () => { + afterEach(async () => { + // A station built with persistence writes its configuration from an + // AsyncLock continuation, i.e. after this synchronous teardown point. + // Drain it so `cleanupStationTemplates()` removes every temp dir. + await flushMicrotasks() + standardCleanup() + cleanupStationTemplates() + }) + + await it('should reject a malformed supervision URL with a field-naming configuration error', () => { + // A template with a relative or malformed supervision URL must surface a + // structured configuration error, never the bare `TypeError` from `new URL()`. + for (const malformedUrl of ['/relative', MALFORMED_SUPERVISION_URL]) { + assert.throws( + () => + createStationFromTemplate(copyStationTemplate({ supervisionUrls: malformedUrl }), { + baseName: TEST_CHARGING_STATION_BASE_NAME, + fixedName: true, + persistentConfiguration: false, + supervisionUrls: malformedUrl, + }), + { + message: `${TEST_CHARGING_STATION_BASE_NAME}: Invalid supervision url '${malformedUrl}' configured in 'supervisionUrls'`, + name: 'BaseError', + } + ) + } + }) + + await it('should revert stationInfo.supervisionUrls when setSupervisionUrl rejects the URL', () => { + const station = createStationFromTemplate(copyStationTemplate(), { + baseName: TEST_CHARGING_STATION_BASE_NAME, + fixedName: true, + persistentConfiguration: false, + }) + const initialSupervisionUrls = station.stationInfo?.supervisionUrls + + assert.throws( + () => { + station.setSupervisionUrl(MALFORMED_SUPERVISION_URL) + }, + { + message: `${TEST_CHARGING_STATION_BASE_NAME}: Invalid supervision url '${MALFORMED_SUPERVISION_URL}' configured in 'supervisionUrls'`, + name: 'BaseError', + } + ) + assert.strictEqual(station.stationInfo?.supervisionUrls, initialSupervisionUrls) + // The retained creation options are only mirrored on success, so a rejected + // update must not leak into the reset/template-reload path either. + assert.strictEqual( + stationInternalsOf(station).creationOptions?.supervisionUrls, + initialSupervisionUrls + ) + }) + + await it('should reject a malformed URL on the OCPP-configuration supervision branch', () => { + // This branch writes the URL into an OCPP configuration key and never calls + // getConfiguredSupervisionUrl, so it needs its own validation: a value that + // is not checked here would surface later as a bare TypeError from + // wsConnectionUrl. + const station = createStationFromTemplate( + copyStationTemplate({ + supervisionUrlOcppConfiguration: true, + supervisionUrlOcppKey: OCPP_SUPERVISION_URL_KEY, + }), + { baseName: TEST_CHARGING_STATION_BASE_NAME, fixedName: true, persistentConfiguration: false } + ) + const initialWsConnectionUrl = station.wsConnectionUrl.href + + assert.throws( + () => { + station.setSupervisionUrl(MALFORMED_SUPERVISION_URL) + }, + { + message: `${TEST_CHARGING_STATION_BASE_NAME}: Invalid supervision url '${MALFORMED_SUPERVISION_URL}' configured in '${OCPP_SUPERVISION_URL_KEY}'`, + name: 'BaseError', + } + ) + // The rejected value must not have reached the OCPP configuration key. + assert.strictEqual(station.wsConnectionUrl.href, initialWsConnectionUrl) + }) + + await it('should reject a malformed supervision URL seeded in the OCPP configuration key', () => { + // The OCPP configuration key can carry the supervision URL without going + // through setSupervisionUrl (template Configuration, or an OCPP + // ChangeConfiguration). The wsConnectionUrl getter is the only place that + // parses it, so it must raise the structured error too. + const station = createStationFromTemplate( + copyStationTemplate({ + Configuration: { + configurationKey: [ + { + key: OCPP_SUPERVISION_URL_KEY, + readonly: false, + value: MALFORMED_SUPERVISION_URL, + }, + ], + }, + supervisionUrlOcppConfiguration: true, + supervisionUrlOcppKey: OCPP_SUPERVISION_URL_KEY, + }), + { baseName: TEST_CHARGING_STATION_BASE_NAME, fixedName: true, persistentConfiguration: false } + ) + + assert.throws(() => station.wsConnectionUrl, { + message: `${TEST_CHARGING_STATION_BASE_NAME}: Invalid supervision url '${MALFORMED_SUPERVISION_URL}' configured in '${OCPP_SUPERVISION_URL_KEY}'`, + name: 'BaseError', + }) + }) + + await it('should normalize supervision URLs in a worker and preserve persisted state on rejection', async () => { + // Arrange: a real worker exercises the updated event and both storage branches. + const url = 'wss://EXAMPLE.org:443 ' + const updatedUrls: string[] = [] + const worker = new Worker(new URL('./fixtures/supervisionUrlWorker.mjs', import.meta.url), { + execArgv: [], + workerData: { + key: OCPP_SUPERVISION_URL_KEY, + stationId: TEST_CHARGING_STATION_BASE_NAME, + url, + }, + }) + try { + // Act: the fixture performs a valid update, then a rejected update, and drains persistence. + const scenarios = await new Promise((resolve, reject) => { + worker.on('message', (message: SupervisionUrlWorkerMessage) => { + if (message.event === 'supervisionUrlResult') { + resolve(message.data) + } else if (message.event === 'supervisionUrlFailure') { + const error = new BaseError(message.data.message) + error.stack = message.data.stack + reject(error) + } else if (message.event === ChargingStationWorkerMessageEvents.updated) { + updatedUrls.push(message.data.supervisionUrl) + } + }) + worker.once('error', reject) + worker.once('exit', code => { + reject( + new BaseError( + `Supervision URL worker exited before returning results (code ${code.toString()})` + ) + ) + }) + }) + + // Assert: successful normalization preserves raw reset options and rejected updates change nothing. + assert.deepStrictEqual( + scenarios.map(scenario => scenario.supervisionUrlOcppConfiguration), + [false, true] + ) + assert.deepStrictEqual(updatedUrls, [ + `wss://example.org/${TEST_CHARGING_STATION_BASE_NAME}`, + `wss://example.org/${TEST_CHARGING_STATION_BASE_NAME}`, + ]) + for (const scenario of scenarios) { + const field = scenario.supervisionUrlOcppConfiguration + ? OCPP_SUPERVISION_URL_KEY + : 'supervisionUrls' + assert.strictEqual( + scenario.accepted.connectionUrl, + `wss://example.org/${TEST_CHARGING_STATION_BASE_NAME}` + ) + assert.strictEqual(scenario.accepted.creationOptions?.supervisionUrls, url) + assert.strictEqual(scenario.accepted.creationOptions.supervisionUser, 'new-user') + assert.strictEqual(scenario.accepted.creationOptions.supervisionPassword, '') + assert.strictEqual(scenario.accepted.stationInfo.supervisionUser, 'new-user') + assert.strictEqual(scenario.accepted.stationInfo.supervisionPassword, '') + const persisted = JSON.parse(scenario.persistedBefore) as ChargingStationConfiguration + assert.strictEqual(persisted.stationInfo?.supervisionUser, 'new-user') + assert.strictEqual(persisted.stationInfo.supervisionPassword, '') + if (scenario.supervisionUrlOcppConfiguration) { + assert.strictEqual(scenario.accepted.ocppUrl, 'wss://example.org/') + assert.strictEqual( + persisted.configurationKey?.find(key => key.key === OCPP_SUPERVISION_URL_KEY)?.value, + 'wss://example.org/' + ) + } else { + assert.strictEqual(scenario.accepted.stationInfo.supervisionUrls, url) + assert.strictEqual(persisted.stationInfo.supervisionUrls, url) + } + assert.deepStrictEqual(scenario.rejection, { + message: `${TEST_CHARGING_STATION_BASE_NAME}: Invalid supervision url '${MALFORMED_SUPERVISION_URL}' configured in '${field}'`, + name: 'BaseError', + }) + assert.deepStrictEqual(scenario.rejected, scenario.accepted) + assert.strictEqual(scenario.persistedAfter, scenario.persistedBefore) + } + } finally { + await worker.terminate() + } + }) + await it('should accept a valid supervision URL on every entry path', () => { + const station = createStationFromTemplate(copyStationTemplate(), { + baseName: TEST_CHARGING_STATION_BASE_NAME, + fixedName: true, + persistentConfiguration: false, + }) + + station.setSupervisionUrl(TEST_SUPERVISION_URL_ALT_PORT) + assert.strictEqual( + station.wsConnectionUrl.href, + `${TEST_SUPERVISION_URL_ALT_PORT}/${TEST_CHARGING_STATION_BASE_NAME}` + ) + }) +}) diff --git a/tests/charging-station/fixtures/supervisionUrlWorker.mjs b/tests/charging-station/fixtures/supervisionUrlWorker.mjs new file mode 100644 index 00000000..23f11ff7 --- /dev/null +++ b/tests/charging-station/fixtures/supervisionUrlWorker.mjs @@ -0,0 +1,84 @@ +/** + * @file Real-worker supervision URL update scenarios. + * @description Exercises the updated payload and persisted configuration without opening a socket. + */ +import { readFileSync } from 'node:fs' +import { parentPort, workerData } from 'node:worker_threads' +// eslint-disable-next-line n/no-unpublished-import -- Test-only TypeScript loader. +import { register } from 'tsx/esm/api' + +const unregister = register() +const { getConfigurationKey } = + await import('../../../src/charging-station/ConfigurationKeyUtils.ts') +const { flushMicrotasks, standardCleanup } = await import('../../helpers/TestLifecycleHelpers.ts') +const { + cleanupStationTemplates, + copyStationTemplate, + createStationFromTemplate, + resolvePersistedConfigurationFile, +} = await import('../helpers/StationHelpers.realStation.ts') + +const scenarios = [] +let station +let failure +try { + for (const supervisionUrlOcppConfiguration of [false, true]) { + const template = copyStationTemplate({ + supervisionUrlOcppConfiguration, + supervisionUrlOcppKey: workerData.key, + }) + station = createStationFromTemplate(template, { + baseName: workerData.stationId, + fixedName: true, + persistentConfiguration: true, + supervisionPassword: 'old-password', + supervisionUser: 'old-user', + }) + await station.pendingConfigurationSave + const captureState = () => ({ + connectionUrl: station.wsConnectionUrl.href, + creationOptions: structuredClone(station.creationOptions), + ocppUrl: getConfigurationKey(station, workerData.key)?.value, + stationInfo: { + supervisionPassword: station.stationInfo?.supervisionPassword, + supervisionUrls: station.stationInfo?.supervisionUrls, + supervisionUser: station.stationInfo?.supervisionUser, + }, + }) + + // In a worker, updated synchronously builds a payload that reads wsConnectionUrl. + station.setSupervisionUrl(workerData.url, 'new-user', '') + await station.pendingConfigurationSave + const accepted = captureState() + const configurationFile = resolvePersistedConfigurationFile(template) + const persistedBefore = readFileSync(configurationFile, 'utf8') + let rejection + try { + station.setSupervisionUrl('not a url', 'rejected-user', 'rejected-password') + } catch (error) { + rejection = { message: error.message, name: error.name } + } + await station.pendingConfigurationSave + scenarios.push({ + accepted, + persistedAfter: readFileSync(configurationFile, 'utf8'), + persistedBefore, + rejected: captureState(), + rejection, + supervisionUrlOcppConfiguration, + }) + } +} catch (error) { + failure = { message: error.message, stack: error.stack } +} finally { + await station?.pendingConfigurationSave + await flushMicrotasks() + standardCleanup() + cleanupStationTemplates() + unregister() +} +parentPort?.postMessage( + failure == null + ? { data: scenarios, event: 'supervisionUrlResult' } + : { data: failure, event: 'supervisionUrlFailure' } +) -- 2.53.0