From de48dcef64b051948df9ef1ae53e16f282883221 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 4 Jun 2025 22:57:33 +0000 Subject: [PATCH] Jules was unable to complete the task in time. Please review the work done so far and provide feedback for Jules to continue. --- src/Server.php | 126 +++++----- src/Transport/AbstractTransport.php | 125 ++++++++++ src/Transport/HttpTransport.php | 227 ++++++++---------- src/Transport/StdioTransport.php | 81 ++----- src/Transport/TransportInterface.php | 10 +- tests/ServerTest.php | 45 ++-- tests/Transport/AbstractTransportTest.php | 161 +++++++++++++ tests/Transport/HttpTransportTest.php | 202 ++++++++++------ tests/Transport/StdioTransportTest.php | 51 ++-- tests/Transport/TestableAbstractTransport.php | 72 ++++++ tests/Transport/TestableHttpTransport.php | 119 +++------ 11 files changed, 766 insertions(+), 453 deletions(-) create mode 100644 tests/Transport/AbstractTransportTest.php create mode 100644 tests/Transport/TestableAbstractTransport.php diff --git a/src/Server.php b/src/Server.php index a107cee..390b1a2 100644 --- a/src/Server.php +++ b/src/Server.php @@ -203,72 +203,58 @@ private function runHttpRequestCycle(): void $responsePayload = null; try { - // HttpTransport->receive() returns array (decoded JSON) or throws TransportException. - // It ensures the payload is a valid JSON object or array of objects. - $rawPayload = $httpTransport->receive(); - - $responseMessages = []; - // HttpTransport's receive method ensures $rawPayload is either an associative array (single) - // or a non-empty list of associative arrays (batch). - $isBatchRequest = array_is_list($rawPayload); - - if ($isBatchRequest) { - foreach ($rawPayload as $rawMessage) { - // $rawMessage is expected to be an associative array here. - try { - $messageJson = json_encode($rawMessage); - if ($messageJson === false) { - // This should be rare if $rawMessage is a valid array from json_decode - throw new \RuntimeException("Failed to re-encode batch item for parsing.", JsonRpcMessage::PARSE_ERROR); - } - $message = JsonRpcMessage::fromJson($messageJson); - $response = $this->processSingleMessage($message); - if ($response) { - $responseMessages[] = $response; - } - } catch (\Throwable $e) { - $id = is_array($rawMessage) && array_key_exists('id', $rawMessage) ? $rawMessage['id'] : null; - // Ensure ID is string or null for JsonRpcMessage::error - $id = is_scalar($id) ? (string)$id : null; - $responseMessages[] = JsonRpcMessage::error(JsonRpcMessage::INTERNAL_ERROR, "Error processing item in batch: " . $e->getMessage(), $id); - } - } - $responsePayload = !empty($responseMessages) ? $responseMessages : null; - } else { // Single request (which is an associative array) - try { - $messageJson = json_encode($rawPayload); - if ($messageJson === false) { - throw new \RuntimeException("Failed to re-encode single request for parsing.", JsonRpcMessage::PARSE_ERROR); - } - $message = JsonRpcMessage::fromJson($messageJson); - $responsePayload = $this->processSingleMessage($message); - } catch (\Throwable $e) { - $id = is_array($rawPayload) && array_key_exists('id', $rawPayload) ? $rawPayload['id'] : null; - // Ensure ID is string or null for JsonRpcMessage::error - $id = is_scalar($id) ? (string)$id : null; - $errorCode = JsonRpcMessage::INTERNAL_ERROR; // Default - if ($e instanceof \MCP\Server\Exception\MethodNotSupportedException) { - $errorCode = JsonRpcMessage::METHOD_NOT_FOUND; - } elseif ($e instanceof \MCP\Server\Exception\InvalidRequestException) { - $errorCode = JsonRpcMessage::INVALID_REQUEST; - } elseif ($e instanceof \MCP\Server\Exception\InvalidParamsException) { - $errorCode = JsonRpcMessage::INVALID_PARAMS; - } elseif ($e instanceof \RuntimeException && $e->getCode() !== 0 && is_int($e->getCode())) { - $errorCode = $e->getCode(); - } - $responsePayload = JsonRpcMessage::error($errorCode, "Error processing request: " . $e->getMessage(), $id); + // HttpTransport::receive() now returns JsonRpcMessage[]|null|false or throws TransportException. + $messagesReceived = $httpTransport->receive(); + $isBatchResponse = false; // Default: assume response is not a batch unless determined otherwise. + + if ($messagesReceived === false) { + // Transport validation failed (e.g., wrong HTTP method, wrong Content-Type). + $this->logMessage('error', 'HttpTransport->receive() returned false, request unsuitable.', 'Server.runHttpRequestCycle'); + $responsePayload = JsonRpcMessage::error(JsonRpcMessage::INVALID_REQUEST, 'Invalid request (transport level error).', null); + } elseif ($messagesReceived === null) { + // Empty or whitespace-only body, parseMessages returned null. + $this->logMessage('error', 'HttpTransport->receive() returned null (empty request body).', 'Server.runHttpRequestCycle'); + $responsePayload = JsonRpcMessage::error(JsonRpcMessage::INVALID_REQUEST, 'Request body was empty or contained only whitespace.', null); + } else { + // $messagesReceived is JsonRpcMessage[] (can be an empty array for "[]" batch from parseMessages) + $isBatchResponse = $httpTransport->lastRequestAppearedAsBatch(); + $responseMessages = $this->processMessageBatch($messagesReceived); // Use the new helper + + if ($isBatchResponse) { + // For a batch request, always respond with an array of responses. + // $responseMessages is already JsonRpcMessage[] from processMessageBatch. + // If all were notifications, $responseMessages will be empty, resulting in "[]" response. + $responsePayload = $responseMessages; + } elseif (!empty($responseMessages)) { + // Single request that yielded a response. + $responsePayload = $responseMessages[0]; + } else { + // Single request that was a notification (no response generated), + // or an empty batch "[]" that didn't appear as a batch (edge case, defensive). + // $responsePayload remains null. + $responsePayload = null; } } } catch (TransportException $e) { $this->logMessage('error', 'TransportException in HTTP cycle: ' . $e->getMessage(), 'Server.runHttpRequestCycle', ['code' => $e->getCode()]); $errorCode = (is_int($e->getCode()) && $e->getCode() !== 0) ? $e->getCode() : JsonRpcMessage::INTERNAL_ERROR; - $responsePayload = JsonRpcMessage::error($errorCode, $e->getMessage(), null); // ID is null for transport errors not tied to a specific request ID + $responsePayload = JsonRpcMessage::error($errorCode, $e->getMessage(), null); + $isBatchResponse = false; // Not a batch response in case of these transport errors before processing } catch (\Throwable $e) { // Catchall for unexpected errors $this->logMessage('critical', "Critical Server Error in HTTP cycle: " . $e->getMessage(), 'Server.runHttpRequestCycle', ['trace' => $e->getTraceAsString()]); - $responsePayload = JsonRpcMessage::error(JsonRpcMessage::INTERNAL_ERROR, 'Internal server error.', null); // ID is null + $responsePayload = JsonRpcMessage::error(JsonRpcMessage::INTERNAL_ERROR, 'Internal server error.', null); + $isBatchResponse = false; // Not a batch response for critical internal errors } - $httpTransport->send($responsePayload); // $responsePayload could be single JsonRpcMessage or array of them, or null for no response + // Only call send if there's a non-null payload OR if it's a batch response (which might be an empty array for all-notification batches). + if ($responsePayload !== null || $isBatchResponse) { + /** @var JsonRpcMessage|JsonRpcMessage[] $responsePayload PHPStan type hint */ + // If $isBatchResponse is true, $responsePayload is guaranteed to be an array (possibly empty from processMessageBatch). + // If $responsePayload is not null and $isBatchResponse is false, it's a JsonRpcMessage. + $httpTransport->send($responsePayload); + } + // If $responsePayload is null AND $isBatchResponse is false (e.g. single notification), send() is not called. + // HttpTransport->getResponse() will then provide a default (e.g. HTTP 204 or empty 200). $httpResponse = $httpTransport->getResponse(); // This will always return a ResponseInterface if (!headers_sent()) { @@ -293,6 +279,32 @@ private function runHttpRequestCycle(): void } } + /** + * Processes a batch of received JsonRpcMessage objects. + * + * @param JsonRpcMessage[] $receivedMessages An array of messages to process. + * This array can be empty if an empty batch "[]" was received. + * @return JsonRpcMessage[] An array of response messages. Empty if all messages were notifications or errors for notifications. + */ + private function processMessageBatch(array $receivedMessages): array + { + $responseMessages = []; + if (empty($receivedMessages)) { + // If an empty batch "[]" was received and parsed by AbstractTransport::parseMessages, + // $receivedMessages will be an empty array. No processing needed. + return []; + } + + foreach ($receivedMessages as $currentMessage) { + // Type safety: $currentMessage should already be JsonRpcMessage + // due to the return type of AbstractTransport::parseMessages. + $response = $this->processSingleMessage($currentMessage); + if ($response) { + $responseMessages[] = $response; + } + } + return $responseMessages; + } /** * Shuts down the server and its capabilities. diff --git a/src/Transport/AbstractTransport.php b/src/Transport/AbstractTransport.php index d565cc0..7d13067 100644 --- a/src/Transport/AbstractTransport.php +++ b/src/Transport/AbstractTransport.php @@ -37,9 +37,134 @@ protected function encodeMessage(JsonRpcMessage $message): string return $json; } + /** + * Parses a raw string which may contain one or more JSON-RPC messages. + * + * This method attempts to decode the incoming raw data string. It expects + * the raw data to be either a single JSON object (a single JSON-RPC request) + * or a JSON array of JSON objects (a batch of JSON-RPC requests). + * + * - If $rawData represents a single JSON-RPC message, it's wrapped in an array. + * - If $rawData represents a batch of JSON-RPC messages, each is processed. + * - If $rawData is empty after trim, returns `null`. + * - If $rawData has malformed JSON, throws `TransportException` with code `JsonRpcMessage::PARSE_ERROR`. + * - If $rawData is valid JSON but not a valid JSON-RPC structure (e.g. not an object/array, + * or messages within a batch are invalid), throws `TransportException` with code `JsonRpcMessage::INVALID_REQUEST`. + * + * @param string $rawData The raw string data received from the transport. + * @return JsonRpcMessage[]|null An array of JsonRpcMessage objects if parsing is successful and input is not empty, + * or null if $rawData is empty after trimming. + * @throws TransportException If parsing fails due to malformed JSON, invalid JSON-RPC structure, or excessive size. + */ + protected function parseMessages(string $rawData): ?array + { + $trimmedData = trim($rawData); + if (empty($trimmedData)) { + // As per requirement: "If $rawData is empty after trim, return null." + return null; + } + + if (strlen($trimmedData) > static::MAX_MESSAGE_SIZE) { + throw new TransportException( + "Raw data exceeds size limit of " . static::MAX_MESSAGE_SIZE . " bytes.", + JsonRpcMessage::INVALID_REQUEST + ); + } + + $decodedData = json_decode($trimmedData, true); + $jsonErrorCode = json_last_error(); + + if ($jsonErrorCode !== JSON_ERROR_NONE) { + throw new TransportException( + "Failed to decode JSON: " . json_last_error_msg() . ". Data: " . substr($trimmedData, 0, 200) . "...", + JsonRpcMessage::PARSE_ERROR + ); + } + + if (!is_array($decodedData)) { + throw new TransportException( + "Decoded JSON is not an array or object. Data: " . substr($trimmedData, 0, 200) . "...", + JsonRpcMessage::INVALID_REQUEST + ); + } + + $messages = []; + + if (empty($decodedData)) { // Handles `[]` case, which is a valid empty batch. + return []; + } + + // Determine if it's a batch (indexed array) or single message (associative array) + $isBatch = array_keys($decodedData) === range(0, count($decodedData) - 1); + + if ($isBatch) { + // It's an array of messages (batch) + if (empty($decodedData)) { // Already handled above, but as a safeguard for logic. + return []; + } + foreach ($decodedData as $index => $messageData) { + if (!is_array($messageData)) { + throw new TransportException( + "Invalid item in batch at index {$index}: not an object/array. Item: " . substr((string)json_encode($messageData), 0, 100), + JsonRpcMessage::INVALID_REQUEST + ); + } + try { + $messageJson = json_encode($messageData); + if ($messageJson === false) { + // This case should be rare if $messageData is a valid array from json_decode + throw new TransportException( + "Failed to re-encode message in batch at index {$index}. Data: " . substr((string)json_encode($messageData), 0, 100) . "...", + JsonRpcMessage::INVALID_REQUEST // Or PARSE_ERROR, though it's an encoding issue here + ); + } + $messages[] = JsonRpcMessage::fromJson($messageJson); + } catch (\RuntimeException $e) { + // Assuming JsonRpcMessage::fromJson throws RuntimeException with appropriate codes + throw new TransportException( + "Error parsing message in batch at index {$index}: " . $e->getMessage() . ". Data: " . substr((string)json_encode($messageData), 0, 100) . "...", + $e->getCode() !== 0 ? $e->getCode() : JsonRpcMessage::INVALID_REQUEST, // Use exception code if available + $e + ); + } catch (\Exception $e) { // Catch any other unexpected errors during message construction + throw new TransportException( + "Unexpected error parsing message in batch at index {$index}: " . $e->getMessage() . ". Data: " . substr((string)json_encode($messageData), 0, 100) . "...", + JsonRpcMessage::INTERNAL_ERROR, + $e + ); + } + } + } else { + // It's a single message (associative array) + // Use $trimmedData directly as it's the original JSON string for the single message. + try { + $messages[] = JsonRpcMessage::fromJson($trimmedData); + } catch (\RuntimeException $e) { + // Assuming JsonRpcMessage::fromJson throws RuntimeException with appropriate codes + throw new TransportException( + "Error parsing single message: " . $e->getMessage() . ". Data: " . substr($trimmedData, 0, 200) . "...", + $e->getCode() !== 0 ? $e->getCode() : JsonRpcMessage::INVALID_REQUEST, // Use exception code if available + $e + ); + } catch (\Exception $e) { // Catch any other unexpected errors + throw new TransportException( + "Unexpected error parsing single message: " . $e->getMessage() . ". Data: " . substr($trimmedData, 0, 200) . "...", + JsonRpcMessage::INTERNAL_ERROR, + $e + ); + } + } + + return $messages; + } + /** * Decodes and validates a received JSON string into a JsonRpcMessage. * + * @deprecated This method is deprecated in favor of `parseMessages()`, which can handle + * single messages as well as batches and provides more robust error handling + * for structured data. Consider using `parseMessages()` and then extracting + * the first message if only one is expected. * @param string $data The raw JSON data received from the transport. * @return JsonRpcMessage|null The decoded message, or null if decoding fails (e.g., invalid JSON). * @throws TransportException If the received data string exceeds MAX_MESSAGE_SIZE before decoding. diff --git a/src/Transport/HttpTransport.php b/src/Transport/HttpTransport.php index 98c1945..01e4b2c 100644 --- a/src/Transport/HttpTransport.php +++ b/src/Transport/HttpTransport.php @@ -31,6 +31,8 @@ class HttpTransport extends AbstractTransport private ?ResponseInterface $response = null; /** @var bool Flag to track if the send() method has been called and the response is ready. */ private bool $responsePrepared = false; // To track if send() has been called + /** @var bool Flag to indicate if the last received request body appeared to be a batch. */ + private bool $lastRequestAppearedBatch = false; /** @var ServerRequestInterface The current PSR-7 server request. */ protected ServerRequestInterface $request; @@ -66,164 +68,137 @@ public function __construct( * Receives and decodes the JSON-RPC request payload from the HTTP request. * * Expects a POST request with 'application/json' Content-Type. - * Validates the JSON structure (must be a JSON object or an array of JSON objects). - * This method does not parse into JsonRpcMessage objects itself but returns the - * raw associative array(s) decoded from the JSON payload. + * Validates the HTTP request and uses `parseMessages` to process the body. * - * @return array Returns an associative array for a single JSON-RPC request, - * or a list of associative arrays for a batch request. - * @throws TransportException If the request method is not POST, Content-Type is not JSON, - * the body is empty, JSON is malformed, or the JSON structure - * is not a valid JSON-RPC request object or batch. + * @return JsonRpcMessage[]|null|false An array of `JsonRpcMessage` objects if successful, + * `null` if the request body is effectively empty (after trim), + * or `false` if critical HTTP validation (method, Content-Type) fails. + * @throws TransportException If `parseMessages()` encounters an error (e.g., malformed JSON, + * invalid JSON-RPC structure, message too large), this exception will propagate. */ - public function receive(): array + public function receive(): array|null|false { if ($this->request->getMethod() !== 'POST') { - throw new TransportException('Only POST requests are supported.', JsonRpcMessage::INVALID_REQUEST); + $this->log('HttpTransport::receive: Invalid request method: ' . $this->request->getMethod()); + // Optionally set a specific error response here if desired, + // but returning false signals transport-level unsuitability for this request. + // The Server should ideally handle this by not attempting to process further with this transport. + // If a response is desired, it should be a 405 Method Not Allowed. + // For now, stick to `false` as per subtask interpretation. + return false; } $contentType = $this->request->getHeaderLine('Content-Type'); - if (stripos($contentType, 'application/json') === false) { - throw new TransportException('Content-Type must be application/json.', JsonRpcMessage::INVALID_REQUEST); + if (stripos($contentType, 'application/json') === false && stripos($contentType, 'application/json-rpc') === false) { + $this->log('HttpTransport::receive: Invalid Content-Type: ' . $contentType); + // Similar to above, returning false. Server might produce a 415 Unsupported Media Type. + return false; } $body = (string) $this->request->getBody(); - if (empty($body)) { - throw new TransportException('Request body cannot be empty for JSON-RPC POST.', JsonRpcMessage::INVALID_REQUEST); + // $this->request->getBody()->rewind(); // Ensure stream can be re-read if necessary + + // Determine if the raw body string looks like a batch before parsing + $trimmedBody = trim($body); + if (str_starts_with($trimmedBody, '[') && str_ends_with($trimmedBody, ']')) { + $this->lastRequestAppearedBatch = true; + } else { + $this->lastRequestAppearedBatch = false; } + // `parseMessages` handles empty body (after trim) by returning null. + // It also handles JSON parsing errors and structure validation by throwing TransportException. try { - $parsedBody = json_decode($body, true, 512, JSON_THROW_ON_ERROR); - - // Basic validation for JSON-RPC structure (object or array of objects). - // json_decode($body, true) turns JSON objects into associative arrays. - $isAssocArray = function (array $arr): bool { - // An empty array is not an associative array in JSON-RPC object context. - // An associative array has string keys or non-sequential integer keys. - // array_is_list() checks for sequential, 0-indexed integer keys. - // So, if it's not a list, it's considered associative for this check. - return !empty($arr) && !array_is_list($arr); - }; - - $isValidStructure = false; - if (is_array($parsedBody)) { - if (array_is_list($parsedBody)) { // Potential Batch - if (!empty($parsedBody)) { // Batch array cannot be empty itself - $allBatchItemsValid = true; - foreach ($parsedBody as $item) { - if (!is_array($item) || !$isAssocArray($item)) { // Each item must be a non-empty associative array - $allBatchItemsValid = false; - break; - } - } - if ($allBatchItemsValid) { - $isValidStructure = true; - } - } - } elseif ($isAssocArray($parsedBody)) { // Single request (must be a non-empty associative array) - $isValidStructure = true; - } - } - - if (!$isValidStructure) { - throw new TransportException('Invalid JSON-RPC: Request must be a JSON object or an array of JSON objects.', JsonRpcMessage::INVALID_REQUEST); - } - - // $this->clientRequestWasAckOnly logic removed. - // The JsonRpcMessage parsing (fromJson, fromJsonArray) is removed from transport. - // Transport returns the raw associative array(s). - return $parsedBody; - } catch (\JsonException $e) { - throw new TransportException('JSON Parse Error: ' . $e->getMessage(), JsonRpcMessage::PARSE_ERROR, $e); + return $this->parseMessages($body); // $body is untrimmed, parseMessages will trim + } catch (TransportException $e) { + // Log the exception and let it propagate. + $this->log("TransportException in HttpTransport::receive while calling parseMessages: " . $e->getMessage() . " (Code: " . $e->getCode() . ")"); + throw $e; } - // Removed: catch (\Exception $e) for JsonRpcMessage parsing, as it's no longer done here. + } + + /** + * Checks if the last request received by this transport appeared to be a batch request + * based on its raw structure (e.g., starting with '[' and ending with ']'). + * + * Note: This is a heuristic based on the raw body. `parseMessages` might still return + * a single message if the batch array was empty or contained only one valid message after parsing. + * + * @return bool True if the last request body looked like a batch, false otherwise. + */ + public function lastRequestAppearedAsBatch(): bool + { + return $this->lastRequestAppearedBatch; } /** * Prepares the PSR-7 response with the JSON-RPC payload. * - * This method takes a single JsonRpcMessage, an array of JsonRpcMessages (for batch), - * or a pre-formatted payload array, encodes it to a JSON string, and sets it - * as the body of the PSR-7 response object. The response is typically a 200 OK, - * or a 500 Internal Server Error if encoding fails. + * This method takes a single JsonRpcMessage or an array of JsonRpcMessages (for batch), + * encodes it to a JSON string, and sets it as the body of the PSR-7 response object. + * The response is typically a 200 OK, or a 500 Internal Server Error if encoding/serialization fails. * - * @param JsonRpcMessage|array|null $messageOrPayload The message(s) or pre-formatted payload to send. - * If JsonRpcMessage, it will be serialized. If array, it's assumed to be a batch - * of JsonRpcMessage objects or a ready-to-encode payload. Null means no specific - * payload (though usually an error or empty response would be structured). + * @param JsonRpcMessage|JsonRpcMessage[] $messageOrPayload The message or array of messages to send. + * If JsonRpcMessage, it will be serialized using its `toJson()` method. + * If array (of JsonRpcMessage), it is serialized using `JsonRpcMessage::toJsonArray()`. */ - public function send(JsonRpcMessage|array|null $messageOrPayload): void + public function send(JsonRpcMessage|array $messageOrPayload): void { - // SSE, GET/DELETE, Origin validation, and complex ack-only (202) logic is removed. - // This method now only prepares a standard JSON-RPC response for POST requests. + // Adhering to TransportInterface: send(JsonRpcMessage|array $message) + // The prompt suggested send(array $messages) but this would break the interface. + // We will internally assume if $messageOrPayload is an array, it's JsonRpcMessage[]. - $payloadToEncode = null; + $jsonPayloadString = ''; - if ($messageOrPayload instanceof JsonRpcMessage) { - // If JsonRpcMessage implements JsonSerializable, json_encode will call jsonSerialize() - $payloadToEncode = $messageOrPayload; - } elseif (is_array($messageOrPayload)) { - // Handles an array of JsonRpcMessage objects or a pre-formatted payload array. - // If all elements are JsonRpcMessage, they will be serialized by json_encode. - // If it's a plain array (e.g. batch response), it's used as is. - $payloadToEncode = $messageOrPayload; - } elseif ($messageOrPayload === null) { - // If payload is null, an empty JSON response might be intended (e.g. "[]" for empty batch, or "" which becomes "null"). - // For JSON-RPC, a response to a request usually has 'result' or 'error'. - // A notification expects no response. If this is for a request expecting a response, - // it might result in a parsing error on the client if it's not valid JSON-RPC (e.g. just "null"). - // For now, allow encoding null, which results in JSON "null". - $payloadToEncode = null; - } else { - // Should not be reached due to type hints. Internal error if it does. - // Prepare a JSON-RPC error response. - $errorPayload = [ - 'jsonrpc' => '2.0', - 'error' => [ - 'code' => JsonRpcMessage::INTERNAL_ERROR, - 'message' => 'Server error: Invalid message type for sending.' - ], - 'id' => null - ]; - $encodedErrorString = json_encode($errorPayload); + try { + if ($messageOrPayload instanceof JsonRpcMessage) { + $jsonPayloadString = $messageOrPayload->toJson(); + } elseif (is_array($messageOrPayload)) { + // Assumed to be JsonRpcMessage[] as per intent. + // JsonRpcMessage::toJsonArray will validate this. + // If $messageOrPayload is an empty array, toJsonArray([]) returns "[]". + $jsonPayloadString = JsonRpcMessage::toJsonArray($messageOrPayload); + } else { + // This case should not be reached if called correctly according to type hint. + // However, if it was, create a JSON-RPC error. + $errorMsg = JsonRpcMessage::error( + JsonRpcMessage::INTERNAL_ERROR, + 'Server error: Invalid message type for sending.', + null // No ID available + ); + $jsonPayloadString = $errorMsg->toJson(); + $this->response = $this->responseFactory->createResponse(500) + ->withHeader('Content-Type', 'application/json') + ->withBody($this->streamFactory->createStream($jsonPayloadString)); + $this->responsePrepared = true; + $this->log('HttpTransport::send: Invalid message type provided.'); + return; + } + } catch (\Exception $e) { + // Catch exceptions during toJson() or toJsonArray() (e.g., InvalidArgumentException from toJsonArray) + $this->log('HttpTransport::send: Error during message serialization: ' . $e->getMessage()); + $errorMsg = JsonRpcMessage::error( + JsonRpcMessage::INTERNAL_ERROR, + 'Server error: Failed to serialize JSON response.', + null // ID is difficult to determine reliably here. + ); + $jsonPayloadString = $errorMsg->toJson(); // This should be safe. $this->response = $this->responseFactory->createResponse(500) ->withHeader('Content-Type', 'application/json') - ->withBody($this->streamFactory->createStream($encodedErrorString !== false ? $encodedErrorString : '{"jsonrpc":"2.0","error":{"code":-32000,"message":"Server JSON encoding error"}}')); + ->withBody($this->streamFactory->createStream($jsonPayloadString)); $this->responsePrepared = true; return; } - $jsonPayloadString = json_encode($payloadToEncode); - - if ($jsonPayloadString === false) { - // JSON encoding failed. Prepare a standard JSON-RPC error. - $errorPayload = [ - 'jsonrpc' => '2.0', - 'error' => [ - 'code' => JsonRpcMessage::INTERNAL_ERROR, - 'message' => 'Server error: Failed to encode JSON response: ' . json_last_error_msg() - ], - 'id' => null // ID is difficult to determine reliably at this stage for a failed batch. - ]; - $encodedErrorString = json_encode($errorPayload); // This should not fail. - $this->response = $this->responseFactory->createResponse(500) - ->withHeader('Content-Type', 'application/json') - ->withBody($this->streamFactory->createStream($encodedErrorString !== false ? $encodedErrorString : '{"jsonrpc":"2.0","error":{"code":-32000,"message":"Server JSON encoding error"}}')); - } else { - // Original behavior: always return 200 OK if JSON encoding is successful - $httpStatus = 200; - // The check for ($payloadToEncode instanceof JsonRpcMessage && $payloadToEncode->error !== null) - // was part of the detailed status code mapping. If we revert to always 200 (or 500 on encode failure), - // this specific check isn't strictly needed here for status determination but can be kept for clarity - // if we want to log errors, etc. For minimal change from "original" simple 200/500, it's not used to change status. + // At this point, $jsonPayloadString is successfully created. + // Default HTTP status 200. Specific error statuses (e.g. 400, 500 for JSON-RPC errors) + // are typically embedded in the JsonRpcMessage error object, not as HTTP status codes, + // unless the error is at the HTTP transport level itself. + $this->response = $this->responseFactory->createResponse(200) + ->withHeader('Content-Type', 'application/json') + ->withBody($this->streamFactory->createStream($jsonPayloadString)); - $this->response = $this->responseFactory->createResponse($httpStatus) - ->withHeader('Content-Type', 'application/json') - ->withBody($this->streamFactory->createStream($jsonPayloadString)); - } $this->responsePrepared = true; - // The response is stored in $this->response and will be retrieved by getResponse(). - // No direct output or header manipulation here. } /** diff --git a/src/Transport/StdioTransport.php b/src/Transport/StdioTransport.php index 3d30a57..6c3444a 100644 --- a/src/Transport/StdioTransport.php +++ b/src/Transport/StdioTransport.php @@ -38,76 +38,35 @@ public function __construct() * * Handles single JSON-RPC requests or batch requests (JSON array of requests). * - * @return JsonRpcMessage[]|null An array of JsonRpcMessage objects if a line is successfully parsed. - * Returns an empty array if STDIN is closed (EOF). - * Returns null if an empty line is read (transport still open). - * @throws \RuntimeException If there is a JSON parsing error or an invalid JSON-RPC message structure. + * Reads a line from STDIN and parses it using `AbstractTransport::parseMessages()`. + * + * @return JsonRpcMessage[]|null|false An array of `JsonRpcMessage` objects if successful, + * `null` if an empty line is read (and transport is open), + * or `false` if STDIN is closed (EOF). + * @throws TransportException If `parseMessages()` encounters an error (e.g., malformed JSON, + * invalid JSON-RPC structure, message too large). */ - public function receive(): ?array + public function receive(): array|null|false { $line = fgets($this->stdin); if ($line === false) { - return []; // Stream closed - } - - $line = trim($line); - if ($line === '') { - return null; // No message received, transport open + // EOF or error on stream + return false; // TransportInterface dictates false for closed transport } + // Note: parseMessages handles trimming and empty string checks. + // If $line is just "\n", trim($line) will be empty, and parseMessages will return null. + // If $line contains actual content, parseMessages will process it. try { - $decodedInput = json_decode($line, true, 512, JSON_THROW_ON_ERROR); - - // Check for batch request: a non-empty numerically indexed array - if ( - is_array($decodedInput) - && (count($decodedInput) === 0 - || array_keys($decodedInput) === range(0, count($decodedInput) - 1)) - ) { - if (empty($decodedInput)) { - // Empty array received, spec implies it's invalid for batch, - // but JsonRpcMessage::fromJsonArray will handle this (likely an error or empty messages). - // For consistency, we let fromJsonArray parse it. - return JsonRpcMessage::fromJsonArray($line); - } - // It's a batch, let fromJsonArray handle full parsing - // and validation from original string - return JsonRpcMessage::fromJsonArray($line); - } elseif ( - is_object($decodedInput) // Decoded as object if not assoc=true - || (is_array($decodedInput) && !empty($decodedInput)) // Decoded as non-empty assoc array - ) { - // It's a single request (object) or potentially an associative - // array representing a single request. - // Let fromJson handle full parsing and validation from original string - $message = JsonRpcMessage::fromJson($line); - return [$message]; - } else { - // Invalid structure that is not explicitly an empty array - // or a valid single/batch candidate - throw new \RuntimeException( - 'Invalid JSON-RPC message structure. Expected object or array of objects.', - JsonRpcMessage::PARSE_ERROR - ); - } - } catch (\JsonException $e) { - throw new \RuntimeException( - 'JSON Parse Error: ' . $e->getMessage(), - JsonRpcMessage::PARSE_ERROR, - $e - ); - } catch (\Exception $e) { - // Catch other exceptions from JsonRpcMessage::fromJson or fromJsonArray - $this->log("Error processing received message: " . $e->getMessage()); - // Depending on desired behavior, re-throw or return error specific message - // For now, let's re-throw as a runtime exception consistent with parse errors - throw new \RuntimeException( - 'Error parsing JSON-RPC message: ' . $e->getMessage(), - JsonRpcMessage::INVALID_REQUEST, // Or PARSE_ERROR if more appropriate - $e - ); + return $this->parseMessages($line); + } catch (TransportException $e) { + // Log the exception message from parseMessages and re-throw. + $this->log("TransportException in StdioTransport::receive: " . $e->getMessage() . " (Code: " . $e->getCode() . ")"); + throw $e; } + // No need to catch other generic \Exception here, as parseMessages is expected + // to throw TransportException for known parsing issues. } /** diff --git a/src/Transport/TransportInterface.php b/src/Transport/TransportInterface.php index 6cd95fc..b6ca54a 100644 --- a/src/Transport/TransportInterface.php +++ b/src/Transport/TransportInterface.php @@ -26,19 +26,19 @@ interface TransportInterface * - If no message is currently available but the transport is still open * (e.g., non-blocking read with no data), it should return `null`. * - If the transport is definitively closed (e.g., EOF on STDIN, HTTP connection ended - * before full message), it should return an empty array `[]`. + * before full message), it should return `false`. * * Implementations are responsible for handling framing, decoding, and basic structural * validation of incoming messages. * - * @return JsonRpcMessage[]|null An array of JsonRpcMessage objects, - * null if no message is currently available, - * or an empty array if the transport is closed. + * @return JsonRpcMessage[]|null|false An array of JsonRpcMessage objects, + * null if no message is currently available, + * or false if the transport is closed. * @throws \MCP\Server\Exception\TransportException For critical transport errors * (e.g., connection lost mid-message, unrecoverable framing issues). * @throws \RuntimeException For errors like JSON parsing failures if handled within receive. */ - public function receive(): ?array; + public function receive(): array|null|false; /** * Sends a single JSON-RPC message or an array of JSON-RPC messages through the transport. diff --git a/tests/ServerTest.php b/tests/ServerTest.php index a1fb472..5ebbae3 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -536,32 +536,27 @@ public function testRunHttpRequestCycle(): void // Scenario 5: TransportException set explicitly to be thrown by receive() $transportExceptionId = 'tex_http_' . uniqid(); - $someValidPayload = ['jsonrpc' => '2.0', 'method' => 'initialize', 'params' => ['protocolVersion' => '2025-03-26'], 'id' => $transportExceptionId]; - $mockRequestForTransportException = $this->createMockRequest($someValidPayload); - $httpTransport->setMockRequest($mockRequestForTransportException); - $customTransportException = new TransportException("Custom transport error", 12345); - $httpTransport->setExceptionToThrowOnReceive($customTransportException); + // Scenario 5: TransportException due to malformed JSON + $malformedJsonRequestId = 'malformed_json_http_' . uniqid(); // ID won't actually be parsed + $malformedJsonPayload = '{"jsonrpc": "2.0", "method": "test", "id": "' . $malformedJsonRequestId . '"'; // Intentionally malformed - $server->run(); - $capturedCustomErrorResponse = $httpTransport->getCapturedResponse(); - $this->assertNotNull($capturedCustomErrorResponse); - // The status code will depend on how HttpTransport maps generic TransportException codes. - // If the code is non-standard JSON-RPC, it might default to 500 or use the code if it's a valid HTTP code. - // Server::runHttpRequestCycle catches TransportException and uses its code for JsonRpcMessage::error - // HttpTransport then maps this JsonRpcMessage error code to an HTTP status. - // If TransportException code 12345 is used in JsonRpcMessage, and it's not a standard one, - // JsonRpcMessage::error would make it the error code. - // HttpTransport now sends JSON-RPC errors with HTTP 200 - $this->assertEquals(200, $capturedCustomErrorResponse->getStatusCode()); - $customErrorBody = json_decode((string) $capturedCustomErrorResponse->getBody(), true); - self::assertIsArray($customErrorBody); // Ensure $customErrorBody is an array - $this->assertNull($customErrorBody['id']); // ID might be lost if error happens before parsing ID. - // Server.php L203 tries to get ID from raw payload for general errors. - // TransportException happens before this, so ID is null. - $this->assertArrayHasKey('error', $customErrorBody); - self::assertIsArray($customErrorBody['error']); - $this->assertEquals(12345, $customErrorBody['error']['code']); - $this->assertEquals("Custom transport error", $customErrorBody['error']['message']); + // Use createMockRequest to set up the request with malformed JSON body + $mockRequestWithMalformedJson = $this->createMockRequest($malformedJsonPayload); + $httpTransport->setMockRequest($mockRequestWithMalformedJson); + + $server->run(); // Server should catch TransportException and create a JSON-RPC error response + + $capturedErrorResponse = $httpTransport->getCapturedResponse(); + $this->assertNotNull($capturedErrorResponse); + $this->assertEquals(200, $capturedErrorResponse->getStatusCode(), "HTTP status for parse error should be 200."); + + $errorBody = json_decode((string) $capturedErrorResponse->getBody(), true); + $this->assertIsArray($errorBody, "Error response body should be a JSON array."); + $this->assertNull($errorBody['id'], "ID should be null for a parse error before ID extraction."); + $this->assertArrayHasKey('error', $errorBody, "Error response should contain an 'error' object."); + $this->assertIsArray($errorBody['error'], "The 'error' field should be an array."); + $this->assertEquals(JsonRpcMessage::PARSE_ERROR, $errorBody['error']['code'], "JSON-RPC error code should be PARSE_ERROR."); + $this->assertStringContainsStringIgnoringCase("Failed to decode JSON", $errorBody['error']['message'], "Error message should indicate JSON decoding failure."); } public function testInitializeAndToolCallInSameStream(): void diff --git a/tests/Transport/AbstractTransportTest.php b/tests/Transport/AbstractTransportTest.php new file mode 100644 index 0000000..210d8ea --- /dev/null +++ b/tests/Transport/AbstractTransportTest.php @@ -0,0 +1,161 @@ +transport = new TestableAbstractTransport(); + } + + public function testParseMessagesReturnsNullForEmptyRawData(): void + { + $this->assertNull($this->transport->callParseMessages("")); + $this->assertNull($this->transport->callParseMessages(" ")); + $this->assertNull($this->transport->callParseMessages("\t\n ")); + } + + public function testParseMessagesThrowsTransportExceptionForMalformedJson(): void + { + $this->expectException(TransportException::class); + $this->expectExceptionCode(JsonRpcMessage::PARSE_ERROR); + $this->expectExceptionMessageMatches('/Failed to decode JSON/'); + $this->transport->callParseMessages('{"jsonrpc": "2.0", "method": "foo", "params": ['); + } + + public function testParseMessagesThrowsTransportExceptionForJsonNumber(): void + { + $this->expectException(TransportException::class); + $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); + $this->expectExceptionMessageMatches('/Decoded JSON is not an array or object/'); + $this->transport->callParseMessages('123'); + } + + public function testParseMessagesThrowsTransportExceptionForJsonString(): void + { + $this->expectException(TransportException::class); + $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); + $this->expectExceptionMessageMatches('/Decoded JSON is not an array or object/'); + $this->transport->callParseMessages('"this is a string"'); + } + + public function testParseMessagesThrowsTransportExceptionForJsonNull(): void + { + $this->expectException(TransportException::class); + $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); + $this->expectExceptionMessageMatches('/Decoded JSON is not an array or object/'); + $this->transport->callParseMessages('null'); + } + + public function testParseMessagesThrowsTransportExceptionForOversizedMessage(): void + { + $maxSize = TestableAbstractTransport::getExposedMaxMessageSize(); + // Create a string that is slightly larger than the max size + // A simple request: {"jsonrpc":"2.0","method":"m","id":1} (36 chars) + // We need to pad it. + $baseRequest = '{"jsonrpc":"2.0","method":"m","id":1'; // Remove closing } + $paddingSize = $maxSize - strlen($baseRequest) + 1; + $paddedRequest = $baseRequest . ',"padding":"' . str_repeat('a', $paddingSize) . '"}'; + + $this->expectException(TransportException::class); + $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); + $this->expectExceptionMessageMatches('/Raw data exceeds size limit/'); + $this->transport->callParseMessages($paddedRequest); + } + + public function testParseMessagesParsesValidSingleRequest(): void + { + $json = '{"jsonrpc": "2.0", "method": "subtract", "params": [42, 23], "id": 1}'; + $messages = $this->transport->callParseMessages($json); + $this->assertIsArray($messages); + $this->assertCount(1, $messages); + $this->assertInstanceOf(JsonRpcMessage::class, $messages[0]); + $this->assertEquals('2.0', $messages[0]->jsonrpc); + $this->assertEquals('subtract', $messages[0]->method); + $this->assertEquals([42, 23], $messages[0]->params); + $this->assertEquals('1', $messages[0]->id); + } + + public function testParseMessagesParsesValidBatchRequest(): void + { + $json = '[' . + '{"jsonrpc": "2.0", "method": "sum", "params": [1,2,4], "id": "1"},' . + '{"jsonrpc": "2.0", "method": "notify_hello", "params": [7]},' . + '{"jsonrpc": "2.0", "method": "subtract", "params": [42,23], "id": "2"}' . + ']'; + $messages = $this->transport->callParseMessages($json); + $this->assertIsArray($messages); + $this->assertCount(3, $messages); + + $this->assertInstanceOf(JsonRpcMessage::class, $messages[0]); + $this->assertEquals('sum', $messages[0]->method); + $this->assertEquals('1', $messages[0]->id); + + $this->assertInstanceOf(JsonRpcMessage::class, $messages[1]); + $this->assertEquals('notify_hello', $messages[1]->method); + $this->assertNull($messages[1]->id); + + $this->assertInstanceOf(JsonRpcMessage::class, $messages[2]); + $this->assertEquals('subtract', $messages[2]->method); + $this->assertEquals('2', $messages[2]->id); + } + + public function testParseMessagesParsesEmptyBatchRequest(): void + { + $json = '[]'; + $messages = $this->transport->callParseMessages($json); + $this->assertIsArray($messages); + $this->assertCount(0, $messages); + } + + public function testParseMessagesThrowsForInvalidItemInBatchNotAnObject(): void + { + $json = '[{"jsonrpc": "2.0", "method": "foo", "id": "1"}, 123]'; // 123 is not an object + $this->expectException(TransportException::class); + $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); + $this->expectExceptionMessageMatches('/Invalid item in batch at index 1: not an object\/array/'); + $this->transport->callParseMessages($json); + } + + public function testParseMessagesThrowsForInvalidItemInBatchMalformedRpc(): void + { + // Item is an object, but not a valid RPC message (missing method) + $json = '[{"jsonrpc": "2.0", "id": "1"}]'; + $this->expectException(TransportException::class); + // This code comes from JsonRpcMessage::fromJson via the re-thrown TransportException + $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); + $this->expectExceptionMessageMatches('/Error parsing message in batch at index 0: Missing or invalid method name/'); + $this->transport->callParseMessages($json); + } + + public function testParseMessagesThrowsForFailedReEncodingInBatch(): void + { + // This test is tricky because json_encode failing for an array element + // from json_decode($rawData, true) is very unlikely unless there are + // specific unserializable objects or recursion, which json_decode($rawData, true) shouldn't produce. + // We can simulate by trying to create an invalid UTF-8 sequence if the system is sensitive. + // For now, this is a placeholder, as it's hard to reliably trigger json_encode failure + // from a structure that json_decode produced. + // A more direct way would be to mock json_encode if the class was designed for it. + // Let's assume valid re-encoding for now and rely on other tests for coverage. + $this->markTestSkipped('Difficult to reliably trigger json_encode failure for an array item from json_decode in this context.'); + } + + public function testParseMessagesThrowsForSingleInvalidRpcStructure(): void + { + // Item is an object, but not a valid RPC message (missing method) + $json = '{"jsonrpc": "2.0", "id": "1"}'; + $this->expectException(TransportException::class); + // This code comes from JsonRpcMessage::fromJson via the re-thrown TransportException + $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); + $this->expectExceptionMessageMatches('/Error parsing single message: Missing or invalid method name/'); + $this->transport->callParseMessages($json); + } +} diff --git a/tests/Transport/HttpTransportTest.php b/tests/Transport/HttpTransportTest.php index ecd96b8..1984629 100644 --- a/tests/Transport/HttpTransportTest.php +++ b/tests/Transport/HttpTransportTest.php @@ -16,80 +16,101 @@ class HttpTransportTest extends TestCase { private Psr17Factory $psr17Factory; + // Mock ServerRequestInterface and its StreamInterface body private ServerRequestInterface $mockRequest; private StreamInterface $mockStream; - // private ResponseInterface $mockResponse; // Removed as it's unused - protected function setUp(): void { $this->psr17Factory = new Psr17Factory(); $this->mockRequest = $this->createMock(ServerRequestInterface::class); $this->mockStream = $this->createMock(StreamInterface::class); - // $this->mockResponse = $this->createMock(ResponseInterface::class); // Removed - // Default behavior for getBody + // Default behavior for getBody() on the mock request $this->mockRequest->method('getBody')->willReturn($this->mockStream); } - private function createTransport(): HttpTransport + /** + * Helper to create HttpTransport with mocked dependencies. + * This now uses TestableHttpTransport to allow setting the mock request. + */ + private function createTransport(?ServerRequestInterface $request = null): TestableHttpTransport { - return new HttpTransport( - $this->psr17Factory, // Corrected: ResponseFactoryInterface - $this->psr17Factory, // Corrected: StreamFactoryInterface - $this->mockRequest // Corrected: ServerRequestInterface + // If no request is provided, use the class property $this->mockRequest + $testableTransport = new TestableHttpTransport( + $this->psr17Factory, + $this->psr17Factory, + $request ?? $this->mockRequest // Pass the mock request to TestableHttpTransport constructor ); + // Ensure the mock request is properly set within TestableHttpTransport + // if it wasn't passed via constructor or if we need to re-set/confirm it. + $testableTransport->setMockRequest($request ?? $this->mockRequest); + return $testableTransport; } + public function testConstructor(): void { + // Test with the default mockRequest from setUp $transport = $this->createTransport(); $this->assertInstanceOf(HttpTransport::class, $transport); - // Test that getResponse() returns the initially created (empty) response $response = $transport->getResponse(); $this->assertInstanceOf(ResponseInterface::class, $response); + // Test constructor with explicit null request (HttpTransport should create one from globals) + // For unit testing, we avoid fromGlobals. This instance is not used further. + $transportFromGlobals = new HttpTransport($this->psr17Factory, $this->psr17Factory, null); + $this->assertInstanceOf(HttpTransport::class, $transportFromGlobals); } // --- Tests for receive() --- - public function testReceiveValidSinglePostRequestSimplified(): void + public function testReceiveValidSingleRequest(): void { - $jsonRpcPayload = ['jsonrpc' => '2.0', 'method' => 'test', 'id' => 1]; - $jsonRpcString = json_encode($jsonRpcPayload); + $jsonRpcPayloadArray = ['jsonrpc' => '2.0', 'method' => 'test', 'id' => 1]; + $jsonRpcString = json_encode($jsonRpcPayloadArray); $this->mockRequest->method('getMethod')->willReturn('POST'); $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn('application/json'); $this->mockStream->method('__toString')->willReturn($jsonRpcString); $transport = $this->createTransport(); - $receivedData = $transport->receive(); + $receivedMessages = $transport->receive(); - $this->assertEquals($jsonRpcPayload, $receivedData); + $this->assertIsArray($receivedMessages); + $this->assertCount(1, $receivedMessages); + $this->assertInstanceOf(JsonRpcMessage::class, $receivedMessages[0]); + $this->assertEquals('test', $receivedMessages[0]->method); + $this->assertEquals(1, $receivedMessages[0]->id); } - public function testReceiveValidBatchPostRequestSimplified(): void + public function testReceiveValidBatchRequest(): void { - $jsonRpcPayload = [ + $jsonRpcPayloadArray = [ ['jsonrpc' => '2.0', 'method' => 'test1', 'id' => 1], ['jsonrpc' => '2.0', 'method' => 'test2', 'id' => 2] ]; - $jsonRpcString = json_encode($jsonRpcPayload); + $jsonRpcString = json_encode($jsonRpcPayloadArray); $this->mockRequest->method('getMethod')->willReturn('POST'); $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn('application/json'); $this->mockStream->method('__toString')->willReturn($jsonRpcString); $transport = $this->createTransport(); - $receivedData = $transport->receive(); - - $this->assertEquals($jsonRpcPayload, $receivedData); + $receivedMessages = $transport->receive(); + + $this->assertIsArray($receivedMessages); + $this->assertCount(2, $receivedMessages); + $this->assertInstanceOf(JsonRpcMessage::class, $receivedMessages[0]); + $this->assertEquals('test1', $receivedMessages[0]->method); + $this->assertInstanceOf(JsonRpcMessage::class, $receivedMessages[1]); + $this->assertEquals('test2', $receivedMessages[1]->method); } - public function testReceivePostRequestInvalidJsonSyntaxSimplified(): void + public function testReceivePostRequestInvalidJsonSyntax(): void { $this->expectException(TransportException::class); $this->expectExceptionCode(JsonRpcMessage::PARSE_ERROR); - $this->expectExceptionMessageMatches('/JSON Parse Error/'); + $this->expectExceptionMessageMatches('/Failed to decode JSON/'); // From parseMessages $this->mockRequest->method('getMethod')->willReturn('POST'); $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn('application/json'); @@ -99,11 +120,12 @@ public function testReceivePostRequestInvalidJsonSyntaxSimplified(): void $transport->receive(); } - public function testReceivePostRequestInvalidJsonRpcStructureSimplified(): void + public function testReceivePostRequestInvalidJsonRpcStructureNotObjectOrArray(): void { $this->expectException(TransportException::class); $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); - $this->expectExceptionMessage('Invalid JSON-RPC: Request must be a JSON object or an array of JSON objects.'); + // This message comes from parseMessages + $this->expectExceptionMessageMatches('/Decoded JSON is not an array or object/'); $this->mockRequest->method('getMethod')->willReturn('POST'); $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn('application/json'); @@ -113,66 +135,120 @@ public function testReceivePostRequestInvalidJsonRpcStructureSimplified(): void $transport->receive(); } - public function testReceivePostRequestEmptyBodySimplified(): void + public function testReceivePostRequestEmptyBodyReturnsNull(): void { - $this->expectException(TransportException::class); - $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); - $this->expectExceptionMessage('Request body cannot be empty for JSON-RPC POST.'); - $this->mockRequest->method('getMethod')->willReturn('POST'); $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn('application/json'); - $this->mockStream->method('__toString')->willReturn(''); + $this->mockStream->method('__toString')->willReturn(''); // Empty body $transport = $this->createTransport(); - $transport->receive(); + $this->assertNull($transport->receive()); } - public function testReceivePostRequestIncorrectContentTypeSimplified(): void + public function testReceivePostRequestWhitespaceBodyReturnsNull(): void { - $this->expectException(TransportException::class); - $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); - $this->expectExceptionMessage('Content-Type must be application/json.'); + $this->mockRequest->method('getMethod')->willReturn('POST'); + $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn('application/json'); + $this->mockStream->method('__toString')->willReturn(' '); // Whitespace body + + $transport = $this->createTransport(); + $this->assertNull($transport->receive()); + } + public function testReceivePostRequestIncorrectContentTypeReturnsFalse(): void + { $this->mockRequest->method('getMethod')->willReturn('POST'); $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn('text/plain'); + // Body content doesn't matter here as the Content-Type check should fail first $this->mockStream->method('__toString')->willReturn('{"jsonrpc":"2.0","method":"test"}'); $transport = $this->createTransport(); - $transport->receive(); + $this->assertFalse($transport->receive()); } - public function testReceivePostRequestMissingContentTypeSimplified(): void + public function testReceivePostRequestApplicationJsonRpcContentTypeIsValid(): void { - $this->expectException(TransportException::class); - $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); - $this->expectExceptionMessage('Content-Type must be application/json.'); + $jsonRpcPayloadArray = ['jsonrpc' => '2.0', 'method' => 'test', 'id' => 1]; + $jsonRpcString = json_encode($jsonRpcPayloadArray); $this->mockRequest->method('getMethod')->willReturn('POST'); - $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn(''); - $this->mockStream->method('__toString')->willReturn('{"jsonrpc":"2.0","method":"test"}'); + $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn('application/json-rpc'); + $this->mockStream->method('__toString')->willReturn($jsonRpcString); $transport = $this->createTransport(); - $transport->receive(); + $receivedMessages = $transport->receive(); + $this->assertIsArray($receivedMessages); + $this->assertCount(1, $receivedMessages); + $this->assertInstanceOf(JsonRpcMessage::class, $receivedMessages[0]); } - public function testReceiveNonPostRequestSimplified(): void + public function testReceivePostRequestMissingContentTypeReturnsFalse(): void { - $this->expectException(TransportException::class); - $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); - $this->expectExceptionMessage('Only POST requests are supported.'); + $this->mockRequest->method('getMethod')->willReturn('POST'); + $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn(''); // Missing + $this->mockStream->method('__toString')->willReturn('{"jsonrpc":"2.0","method":"test"}'); + + $transport = $this->createTransport(); + $this->assertFalse($transport->receive()); + } + public function testReceiveNonPostRequestReturnsFalse(): void + { $this->mockRequest->method('getMethod')->willReturn('GET'); + // Other mocks don't matter as method check is first $transport = $this->createTransport(); - $transport->receive(); + $this->assertFalse($transport->receive()); + } + + // --- Tests for receive() an lastRequestAppearedAsBatch() --- + + /** + * @dataProvider batchDetectionDataProvider + */ + public function testLastRequestAppearedAsBatchDetection(string $requestBody, bool $expectedIsBatch): void + { + $this->mockRequest->method('getMethod')->willReturn('POST'); + $this->mockRequest->method('getHeaderLine')->with('Content-Type')->willReturn('application/json'); + $this->mockStream->method('__toString')->willReturn($requestBody); + + $transport = $this->createTransport(); + + // Call receive to trigger parsing and setting of the flag + try { + $transport->receive(); + } catch (TransportException $e) { + // Expected for malformed JSON, but flag should still be set based on initial structure + } + + $this->assertSame($expectedIsBatch, $transport->lastRequestAppearedAsBatch()); + } + + /** + * @return array + */ + public static function batchDetectionDataProvider(): array + { + return [ + 'single object' => ['{"id":1,"method":"test"}', false], + 'single object with whitespace' => [' {"id":1,"method":"test"} ', false], + 'batch array' => ['[{"id":1,"method":"test"}]', true], + 'batch array with whitespace' => [' [{"id":1,"method":"test"}] ', true], + 'empty batch array' => ['[]', true], + 'empty batch array with whitespace' => [' [] ', true], + 'malformed batch-like (missing closing bracket)' => ['[{"id":1}', false], // Does not end with ']' + 'malformed single-like (missing closing brace)' => ['{"id":1', false], // Does not end with '}' + 'simple string (not batch)' => ['"hello"', false], + 'empty string (not batch)' => ['', false], + 'whitespace string (not batch)' => [' ', false], + ]; } // --- Tests for send() --- - public function testSendSingleMessageSimplified(): void + public function testSendSingleMessage(): void { $transport = $this->createTransport(); - // Create a real JsonRpcMessage, assuming JsonRpcMessage class is available and works $rpcResponse = JsonRpcMessage::result(['foo' => 'bar'], '1'); $transport->send($rpcResponse); @@ -180,14 +256,11 @@ public function testSendSingleMessageSimplified(): void $response = $transport->getResponse(); $this->assertEquals(200, $response->getStatusCode()); $this->assertStringContainsString('application/json', $response->getHeaderLine('Content-Type')); - $expectedJson = '{"jsonrpc":"2.0","id":"1","result":{"foo":"bar"}}'; - // Note: The order of keys in actual JSON might vary. assertJsonStringEqualsJsonString handles this. - // Also, JsonRpcMessage::jsonSerialize() implementation detail matters here. $this->assertJsonStringEqualsJsonString($expectedJson, (string) $response->getBody()); } - public function testSendBatchMessageSimplified(): void + public function testSendBatchMessage(): void { $transport = $this->createTransport(); $rpcResponses = [ @@ -199,23 +272,13 @@ public function testSendBatchMessageSimplified(): void $response = $transport->getResponse(); $this->assertEquals(200, $response->getStatusCode()); $this->assertStringContainsString('application/json', $response->getHeaderLine('Content-Type')); - $expectedJson = '[{"jsonrpc":"2.0","id":"1","result":{"foo":"bar"}},{"jsonrpc":"2.0","id":"2","error":{"code":-32600,"message":"Invalid Request"}}]'; $this->assertJsonStringEqualsJsonString($expectedJson, (string) $response->getBody()); } - public function testSendNullMessageSimplified(): void - { - $transport = $this->createTransport(); - $transport->send(null); - - $response = $transport->getResponse(); - $this->assertEquals(200, $response->getStatusCode()); - $this->assertStringContainsString('application/json', $response->getHeaderLine('Content-Type')); - $this->assertEquals('null', (string) $response->getBody()); - } + // testSendNullMessageSimplified() is removed as send(null) is no longer a valid direct call based on refactored logic. - public function testSendEmptyBatchMessageSimplified(): void + public function testSendEmptyArrayMessage(): void { $transport = $this->createTransport(); $transport->send([]); // Empty batch @@ -248,8 +311,9 @@ public function testSendHandlesJsonEncodeFailureSimplified(): void $this->assertJson($body); $decodedBody = json_decode($body, true); $this->assertEquals('2.0', $decodedBody['jsonrpc']); - $this->assertNull($decodedBody['id']); + $this->assertNull($decodedBody['id']); // ID is null because it couldn't be determined from the failed message $this->assertEquals(JsonRpcMessage::INTERNAL_ERROR, $decodedBody['error']['code']); - $this->assertStringContainsString('Failed to encode JSON response', $decodedBody['error']['message']); + // This is the message set by HttpTransport::send when JsonRpcMessage::toJson/toJsonArray fails + $this->assertEquals('Server error: Failed to serialize JSON response.', $decodedBody['error']['message']); } } diff --git a/tests/Transport/StdioTransportTest.php b/tests/Transport/StdioTransportTest.php index 65f2cad..e4f8de8 100644 --- a/tests/Transport/StdioTransportTest.php +++ b/tests/Transport/StdioTransportTest.php @@ -61,59 +61,64 @@ public function testReceiveEmptyJsonArray(): void $this->assertEmpty($messages); } - public function testReceiveInvalidJsonThrowsException(): void // Renamed and Updated + public function testReceiveInvalidJsonThrowsTransportException(): void { $this->transport->writeToInput('{"invalidjson'); - $this->expectException(\RuntimeException::class); - // StdioTransport::receive catches JsonException and re-throws RuntimeException with code PARSE_ERROR + $this->expectException(\MCP\Server\Exception\TransportException::class); $this->expectExceptionCode(JsonRpcMessage::PARSE_ERROR); + $this->expectExceptionMessageMatches('/Failed to decode JSON/'); $this->transport->receive(); } - public function testReceiveEmptyLineReturnsNull(): void // Renamed and Updated + public function testReceiveEmptyLineReturnsNull(): void { - $this->transport->writeToInput(''); // Empty line, but still a line + // TestableStdioTransport->writeToInput adds a newline. + // fgets will read this newline. parseMessages will trim it. + // trim("\n") is empty, so parseMessages returns null. + $this->transport->writeToInput(''); $receivedMessages = $this->transport->receive(); - $this->assertNull($receivedMessages); // As per StdioTransport logic for empty line + $this->assertNull($receivedMessages); } - public function testReceiveStreamClosedReturnsEmptyArray(): void // Revised logic for EOF + public function testReceiveStreamClosedReturnsFalse(): void { // Write one line $this->transport->writeToInput('{"jsonrpc":"2.0","method":"ping","id":1}'); // Consume that one line $firstResult = $this->transport->receive(); - $this->assertNotNull($firstResult); - if ($firstResult !== null) { // Check to satisfy static analyzer + $this->assertNotNull($firstResult, "First receive() call should return a message or null, not false EOF yet."); + if (is_array($firstResult)) { // Ensure it's an array before counting $this->assertCount(1, $firstResult); + } else { + // If it's not an array here, it might be null (empty line), which is unexpected for this specific test input. + // Or it could be false if the stream was already at EOF, also unexpected here. + // Fail if it's not an array, as this test expects a valid message first. + $this->fail("Expected an array of messages from the first receive() call, got " . gettype($firstResult)); } // Now try to receive again, fgets should return false as no more data $messages = $this->transport->receive(); - $this->assertIsArray($messages); - $this->assertEmpty($messages); + $this->assertFalse($messages); } - public function testReceiveInvalidJsonStructureThrowsException(): void + public function testReceiveInvalidJsonStructureThrowsTransportException(): void { $this->transport->writeToInput('"just a string"'); // A JSON scalar - $this->expectException(\RuntimeException::class); - // This specific message comes from the generic catch (\Exception $e) block - // that re-throws the initially thrown "Invalid JSON-RPC message structure." exception. - $this->expectExceptionMessage('Error parsing JSON-RPC message: Invalid JSON-RPC message structure.'); - $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); // Code from the generic catch block + $this->expectException(\MCP\Server\Exception\TransportException::class); + $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); + $this->expectExceptionMessageMatches('/Decoded JSON is not an array or object/'); $this->transport->receive(); } - public function testReceiveCatchesExceptionFromJsonRpcMessage(): void + public function testReceiveInvalidRpcStructureThrowsTransportException(): void { // This JSON is an object but is missing the 'method' field, - // which JsonRpcMessage::fromJson() will throw an error for. + // which JsonRpcMessage::fromJson() will throw an error for (via parseMessages). $this->transport->writeToInput('{"jsonrpc": "2.0", "id": 1}'); - $this->expectException(\RuntimeException::class); - // This message comes from the generic catch (\Exception $e) block - // re-throwing the exception from JsonRpcMessage::fromJson(). - $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); // Code from the generic catch block + $this->expectException(\MCP\Server\Exception\TransportException::class); + $this->expectExceptionCode(JsonRpcMessage::INVALID_REQUEST); + // Message will be something like "Error parsing single message: Missing or invalid method name..." + $this->expectExceptionMessageMatches('/Error parsing single message: Missing or invalid method name in JSON-RPC request/'); $this->transport->receive(); } diff --git a/tests/Transport/TestableAbstractTransport.php b/tests/Transport/TestableAbstractTransport.php new file mode 100644 index 0000000..a2b606c --- /dev/null +++ b/tests/Transport/TestableAbstractTransport.php @@ -0,0 +1,72 @@ +loggedMessages[] = $message; + // parent::log($message); // Optionally call parent to also use error_log + } + + /** @return string[] */ + public function getLoggedMessages(): array + { + return $this->loggedMessages; + } + + public function isClosed(): bool + { + return false; // Not relevant for parseMessages tests + } + + public function isStreamOpen(): bool + { + return false; // Not relevant for parseMessages tests + } + + /** + * Exposes the protected parseMessages method for testing. + * + * @param string $rawData The raw data to parse. + * @return JsonRpcMessage[]|null + * @throws \MCP\Server\Exception\TransportException + */ + public function callParseMessages(string $rawData): ?array + { + return $this->parseMessages($rawData); + } + + /** + * Exposes the protected MAX_MESSAGE_SIZE constant for testing. + * @return int + */ + public static function getExposedMaxMessageSize(): int + { + return static::MAX_MESSAGE_SIZE; // Or self::MAX_MESSAGE_SIZE + } +} diff --git a/tests/Transport/TestableHttpTransport.php b/tests/Transport/TestableHttpTransport.php index 8f70c46..41476b5 100644 --- a/tests/Transport/TestableHttpTransport.php +++ b/tests/Transport/TestableHttpTransport.php @@ -16,109 +16,54 @@ class TestableHttpTransport extends HttpTransport { private ?ServerRequestInterface $mockRequest = null; - private ?\Throwable $exceptionToThrowOnReceive = null; + // Removed: private ?\Throwable $exceptionToThrowOnReceive = null; public function __construct( ?ResponseFactoryInterface $responseFactory = null, - ?StreamFactoryInterface $streamFactory = null + ?StreamFactoryInterface $streamFactory = null, + // Allow injecting the initial mock request directly via constructor for convenience + ?ServerRequestInterface $initialMockRequest = null ) { - // Pass null for the request. The parent HttpTransport will default to fromGlobals(), - // but TestableHttpTransport::getRequest() overrides to use the mock request. - // Factories are passed through; if null, parent HttpTransport creates defaults. - parent::__construct($responseFactory, $streamFactory, null); + // HttpTransport's constructor expects a ServerRequestInterface or null. + // We pass the $initialMockRequest here. If it's null, HttpTransport creates one from globals. + // Our getRequest() override will ensure this mock is used if set later via setMockRequest. + parent::__construct($responseFactory, $streamFactory, $initialMockRequest); + if ($initialMockRequest !== null) { + $this->mockRequest = $initialMockRequest; + // Ensure the protected $this->request in HttpTransport is also set. + $this->request = $initialMockRequest; + } } - // This is the primary way to inject the request for the test. + // This is the primary way to inject or update the request for the test. public function setMockRequest(ServerRequestInterface $request): void { $this->mockRequest = $request; - // Also update the internal $this->request property of AbstractTransport - // so that if any parent method relies on $this->request directly, it's set. - // And so that our getRequest() override below can also set it if needed. + // Update the protected $this->request in HttpTransport. $this->request = $this->mockRequest; } - public function setExceptionToThrowOnReceive(\Throwable $e): void - { - $this->exceptionToThrowOnReceive = $e; - } + // Removed: public function setExceptionToThrowOnReceive(\Throwable $e) /** * Override getRequest to ensure our mock request is used by receive(). - * The receive() method (whether parent's or overridden) will call getRequest(). - */ - public function getRequest(): ServerRequestInterface - { - if ($this->mockRequest !== null) { - // Ensure the parent's $this->request is also updated if it hasn't been. - if ($this->request !== $this->mockRequest) { - $this->request = $this->mockRequest; - } - return $this->mockRequest; - } - // If no mock request is set, and we are in a test environment, - // calling parent::getRequest() could lead to errors if it tries to access SAPI globals. - throw new \LogicException('Mock request not set in TestableHttpTransport. Call setMockRequest() before receive() is triggered.'); - } - - /** - * Override receive to use the mock request's body or throw a predefined exception. - * This method is called by Server::runHttpRequestCycle(). - * @return array + * HttpTransport::receive() calls $this->request. This method ensures $this->request is our mock. + * Note: HttpTransport::receive() itself uses $this->request directly, not $this->getRequest(). + * The constructor and setMockRequest now directly set $this->request (protected in parent). + * This method is more of a conceptual override if there were other places getRequest() was used. + * For direct testing of HttpTransport::receive(), setting $this->request via setMockRequest is key. */ - public function receive(): array - { - if ($this->exceptionToThrowOnReceive !== null) { - $e = $this->exceptionToThrowOnReceive; - $this->exceptionToThrowOnReceive = null; // Clear after use to prevent re-throwing - throw $e; - } - - // Our overridden getRequest() will be called here. - $currentRequest = $this->getRequest(); - $body = $currentRequest->getBody(); - $body->rewind(); // Ensure reading from the start of the stream - $contents = $body->getContents(); - - if (empty($contents)) { - throw new TransportException('Request body is empty.', \MCP\Server\Message\JsonRpcMessage::INVALID_REQUEST); - } - - $decoded = json_decode($contents, true); - - // Check for JSON decoding errors - if (json_last_error() !== JSON_ERROR_NONE) { - throw new TransportException('JSON decode error: ' . json_last_error_msg(), JsonRpcMessage::PARSE_ERROR); - } - - // Validate the basic structure of the decoded JSON for JSON-RPC - if (!is_array($decoded)) { - // Must be a JSON object (associative array) or array of objects (numerically indexed array for batch) - throw new TransportException('Invalid JSON payload: expected JSON object or array of objects.', JsonRpcMessage::INVALID_REQUEST); - } - - $isBatch = array_is_list($decoded); - if ($isBatch) { // Batch request - if (empty($decoded)) { // Batch array cannot be empty - throw new TransportException('Invalid JSON-RPC batch: Batch array cannot be empty.', JsonRpcMessage::INVALID_REQUEST); - } - foreach ($decoded as $item) { - // Each item in a batch must be a JSON object (associative array) - if (!is_array($item) || array_is_list($item)) { - throw new TransportException('Invalid JSON-RPC batch: All items in batch must be JSON objects.', JsonRpcMessage::INVALID_REQUEST); - } - } - } else { // Single request (must be an associative array/JSON object) - // If $decoded is an empty array [], it means the JSON was "{}" - // This is an empty object, which is not a valid JSON-RPC request. - if (empty($decoded)) { - throw new TransportException('Invalid JSON-RPC request: Request object cannot be empty.', JsonRpcMessage::INVALID_REQUEST); - } - } - - // The Server will further validate 'jsonrpc', 'method', 'params', 'id' fields. - return $decoded; - } + // public function getRequest(): ServerRequestInterface // This override might not be strictly necessary + // { + // if ($this->mockRequest !== null) { + // return $this->mockRequest; + // } + // // Fallback to parent or error if not set, though HttpTransport sets $this->request in constructor. + // return parent::getRequest(); // Or throw if always expect mock to be set. + // } + + // Removed: public function receive(): array + // We want to test HttpTransport::receive(), not override it here. /** * After the Server calls send() (which is the parent HttpTransport::send()),