From: Jérôme Benoit Date: Fri, 3 Jul 2026 15:51:46 +0000 (+0200) Subject: refactor(ocpp20): rewrite H9/H10/H11 + C10/C12 audit markers as descriptive comments... X-Git-Tag: cli@v4.11.0~72 X-Git-Url: https://git.piment-noir.org/?a=commitdiff_plain;h=a68ca80e2390476c8407ee0149b409e444d95bd5;p=e-mobility-charging-stations-simulator.git refactor(ocpp20): rewrite H9/H10/H11 + C10/C12 audit markers as descriptive comments (issue #1936 l) (#1939) Nine inline comments in OCPP20IncomingRequestService.ts (7) and its sibling test file OCPP20IncomingRequestService-UpdateFirmware.test.ts (2) carried internal audit-round finding numbers (H9, H10, H11, C10, C12) as if they were code identifiers. Production-file markers (7) originate from commit 3a6e89f634 'fix(ocpp20): remediate all OCPP 2.0.1 audit findings (#1726)' which predates PR #1935. Test-file markers (2 H11) originate from commit b50f9e5f 'refactor(tests): separate handler/listener tests and remove setTimeout hacks' — same audit-round family, later commit. The audit markers had the shape //[:-)] which is distinct from the legitimate OCPP spec references elsewhere in the repo (B11 - Reset, F06.FR.06, A02.FR.06, E05.FR.09, L01.FR.30, C10.FR.04, C12.FR.05, ...) that all follow .FR.. Each rewrite replaces the marker with a WHY-rationale or BDD-style expectation describing the current-state behavior. See the PR body table for the full before/after list. Verification (POSIX character classes; portable across all git-grep -E builds — the \s shorthand is a PCRE extension not supported by POSIX ERE and silently returns no match on macOS git): git grep -inE '^[[:space:]]*//[[:space:]]*[A-Z]+[0-9]+[]:)-]' -- \ src/ tests/ returns empty. Comment-only change. No runtime behavior change. Test invariant fail: 0, skipped: 6 holds. Closes issue #1936 item (l). --- diff --git a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts index 83a9433d..de66867a 100644 --- a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts +++ b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts @@ -3037,7 +3037,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { `${chargingStation.logPrefix()} ${moduleName}.handleRequestUpdateFirmware: Received UpdateFirmware request with requestId ${requestId.toString()} for location '${firmware.location}'` ) - // C10: Validate signing certificate PEM format if present + // Reject malformed signing certificates up front so the firmware update path handles only parsable PEMs if (isNotEmptyString(firmware.signingCertificate)) { if ( !hasCertificateManager(chargingStation) || @@ -3066,7 +3066,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { ) } - // H10: Cancel any in-progress firmware update + // Cancel any in-progress firmware update; respond with AcceptedCanceled so the new request starts fresh const stationState = this.getStationState(chargingStation) if (stationState.activeFirmwareUpdateAbortController != null) { const previousRequestId = stationState.activeFirmwareUpdateRequestId @@ -3673,7 +3673,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { ): Promise { const { installDateTime, location, retrieveDateTime, signature } = firmware - // H10: Set up abort controller for cancellation support + // Store the abort controller so a subsequent UpdateFirmware can cancel this in-progress update const abortController = new AbortController() const stationState = this.getStationState(chargingStation) stationState.activeFirmwareUpdateAbortController = abortController @@ -3681,7 +3681,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { const checkAborted = (): boolean => abortController.signal.aborted - // C12: If retrieveDateTime is in the future, send DownloadScheduled and wait + // Delay the download until retrieveDateTime; inform the CSMS via DownloadScheduled first const now = Date.now() const retrieveTime = convertToDate(retrieveDateTime)?.getTime() ?? now if (retrieveTime > now) { @@ -3703,7 +3703,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { await sleep(OCPP20Constants.FIRMWARE_STATUS_DELAY_MS) if (checkAborted()) return - // H9: If firmware location is empty or malformed, send DownloadFailed and stop + // Empty or malformed firmware location: simulate the L01.FR.30 download retries, then emit DownloadFailed and stop if (isEmpty(location) || !this.isValidFirmwareLocation(location)) { // L01.FR.30: Simulate download retries before reporting DownloadFailed const maxRetries = retries ?? 0 @@ -3778,7 +3778,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { ) } - // C12: If installDateTime is in the future, send InstallScheduled and wait + // Delay the install until installDateTime; inform the CSMS via InstallScheduled first if (installDateTime != null) { const currentTime = Date.now() const installTime = convertToDate(installDateTime)?.getTime() ?? currentTime @@ -3845,7 +3845,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { requestId ) - // H11: Send SecurityEventNotification for successful firmware update + // Send SecurityEventNotification after a successful firmware update this.sendSecurityEventNotification( chargingStation, 'FirmwareUpdated', diff --git a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts index 21867669..76858b40 100644 --- a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts +++ b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts @@ -405,7 +405,7 @@ await describe('L01/L02 - UpdateFirmware', async () => { await flushMicrotasks() assert.strictEqual(sentRequests[3].payload.status, OCPP20FirmwareStatusEnumType.Installed) - // H11: SecurityEventNotification for FirmwareUpdated + // Expect SecurityEventNotification of type FirmwareUpdated on successful install assert.strictEqual(sentRequests.length, 5) assert.strictEqual( sentRequests[4].command, @@ -723,7 +723,7 @@ await describe('L01/L02 - UpdateFirmware', async () => { await flushMicrotasks() assert.strictEqual(sentRequests[4].payload.status, OCPP20FirmwareStatusEnumType.Installed) - // H11: SecurityEventNotification after Installed + // Expect SecurityEventNotification to be emitted after the Installed status assert.strictEqual(sentRequests.length, 6) assert.strictEqual( sentRequests[5].command,