Tightening CallError validation by adding structure checks and normalizing input formats

Author: sfkelseylee

src/Ocpp/JsonSchemaValidator.php

@@ -22,14 +22,17 @@ class JsonSchemaValidator
     public static function validate(Call|CallResult|CallError|array $message, string $version, bool $throwException = true): bool|CallError
     {
         // CallError messages represent protocol-level error responses (e.g., NotImplemented).
-        // They have no payload to validate — the error code itself IS the message.
+        // They have no JSON schema to validate against — instead we verify they parse
+        // into a well-formed CallError with a recognized error structure.
         if ($message instanceof CallError) {
-            return true;
+            return self::validateCallErrorStructure($message, $throwException);
         }
 
-        // Raw array with messageTypeID=4 is also a CallError — skip validation.
+        // Raw array with messageTypeID=4 is a CallError. Parse it first so we validate
+        // its structure rather than blindly returning true.
         if (is_array($message) && ($message['messageTypeID'] ?? $message[0] ?? null) === 4) {
-            return true;
+            $callError = self::normalizeCallErrorArray($message);
+            return self::validateCallErrorStructure($callError, $throwException);
         }
 
         $registry = new JsonSchemaRegistry();
@@ -281,4 +284,79 @@ private static function resolveSchemaReference(string $reference, array $rootSch
 
         return is_array($current) ? $current : [];
     }
+
+    /**
+     * Validate that a CallError has a well-formed structure.
+     *
+     * CallError messages don't have a JSON schema to validate against, but we
+     * verify basic structural integrity: a non-empty errorCode, a string
+     * errorDescription, and an array errorDetails.
+     */
+    private static function validateCallErrorStructure(CallError $callError, bool $throwException): bool|CallError
+    {
+        $errors = [];
+
+        if (!is_string($callError->errorCode) || $callError->errorCode === '') {
+            $errors[] = 'The property errorCode is required and must be a non-empty string';
+        }
+
+        if (isset($callError->errorDescription) && !is_string($callError->errorDescription)) {
+            $errors[] = 'The property errorDescription must be a string';
+        }
+
+        if (!is_array($callError->errorDetails)) {
+            $errors[] = 'The property errorDetails must be an array';
+        }
+
+        if ($errors !== []) {
+            $protocolError = new ProtocolError(
+                $callError->uniqueId,
+                null,
+                null,
+                $errors
+            );
+
+            if ($throwException) {
+                throw new \Exception(self::formatValidationExceptionMessage($protocolError));
+            }
+
+            return $protocolError;
+        }
+
+        return true;
+    }
+
+    /**
+     * Normalize a raw CallError array into a CallError object.
+     *
+     * Handles both the raw OCPP wire format [4, uniqueId, errorCode, ...]
+     * and the parsed message format ['messageTypeID' => 4, ...].
+     */
+    private static function normalizeCallErrorArray(array $message): CallError
+    {
+        // Raw OCPP wire format: [4, uniqueId, errorCode, errorDescription, errorDetails]
+        if (isset($message[0]) && $message[0] === 4) {
+            return CallError::fromArray($message);
+        }
+
+        // Parsed message format from parseJsonMessage:
+        // ['messageTypeID' => 4, 'uniqueId' => '...', 'action' => '...', 'payload' => [...]]
+        $uniqueId = $message['uniqueId'] ?? '';
+        $errorCode = $message['action'] ?? $message['errorCode'] ?? '';
+        $payload = $message['payload'] ?? [];
+
+        // The payload in parsed format may contain errorDescription and errorDetails
+        $errorDescription = '';
+        $errorDetails = [];
+
+        if (is_array($payload)) {
+            $errorDescription = $payload['errorDescription'] ?? '';
+            $errorDetails = $payload['errorDetails'] ?? [];
+        }
+
+        // Rebuild into raw format for CallError::fromArray
+        $raw = [4, $uniqueId, $errorCode, $errorDescription, $errorDetails];
+
+        return CallError::fromArray($raw);
+    }
 }

tests/CallErrorTest.php

@@ -219,7 +219,7 @@ public function test_to_array_roundtrip_with_details(): void
         $this->assertEquals((object) ['detail' => 'extra'], $array[4]);
     }
 
-    public function test_validator_skips_call_error_objects(): void
+    public function test_validator_accepts_well_formed_call_error_objects(): void
     {
         $error = new NotImplementedError('test-id');
 
@@ -228,7 +228,7 @@ public function test_validator_skips_call_error_objects(): void
         $this->assertTrue($result);
     }
 
-    public function test_validator_skips_call_error_arrays(): void
+    public function test_validator_accepts_call_error_parsed_format(): void
     {
         $rawCallError = [
             'messageTypeID' => 4,
@@ -242,7 +242,7 @@ public function test_validator_skips_call_error_arrays(): void
         $this->assertTrue($result);
     }
 
-    public function test_validator_skips_call_error_array_with_numeric_keys(): void
+    public function test_validator_accepts_call_error_wire_format(): void
     {
         $rawCallError = [4, 'test-id', 'NotImplemented', '', []];
 
@@ -251,6 +251,50 @@ public function test_validator_skips_call_error_array_with_numeric_keys(): void
         $this->assertTrue($result);
     }
 
+    public function test_validator_rejects_call_error_with_empty_error_code(): void
+    {
+        $error = new NotImplementedError('test-id');
+        // Override the errorCode to be empty — simulates a malformed CallError
+        $error->errorCode = '';
+
+        $result = JsonSchemaValidator::validate($error, 'v1.6', false);
+
+        $this->assertInstanceOf(ProtocolError::class, $result);
+        $this->assertStringContainsString('errorCode', $result->errorDetails[0]);
+    }
+
+    public function test_validator_throws_on_malformed_call_error_when_throw_enabled(): void
+    {
+        $error = new NotImplementedError('test-id');
+        $error->errorCode = '';
+
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage('ProtocolError:');
+
+        JsonSchemaValidator::validate($error, 'v1.6', true);
+    }
+
+    public function test_validator_rejects_raw_array_type4_with_empty_error_code(): void
+    {
+        $raw = [4, 'test-id', '', '', []];
+
+        $result = JsonSchemaValidator::validate($raw, 'v1.6', false);
+
+        $this->assertInstanceOf(ProtocolError::class, $result);
+        $this->assertStringContainsString('errorCode', $result->errorDetails[0]);
+    }
+
+    public function test_validator_accepts_unknown_error_codes(): void
+    {
+        // Unknown error codes should still be accepted — they map to UnknownCallErrorCodeError
+        // and are structurally valid (non-empty errorCode string)
+        $raw = [4, 'test-id', 'SomeCustomError', 'Custom message', []];
+
+        $result = JsonSchemaValidator::validate($raw, 'v1.6');
+
+        $this->assertTrue($result);
+    }
+
     public function test_registry_create_from_array_handles_call_error(): void
     {
         $registry = new JsonSchemaRegistry();