From: Jérôme Benoit Date: Fri, 17 Jul 2026 19:58:43 +0000 (+0200) Subject: fix(ocpp): reject charging profiles by EVSE existence and isolate non-persistent... X-Git-Tag: cli@v4.11.0~21 X-Git-Url: https://git.piment-noir.org/?a=commitdiff_plain;h=aba537580d721ce053ba681047f8950205a7a4c8;p=e-mobility-charging-stations-simulator.git fix(ocpp): reject charging profiles by EVSE existence and isolate non-persistent station configuration (#2022, #2024) (#2025) * fix(ocpp): reject charging profiles by EVSE existence, not EVSE count (#2022) OCPP 2.0 validateChargingProfile() guarded evseId with a count comparison (evseId > getNumberOfEvses()). getNumberOfEvses() counts EVSEs (excluding EVSE 0) and is not a maximum id, so a station with non-contiguous EVSE ids {0,1,3} false-rejected a valid profile for EVSE 3 (3 > 2). Use the existing hasEvse() existence check instead, mirroring the OCPP 1.6 hasConnector twin, and align the message to "EVSE does not exist". RequestStartTransaction carries the TxProfile (F01.FR.08-10 / F02.FR.16-18); existence-based rejection mirrors K01.FR.28 (conformance test TC_K_14_CS). Test: a charging profile for the existing, non-contiguous EVSE 3 is accepted (a count-based guard would reject it, 3 > 2). Mutation-verified: reverting the guard makes the test fail. Signed-off-by: Jérôme Benoit * fix(charging-station): clone template OCPP configuration to isolate non-persistent stations (#2024) getOcppConfigurationFromTemplate() returned the SharedLRUCache-cached template's Configuration by reference. For non-persistent stations (ocppPersistentConfiguration=false) this aliased the shared configurationKey array, so an in-place setConfigurationKeyValue() mutation polluted the cached template. Clone the Configuration, mirroring the existing connector/EVSE clone pattern (clone(undefined) === undefined). Test: a non-persistent station's Configuration is an independent copy of the cached template, and mutating a key on it leaves that cached template unchanged. Mutation-verified: reverting the clone makes the test fail. Retires the now-redundant SharedLRUCache-clearing afterEach workaround in ChargingStation-ResetIdentity.test.ts (file still passes). Signed-off-by: Jérôme Benoit --------- Signed-off-by: Jérôme Benoit --- diff --git a/src/charging-station/ChargingStation.ts b/src/charging-station/ChargingStation.ts index 30fc6d1d..3dc9d512 100644 --- a/src/charging-station/ChargingStation.ts +++ b/src/charging-station/ChargingStation.ts @@ -1627,7 +1627,7 @@ export class ChargingStation extends EventEmitter { } private getOcppConfigurationFromTemplate (): ChargingStationOcppConfiguration | undefined { - return this.getTemplateFromFile()?.Configuration + return clone(this.getTemplateFromFile()?.Configuration) } private getPowerDivider (): number { diff --git a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts index 1e90cbb4..286e7007 100644 --- a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts +++ b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts @@ -4495,9 +4495,9 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService chargingStation.getNumberOfEvses()) { + if (chargingStation.hasEvses && !chargingStation.hasEvse(evseId)) { logger.warn( - `${chargingStation.logPrefix()} ${moduleName}.validateChargingProfile: EVSE ${evseId.toString()} exceeds available EVSEs (${chargingStation.getNumberOfEvses().toString()})` + `${chargingStation.logPrefix()} ${moduleName}.validateChargingProfile: EVSE ${evseId.toString()} does not exist` ) return false } diff --git a/tests/charging-station/ChargingStation-ConfigurationTemplateIsolation.test.ts b/tests/charging-station/ChargingStation-ConfigurationTemplateIsolation.test.ts new file mode 100644 index 00000000..224e43e8 --- /dev/null +++ b/tests/charging-station/ChargingStation-ConfigurationTemplateIsolation.test.ts @@ -0,0 +1,94 @@ +/** + * @file Tests that a non-persistent charging station owns an independent OCPP Configuration. + * @description getOcppConfigurationFromTemplate() must clone the cached template's + * Configuration so a runtime configuration-key change on a non-persistent station stays + * local and does not mutate the shared template held in the SharedLRUCache. + */ +import assert from 'node:assert/strict' +import { copyFileSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, it } from 'node:test' + +import type { ChargingStationTemplate } from '../../src/types/index.js' + +import { ChargingStation } from '../../src/charging-station/ChargingStation.js' +import { + getConfigurationKey, + setConfigurationKeyValue, +} from '../../src/charging-station/ConfigurationKeyUtils.js' +import { SharedLRUCache } from '../../src/charging-station/SharedLRUCache.js' +import { standardCleanup } from '../helpers/TestLifecycleHelpers.js' + +// templateFileHash is the SharedLRUCache key; reached via a typed boundary cast (no `as any`). +const templateHashOf = (station: ChargingStation): string => + (station as unknown as { templateFileHash: string }).templateFileHash + +const configValue = (template: ChargingStationTemplate, key: string): string | undefined => + template.Configuration?.configurationKey?.find(configKey => configKey.key === key)?.value + +// A pre-existing, mutable key shipped by the virtual-simple template. +const CONFIG_KEY = 'MeterValueSampleInterval' +const TEMPLATE_VALUE = '30' + +const tmpRoots: string[] = [] + +// Fresh template in its own temp station-templates dir. A station caches its parsed template +// in the SharedLRUCache under a content-derived key; templateHashOf() fetches that exact entry +// so the test can assert the station's Configuration is an independent copy of it. +const makeTemplate = (): string => { + const root = mkdtempSync(join(tmpdir(), 'cs-config-isolation-')) + tmpRoots.push(root) + mkdirSync(join(root, 'station-templates'), { recursive: true }) + const file = join(root, 'station-templates', 'virtual-simple.station-template.json') + copyFileSync( + join(process.cwd(), 'src/assets/station-templates/virtual-simple.station-template.json'), + file + ) + return file +} + +await describe('ChargingStation OCPP Configuration isolation', async () => { + afterEach(() => { + standardCleanup() + for (const root of tmpRoots.splice(0)) { + rmSync(root, { force: true, recursive: true }) + } + }) + + await it('should not mutate the shared cached template when a non-persistent station changes a configuration key', () => { + const templateFile = makeTemplate() + const station = new ChargingStation(1, templateFile, { + autoStart: false, + persistentConfiguration: false, + supervisionUrls: 'ws://localhost:9999/', + }) + + // The exact cached template the station parsed and read its Configuration from. + const cachedTemplate = SharedLRUCache.getInstance().getChargingStationTemplate( + templateHashOf(station) + ) + assert.strictEqual(configValue(cachedTemplate, CONFIG_KEY), TEMPLATE_VALUE) + + // A non-persistent station must own an independent copy, not the cached template's array. + // Assert both operands are present first so the reference check cannot pass vacuously. + assert.ok( + station.ocppConfiguration?.configurationKey != null, + 'station ocppConfiguration must be initialized' + ) + assert.ok( + cachedTemplate.Configuration?.configurationKey != null, + 'cached template Configuration must be present' + ) + assert.notStrictEqual( + station.ocppConfiguration.configurationKey, + cachedTemplate.Configuration.configurationKey + ) + + setConfigurationKeyValue(station, CONFIG_KEY, '999') + + assert.strictEqual(getConfigurationKey(station, CONFIG_KEY)?.value, '999') + // The shared cached template is untouched. + assert.strictEqual(configValue(cachedTemplate, CONFIG_KEY), TEMPLATE_VALUE) + }) +}) diff --git a/tests/charging-station/ChargingStation-ResetIdentity.test.ts b/tests/charging-station/ChargingStation-ResetIdentity.test.ts index b6453a6a..fea520f9 100644 --- a/tests/charging-station/ChargingStation-ResetIdentity.test.ts +++ b/tests/charging-station/ChargingStation-ResetIdentity.test.ts @@ -90,10 +90,6 @@ const waitForPersistedId = async ( await describe('ChargingStation keeps its identity across a reset', async () => { afterEach(() => { standardCleanup() - // The real ChargingStation uses the real SharedLRUCache singleton (not the - // mock standardCleanup resets); clear it so a cached template cannot bleed - // between tests. - SharedLRUCache.getInstance().clear() for (const root of tmpRoots.splice(0)) { rmSync(root, { force: true, recursive: true }) } @@ -217,9 +213,10 @@ await describe('ChargingStation keeps its identity across a reset', async () => await station.reset() - // A plain reset() reuses the warm template cache, whose in-place OCPP-key - // mutation still carries the URL; the retained-options mirror is not exercised - // on this path (see the cache-cold reload test below). + // A plain reset() reuses the warm template cache. The station's OCPP + // Configuration is an independent clone, so setSupervisionUrl's key mutation no longer + // persists in the cached template; on reinitialization the URL is re-seeded from the + // retained-options mirror (as in the cache-cold reload test below). assert.strictEqual(station.wsConnectionUrl.host, 'localhost:7777') }) @@ -237,8 +234,8 @@ await describe('ChargingStation keeps its identity across a reset', async () => // Invalidate the cached template exactly as the template-reload path does (the // watcher calls deleteChargingStationTemplate before initialize), forcing the // OCPP key absent on reinitialization so it is re-seeded from configuredSupervisionUrl. - // This is the only path where the retained-options mirror is load-bearing: a - // warm-cache reset passes via the cached mutation and would not guard it. + // The warm-cache reset (above) also re-seeds from the retained-options + // mirror; this test additionally covers the explicit template-reload (cache-cold) path. SharedLRUCache.getInstance().clear() internalsOf(station).initialize(internalsOf(station).creationOptions) diff --git a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStartTransaction.test.ts b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStartTransaction.test.ts index 22c3c0c9..d5df6670 100644 --- a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStartTransaction.test.ts +++ b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-RequestStartTransaction.test.ts @@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, it } from 'node:test' import type { ChargingStation } from '../../../../src/charging-station/index.js' import type { + EvseStatus, OCPP20ChargingProfileType, OCPP20ChargingRateUnitEnumType, OCPP20RequestStartTransactionRequest, @@ -56,6 +57,10 @@ import { resetReportingValueSize, } from './OCPP20TestUtils.js' +// EVSE map access to build a non-contiguous EVSE layout; typed boundary cast (no `as any`). +const evsesOf = (station: ChargingStation): Map => + (station as unknown as { evses: Map }).evses + await describe('F01 & F02 - Remote Start Transaction', async () => { let mockStation: ChargingStation let incomingRequestService: OCPP20IncomingRequestService @@ -410,6 +415,51 @@ await describe('F01 & F02 - Remote Start Transaction', async () => { ) }) + // RequestStartTransaction may carry a TxProfile for the target EVSE (OCPP 2.0.1 + // part 2 F01.FR.08-10 / F02.FR.16-18). validateChargingProfile must reject it by EVSE + // existence, not by count: getNumberOfEvses() counts EVSEs (excluding EVSE 0) and is not + // a maximum id, so a count-based guard false-rejects existing but non-contiguous EVSEs. + // Existence-based rejection mirrors K01.FR.28 (conformance test TC_K_14_CS). + await it('should accept a charging profile for an existing non-contiguous EVSE', async () => { + // Arrange: remove EVSE 2 so EVSE ids are non-contiguous {0, 1, 3}. getNumberOfEvses() + // then reports 2 while the existing target id is 3 — the exact shape a count-based guard + // wrongly rejects but an existence check accepts. + evsesOf(mockStation).delete(2) + assert.strictEqual(mockStation.getNumberOfEvses(), 2) + assert.strictEqual(mockStation.hasEvse(3), true) + + const chargingProfile: OCPP20ChargingProfileType = { + chargingProfileKind: OCPP20ChargingProfileKindEnumType.Relative, + chargingProfilePurpose: OCPP20ChargingProfilePurposeEnumType.TxProfile, + chargingSchedule: [ + { + chargingRateUnit: 'A' as OCPP20ChargingRateUnitEnumType, + chargingSchedulePeriod: [ + { + limit: 30, + startPeriod: 0, + }, + ], + id: 1, + }, + ], + id: 1, + stackLevel: 0, + } + + const response = await testableService.handleRequestStartTransaction(mockStation, { + chargingProfile, + evseId: 3, + idToken: { + idToken: 'NON_CONTIGUOUS_EVSE_TOKEN', + type: OCPP20IdTokenEnumType.ISO14443, + }, + remoteStartId: 322, + }) + assert.strictEqual(response.status, RequestStartStopStatusEnumType.Accepted) + assert.notStrictEqual(response.transactionId, undefined) + }) + // FR: F01.FR.09, F01.FR.10 await it('should reject RequestStartTransaction when connector is already occupied', async () => { // First, start a transaction to occupy the connector