Skip to content

Commit 28f11fe

Browse files
google-labs-jules[bot]james2037
authored andcommitted
Fix: Correctly parse JSON-RPC batch requests
The previous implementation incorrectly handled JSON-RPC batch requests, leading to parsing errors when an array of requests was received. This commit addresses the issue by: 1. Modifying `StdioTransport::receive()` to ensure that when a batch request (an array of JSON objects) is detected, it's correctly identified. The raw JSON string is then passed to `JsonRpcMessage`. 2. Modifying `JsonRpcMessage::fromJsonArray()` to first decode the incoming JSON string into an array of `stdClass` objects. It then iterates through this array, calling a new method `fromJsonObject()` for each item. This avoids inefficient re-encoding/decoding. 3. Introducing a new public static method `JsonRpcMessage::fromJsonObject()`. This method takes a `stdClass` object (a single decoded JSON-RPC message) and performs the same validation and object construction logic as the original `fromJson()` method, but operates directly on the object's properties. This includes casting nested structures like `params`, `result`, and `error` to arrays where appropriate to maintain type consistency. 4. Adding comprehensive unit tests for the modified `StdioTransport::receive()` and for the new and modified methods in `JsonRpcMessage`. These tests cover various scenarios, including valid and invalid batch requests, requests, notifications, and responses. These changes ensure that the server can now correctly parse and process JSON-RPC batch requests according to the specification, resolving the "Expected object, received array" ZodError. All linters and unit tests pass with these changes.
1 parent a0d947d commit 28f11fe

File tree

4 files changed

+432
-42
lines changed

4 files changed

+432
-42
lines changed

src/Message/JsonRpcMessage.php

Lines changed: 101 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,104 @@ public static function fromJson(string $json): self
131131
);
132132
}
133133

134+
/**
135+
* Creates a JsonRpcMessage from a pre-decoded stdClass object.
136+
*
137+
* @param \stdClass $data The decoded JSON-RPC message object.
138+
* @return self The created JsonRpcMessage instance.
139+
* @throws \RuntimeException If the object structure is invalid.
140+
*/
141+
public static function fromJsonObject(\stdClass $data): self
142+
{
143+
if (!property_exists($data, 'jsonrpc') || $data->jsonrpc !== '2.0') {
144+
throw new \RuntimeException('Invalid JSON-RPC version', self::INVALID_REQUEST);
145+
}
146+
147+
// Handle response messages (result or error)
148+
if (property_exists($data, 'result') || property_exists($data, 'error')) {
149+
// According to JSON-RPC 2.0 spec, ID must be present for responses,
150+
// and should be the same as the ID of the request.
151+
// It can be null if the request ID was null (though unusual) or if there was an error
152+
// that prevented request ID parsing (e.g. Parse Error/Invalid Request).
153+
// Our constructor new self('', null, $id) expects $id to be string|null.
154+
$id = null;
155+
if (property_exists($data, 'id')) {
156+
// Ensure ID is string or null. Numeric IDs are cast to string.
157+
$id = (is_string($data->id) || is_numeric($data->id)) ? (string)$data->id : null;
158+
}
159+
// If ID is strictly required and not just potentially null:
160+
// if (!property_exists($data, 'id')) {
161+
// throw new \RuntimeException('Response must include ID', self::INVALID_REQUEST);
162+
// }
163+
// $id = (string)$data->id; // Assuming ID here must not be null for a response.
164+
165+
$msg = new self('', null, $id); // Method and params are not relevant for responses.
166+
167+
if (property_exists($data, 'result')) {
168+
// Result can be any type, but we store it in an array field.
169+
// If it's a structured type (object/array), ensure it's an array.
170+
// If it's a scalar, it might be an issue if not wrapped in an array by convention.
171+
// For now, mirroring original behavior: whatever $data->result is, assign it.
172+
// The JsonRpcMessage::$result is type ?array. This implies result should be array or null.
173+
// Let's ensure it's compatible.
174+
if (is_object($data->result) || is_array($data->result)) {
175+
$msg->result = (array)$data->result;
176+
} elseif ($data->result === null) {
177+
$msg->result = null;
178+
} else {
179+
// If result is a scalar, this was not explicitly handled before.
180+
// Wrapping scalar in an array, or throwing error, are options.
181+
// To be safe and expect a "structured value" usually:
182+
throw new \RuntimeException('Response result must be a structured type (object/array) or null.', self::INVALID_REQUEST);
183+
}
184+
} else { // This means property_exists($data, 'error') is true
185+
if (!is_object($data->error)) { // Error member MUST be an object
186+
throw new \RuntimeException('Invalid error object in JSON-RPC response (must be an object)', self::INVALID_REQUEST);
187+
}
188+
// Perform a shallow cast for the main error object
189+
$errorArray = (array)$data->error;
190+
191+
// If the 'data' field within the error object exists and is an object, cast it to an array too
192+
if (isset($errorArray['data']) && is_object($errorArray['data'])) {
193+
$errorArray['data'] = (array)$errorArray['data'];
194+
}
195+
$msg->error = $errorArray;
196+
}
197+
return $msg;
198+
}
199+
200+
// Handle request or notification messages
201+
if (!property_exists($data, 'method') || !is_string($data->method) || empty($data->method)) {
202+
throw new \RuntimeException('Missing, invalid, or empty method name in JSON-RPC request', self::INVALID_REQUEST);
203+
}
204+
205+
$params = null;
206+
if (property_exists($data, 'params')) {
207+
if (is_object($data->params) || is_array($data->params)) {
208+
// JSON-RPC params can be an array (positional) or object (named).
209+
// Our constructor expects ?array. So, convert object to array.
210+
$params = (array)$data->params;
211+
} else {
212+
// Params exist but are not a structured type (object/array)
213+
throw new \RuntimeException('Invalid params: must be object or array if present.', self::INVALID_PARAMS);
214+
}
215+
}
216+
217+
$id = null;
218+
if (property_exists($data, 'id')) {
219+
if (is_string($data->id) || is_numeric($data->id)) {
220+
$id = (string)$data->id;
221+
} elseif ($data->id === null) { // Explicitly null is allowed (fixed phpcs warning)
222+
$id = null; // Explicitly null is allowed
223+
} else {
224+
// ID is present but of an invalid type (e.g., object, array)
225+
throw new \RuntimeException('Invalid ID: must be string, number, or null.', self::INVALID_REQUEST);
226+
}
227+
}
228+
229+
return new self($data->method, $params, $id);
230+
}
231+
134232
/**
135233
* Creates a JSON-RPC success response message.
136234
*
@@ -203,10 +301,9 @@ public static function fromJsonArray(string $json): array
203301

204302
$messages = [];
205303
foreach ($data as $item) {
206-
// Re-encode each item to use the existing fromJson method
207-
// This is less efficient but reuses existing parsing logic
208-
$itemJson = json_encode($item, JSON_THROW_ON_ERROR);
209-
$messages[] = self::fromJson($itemJson);
304+
// $item is already a PHP stdClass object from the initial json_decode.
305+
// Call the new fromJsonObject method directly.
306+
$messages[] = self::fromJsonObject($item);
210307
}
211308

212309
return $messages;

src/Transport/StdioTransport.php

Lines changed: 36 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -57,54 +57,52 @@ public function receive(): ?array
5757
}
5858

5959
try {
60-
$decodedInput = json_decode($line, true, 512, JSON_THROW_ON_ERROR);
61-
62-
// Check for batch request: a non-empty numerically indexed array
63-
if (
64-
is_array($decodedInput)
65-
&& (count($decodedInput) === 0
66-
|| array_keys($decodedInput) === range(0, count($decodedInput) - 1))
67-
) {
68-
if (empty($decodedInput)) {
69-
// Empty array received, spec implies it's invalid for batch,
70-
// but JsonRpcMessage::fromJsonArray will handle this (likely an error or empty messages).
71-
// For consistency, we let fromJsonArray parse it.
72-
return JsonRpcMessage::fromJsonArray($line);
60+
$trimmedLine = trim($line);
61+
$isLikelyBatch = strpos($trimmedLine, '[') === 0;
62+
63+
// Conditional decoding: objects for batch (false), associative arrays for single (true).
64+
// This $decodedInput is used for structural checks before passing the raw string.
65+
$decodedInput = json_decode($trimmedLine, !$isLikelyBatch, 512, JSON_THROW_ON_ERROR);
66+
67+
if ($isLikelyBatch) {
68+
// $decodedInput should be an array (of objects, due to 'false' in json_decode).
69+
if (is_array($decodedInput)) {
70+
// Pass the original string to fromJsonArray, as it expects a string.
71+
return JsonRpcMessage::fromJsonArray($trimmedLine);
72+
} else {
73+
// $trimmedLine started with '[' but didn't decode to a PHP array.
74+
throw new \RuntimeException(
75+
'Invalid JSON-RPC batch structure. Expected a JSON array after decoding.',
76+
JsonRpcMessage::PARSE_ERROR
77+
);
78+
}
79+
} else { // Single request
80+
if (is_array($decodedInput) && !empty($decodedInput)) { // Check for non-empty assoc array
81+
$message = JsonRpcMessage::fromJson($trimmedLine);
82+
return [$message];
83+
} elseif (is_object($decodedInput)) { // Should not happen if assoc is true typically, but check for robustness
84+
$message = JsonRpcMessage::fromJson($trimmedLine);
85+
return [$message];
86+
} else {
87+
// $decodedInput is not a non-empty associative array or an object.
88+
// This uses the generic message that tests expect.
89+
throw new \RuntimeException(
90+
'Invalid JSON-RPC message structure. Expected object or array of objects.',
91+
JsonRpcMessage::PARSE_ERROR
92+
);
7393
}
74-
// It's a batch, let fromJsonArray handle full parsing
75-
// and validation from original string
76-
return JsonRpcMessage::fromJsonArray($line);
77-
} elseif (
78-
is_object($decodedInput) // Decoded as object if not assoc=true
79-
|| (is_array($decodedInput) && !empty($decodedInput)) // Decoded as non-empty assoc array
80-
) {
81-
// It's a single request (object) or potentially an associative
82-
// array representing a single request.
83-
// Let fromJson handle full parsing and validation from original string
84-
$message = JsonRpcMessage::fromJson($line);
85-
return [$message];
86-
} else {
87-
// Invalid structure that is not explicitly an empty array
88-
// or a valid single/batch candidate
89-
throw new \RuntimeException(
90-
'Invalid JSON-RPC message structure. Expected object or array of objects.',
91-
JsonRpcMessage::PARSE_ERROR
92-
);
9394
}
94-
} catch (\JsonException $e) {
95+
} catch (\JsonException $e) { // Catch JSON parsing errors first
9596
throw new \RuntimeException(
9697
'JSON Parse Error: ' . $e->getMessage(),
9798
JsonRpcMessage::PARSE_ERROR,
9899
$e
99100
);
100-
} catch (\Exception $e) {
101-
// Catch other exceptions from JsonRpcMessage::fromJson or fromJsonArray
101+
} catch (\Exception $e) { // Catch other exceptions from JsonRpcMessage processing
102102
$this->log("Error processing received message: " . $e->getMessage());
103-
// Depending on desired behavior, re-throw or return error specific message
104-
// For now, let's re-throw as a runtime exception consistent with parse errors
105103
throw new \RuntimeException(
106104
'Error parsing JSON-RPC message: ' . $e->getMessage(),
107-
JsonRpcMessage::INVALID_REQUEST, // Or PARSE_ERROR if more appropriate
105+
JsonRpcMessage::INVALID_REQUEST,
108106
$e
109107
);
110108
}

0 commit comments

Comments
 (0)