From 43e7e55ce3617977f2a05ed78cd727ac3f91fee6 Mon Sep 17 00:00:00 2001 From: acoulton Date: Tue, 17 Sep 2024 08:12:10 +0100 Subject: [PATCH 1/3] Add `Response::setJSON` and `Controller::respondJSON` Simple helpers for sending JSON responses with standard content-type header and an HTTP status (since this is commonly variable for JSON APIs) as a single operation. Added as a controller shorthand method for maximum sugar. --- CHANGELOG.md | 2 ++ classes/Kohana/Controller.php | 10 ++++++ classes/Kohana/Response.php | 17 ++++++++++ tests/kohana/ResponseTest.php | 58 +++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1df105c1..f5190bda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ You're really going to want to read this. ## Unreleased +* Add `Response::setJSON` and `Controller::respondJSON` as helpers for sending JSON responses. + ## 4.10.0 (2022-11-09) * [BEHAVIOUR CHANGE] Kohana no longer overrides the PHP default session_cache_limiter option diff --git a/classes/Kohana/Controller.php b/classes/Kohana/Controller.php index 14531dfe..ba6224d3 100644 --- a/classes/Kohana/Controller.php +++ b/classes/Kohana/Controller.php @@ -150,4 +150,14 @@ protected function check_cache($etag = NULL) return HTTP::check_cache($this->request, $this->response, $etag); } + /** + * Sugar method to reduce verbosity of sending a JSON response + * + * @see Response::setJSON() + */ + protected function respondJSON(mixed $body, int $status = 200): void + { + $this->response->setJSON($body, $status); + } + } diff --git a/classes/Kohana/Response.php b/classes/Kohana/Response.php index e34f5351..9e111b03 100644 --- a/classes/Kohana/Response.php +++ b/classes/Kohana/Response.php @@ -193,6 +193,23 @@ public function body($content = NULL) return $this; } + /** + * Set a JSON response with body, content-type header, and HTTP status + * + * @param mixed $body Anything that can be json-serialized + * @param int $status + * + * @return void + * @throws JsonException + * @throws Kohana_Exception if status code is unknown + */ + public function setJSON(mixed $body, int $status = 200): void + { + $this->body(json_encode($body, JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES)); + $this->status($status); + $this->headers('Content-Type', 'application/json'); + } + /** * Gets or sets the HTTP protocol. The standard protocol to use * is `HTTP/1.1`. diff --git a/tests/kohana/ResponseTest.php b/tests/kohana/ResponseTest.php index 16fbfe99..783bbc22 100644 --- a/tests/kohana/ResponseTest.php +++ b/tests/kohana/ResponseTest.php @@ -84,6 +84,64 @@ public function test_body_string_zero($string, $expected) $this->assertSame($expected, $response->body()); } + public static function provider_set_json(): array + { + return [ + 'simple body with defaults' => [ + ['body' => ['some', 'thing']], + [ + 'body' => '["some","thing"]', + 'status' => 200, + 'headers' => ['content-type' => 'application/json'], + ], + ], + 'custom status code' => [ + ['body' => ['some', 'thing'], 'status' => 409], + [ + 'body' => '["some","thing"]', + 'status' => 409, + 'headers' => ['content-type' => 'application/json'], + ], + ], + 'json serializable' => [ + [ + 'body' => new class implements JsonSerializable { + public function jsonSerialize(): array + { + return ['a' => 'value']; + } + }, + ], + ['body' => '{"a":"value"}', 'status' => 200, 'headers' => ['content-type' => 'application/json']], + ], + 'unescaped slashes' => [ + ['body' => 'some/thing'], + ['body' => '"some/thing"', 'status' => 200, 'headers' => ['content-type' => 'application/json']], + ], + ]; + } + + /** + * @dataProvider provider_set_json + */ + public function test_set_json(array $args, array $expected) + { + $response = new Response(); + $response->setJSON(...$args); + $this->assertSame( + $expected, + ['body' => $response->body(), 'status' => $response->status(), 'headers' => $response->headers()->getArrayCopy()], + ); + } + + public function test_set_json_throws_on_json_error() + { + $response = new Response(); + $this->expectException(JsonException::class); + $this->expectExceptionMessage('UTF-8'); + $response->setJSON(['bad' => "utf8 \xD0"]); + } + /** * provider for test_cookie_set() * From 52121cacb007ad66743e6a51bea60b4f6c3ebea5 Mon Sep 17 00:00:00 2001 From: acoulton Date: Tue, 17 Sep 2024 11:04:08 +0100 Subject: [PATCH 2/3] Add Request::jsonBody and Request::jsonBodyArray for JSON requests Provides helper methods to parse an incoming request as JSON, with basic sanity / security checks for the content type, maximum payload size, and maximum nesting level of JSON objects and arrays. --- CHANGELOG.md | 1 + classes/Kohana/Request.php | 97 ++++++++ .../Request/InvalidJSONRequestException.php | 6 + tests/kohana/RequestTest.php | 214 ++++++++++++++++++ 4 files changed, 318 insertions(+) create mode 100644 classes/Request/InvalidJSONRequestException.php diff --git a/CHANGELOG.md b/CHANGELOG.md index f5190bda..df8c6828 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ You're really going to want to read this. ## Unreleased +* Add `Request::jsonBody` and `Request::jsonBodyArray` for easy access to JSON request bodies. * Add `Response::setJSON` and `Controller::respondJSON` as helpers for sending JSON responses. ## 4.10.0 (2022-11-09) diff --git a/classes/Kohana/Request.php b/classes/Kohana/Request.php index 7dd0fc96..9283ee65 100644 --- a/classes/Kohana/Request.php +++ b/classes/Kohana/Request.php @@ -812,6 +812,103 @@ public function body() return $this->_body; } + /** + * Parse a JSON body (of any type, including single scalar values) from the incoming request + * + * Enforces basic security of the maximum body length & depth, to guard against the easiest + * kinds of JSON attacks. These should be set as low as your app can handle for the specific + * request(s) it is parsing. Note that there are other mechanisms for JSON DOS attacks which + * are harder to guard against in userland - you should ideally only use this method for + * requests from authenticated and trusted users to provide additional protection. + * + * Note also that the maximum JSON length is enforced separately from `post_max_size` in your + * PHP config. This is because you will commonly need to accept longer POST bodies for things + * like file uploads and multipart-encoded form submisssions. + */ + public function jsonBody( + int $max_json_bytes = 1000, + int $max_json_depth = 20, + ): mixed + { + // First check the content-type - if they've randomly sent the wrong type of content that + // could plausibly also be too big. So we want the error message to be clear this is a + // content type problem. + $content_type = $this->headers('content-type'); + if (explode(';', $content_type ?? '', 2)[0] !== 'application/json') { + throw new Request_InvalidJSONRequestException( + $content_type + ? "Unexpected content type (got $content_type)" + : 'No content-type specified' + ); + } + + // Then verify the user-specified content-length is within the allowed max size + // This might be bigger than the actual received payload if e.g. it has been stripped + // due to the post_max_size config. + $content_length = (int) $this->headers('content-length'); + if ($content_length > $max_json_bytes) { + throw new Request_InvalidJSONRequestException( + sprintf( + 'Content larger than max_json_bytes (got %s)', + Text::bytes($content_length, format: '%01.1f%s') + ) + ); + } + + // Then verify the user-specified content-length matches the actual content we've received + $actual_content_length = strlen($this->_body); + if ($content_length !== $actual_content_length) { + throw new Request_InvalidJSONRequestException( + sprintf( + 'Incorrect content-length header: stated %s, got %s', + Text::bytes($content_length, format: '%01.1f%s'), + Text::bytes($actual_content_length, format: '%01.1f%s'), + ) + ); + } + + // OK, it's probably as safe as it can be (the native JSON parser will enforce the max depth) + try { + return json_decode( + $this->_body, + associative: TRUE, + depth: $max_json_depth, + flags: JSON_THROW_ON_ERROR, + ); + } catch (JSONException $e) { + throw new Request_InvalidJSONRequestException( + 'Invalid JSON: '.$e->getMessage(), + previous: $e + ); + } + } + + /** + * Parse JSON body from the incoming request where an object / array (`{}`, `[]`) is expected + * + * Adds an additional sanity check over ->jsonBody() to enforce that the incoming request can + * be safely treated as an array e.g. to access keys from it. + * + * @see Request::jsonBody() for more details on use and security implications of this method. + */ + public function jsonBodyArray( + int $max_json_bytes = 1000, + int $max_json_depth = 20, + ): array + { + // Just pass all the function args to the underlying jsonBody method for initial parse and validation + $decoded = $this->jsonBody(...get_defined_vars()); + + // Then enforce that the body was actually an object / array + if (is_array($decoded)) { + return $decoded; + } + + throw new Request_InvalidJSONRequestException( + 'JSON body not an array or object (got '.get_debug_type($decoded).')' + ); + } + /** * Returns the length of the body for use with * content header diff --git a/classes/Request/InvalidJSONRequestException.php b/classes/Request/InvalidJSONRequestException.php new file mode 100644 index 00000000..369e0c1b --- /dev/null +++ b/classes/Request/InvalidJSONRequestException.php @@ -0,0 +1,6 @@ + '"I am JSON"', + 'header' => new Http_Header([ + 'content-type' => 'application/json', + 'content-length' => '11', + ]), + ]); + + $this->assertSame('I am JSON', $request->jsonBody()); + } + + public static function provider_json_body_array(): array + { + return [ + 'JSON object' => [ + '{"foo":"bar"}', + ['foo' => 'bar'], + ], + 'JSON array' => [ + '["foo","bar"]', + ['foo', 'bar'], + ], + ]; + } + + /** + * @dataProvider provider_json_body_array + */ + public function test_can_return_decoded_json_body_array_or_object(string $body, array $expect) + { + $request = Request::with([ + 'body' => $body, + 'header' => new Http_Header([ + 'content-type' => 'application/json', + 'content-length' => strlen($body), + ]) + ]); + + $this->assertSame($expect, $request->jsonBodyArray()); + } + + public function test_accepts_json_with_charset_in_content_type() + { + // strictly speaking, charset should not be included in the header but occasionally client + // libraries include it. + $request = Request::with([ + 'body' => '"foo"', + 'header' => new Http_Header([ + 'content-type' => 'application/json; charset=utf8', + 'content-length' => '5', + ]), + ]); + $this->assertSame('foo', $request->jsonBody()); + } + + public static function provider_invalid_json_request(): array + { + return [ + 'missing content-type' => [ + [ + 'body' => '{"foo": "bar"}', + 'header' => [ + 'content-length' => '14', + ], + ], + 'No content-type specified' + ], + 'unexpected content-type' => [ + [ + 'body' => '{"foo":"bar"}', + 'header' => [ + 'Content-Type' => 'application/x-www-formdata', + 'Content-Length' => '14', + ], + ], + 'Unexpected content type (got application/x-www-formdata)', + ], + 'invalid JSON' => [ + [ + 'body' => '{unquoted: "bar"}', + 'header' => [ + 'Content-Type' => 'application/json', + 'Content-Length' => '17', + ], + ], + 'Invalid JSON: Syntax error' + ], + 'not an array (for jsonBodyArray)' => [ + [ + 'body' => '"I am a valid JSON string"', + 'header' => [ + 'Content-Type' => 'application/json', + 'Content-Length' => '26', + ], + ], + 'JSON body not an array or object (got string)', + ], + ]; + } + + + /** + * @dataProvider provider_invalid_json_request + */ + public function test_throws_on_invalid_or_unexpected_json_request(array $request, string $expect_msg) + { + $request['header'] = new HTTP_Header($request['header'] ?? []); + $request = Request::with($request); + $this->expectException(Request_InvalidJSONRequestException::class); + $this->expectExceptionMessage($expect_msg); + $request->jsonBodyArray(); + } + + public static function provider_invalid_json_too_large() + { + return [ + 'within expected size' => [ + '{"foo": "bar"}', + ['max_json_bytes' => 14], + ['result' => ['foo' => 'bar']], + ], + 'bigger than expected size' => [ + '{"foo": "bar"}', + ['max_json_bytes' => 13], + ['exception' => 'Content larger than max_json_bytes (got 14.0B)'], + ], + 'up to 1kB default is OK' => [ + '{"fo": "'.str_repeat('a', 990).'"}', + [], + ['result' => ['fo' => str_repeat('a', 990)]], + ], + 'bigger than 1kB default fails' => [ + '{"fo": "'.str_repeat('a', 991).'"}', + [], + ['exception' => 'Content larger than max_json_bytes (got 1.0kB)'], + ], + 'within default max depth' => [ + '{"1":{"2":{"3":{"4":{"5":{"6":{"7":{"8":{"9":{"10":{"11":{"12":{"13":{"14":{"15":{"16":{"17":{"18":{"19":"ok"}}}}}}}}}}}}}}}}}}}', + [], + ['result' => [1=>[2=>[3=>[4=>[5=>[6=>[7=>[8=>[9=>[10=>[11=>[12=>[13=>[14=>[15=>[16=>[17=>[18=>[19=>'ok']]]]]]]]]]]]]]]]]]]], + ], + 'beyond default max depth' => [ + '{"1":{"2":{"3":{"4":{"5":{"6":{"7":{"8":{"9":{"10":{"11":{"12":{"13":{"14":{"15":{"16":{"17":{"18":{"19":{"20":"bad"}}}}}}}}}}}}}}}}}}}}', + [], + ['exception' => 'Invalid JSON: Maximum stack depth exceeded'], + ], + 'custom max depth' => [ + '{"1":{"2":{"3":{"4":{"5":{"6":{"7":{"8":{"9":{"10":{"11":{"12":{"13":{"14":{"15":{"16":{"17":{"18":{"19":{"20":"bad"}}}}}}}}}}}}}}}}}}}}', + ['max_json_depth' => 21], + ['result' => [1=>[2=>[3=>[4=>[5=>[6=>[7=>[8=>[9=>[10=>[11=>[12=>[13=>[14=>[15=>[16=>[17=>[18=>[19=>[20=>'bad']]]]]]]]]]]]]]]]]]]]], + ] + ]; + } + + /** + * @dataProvider provider_invalid_json_too_large + */ + public function test_json_body_methods_protect_against_large_or_abusive_payloads(string $body, array $args, array $expect_behaviour) + { + $request = Request::with([ + 'body' => $body, + 'header' => new HTTP_Header([ + 'content-type' => 'application/json', + 'content-length' => strlen($body), + ]) + ]); + + $actual_behaviour = []; + + try { + $actual_behaviour['result'] = $request->jsonBodyArray(...$args); + } catch (Request_InvalidJSONRequestException $e) { + $actual_behaviour['exception'] = $e->getMessage(); + } + + $this->assertSame($expect_behaviour, $actual_behaviour); + } + + public function test_json_body_methods_protect_against_content_length_tampering() + { + $request = Request::with([ + 'body' => '{"very_long": "'.str_repeat('a', 1_500_000).'"}', + 'header' => new HTTP_Header([ + 'content-type' => 'application/json', + 'content-length' => 152, + ]) + ]); + + $this->expectException(Request_InvalidJSONRequestException::class); + $this->expectExceptionMessage('Incorrect content-length header: stated 152.0B, got 1.5MB'); + $request->jsonBody(); + } + + public function test_json_body_methods_cope_with_post_max_size_truncation() + { + // If the incoming body is greater than post_max_size, the body may have been cleared by the server + // in which case we want to go off the content-length header regardless that this is a mismatch + // to the body content. + $request = Request::with([ + 'body' => '', + 'header' => new HTTP_Header([ + 'content-type' => 'application/json', + 'content-length' => '20500000', + ]) + ]); + + $this->expectException(Request_InvalidJSONRequestException::class); + $this->expectExceptionMessage('Content larger than max_json_bytes (got 20.5MB)'); + $request->jsonBody(); + } + + } // End Kohana_RequestTest From 18faf29e22fd560bd13e2f956a831acd005ea1e1 Mon Sep 17 00:00:00 2001 From: Andrew Coulton Date: Mon, 23 Sep 2024 11:54:54 +0100 Subject: [PATCH 3/3] Bump changelog for 4.11.0 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index df8c6828..1554d0a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ You're really going to want to read this. ## Unreleased +## 4.11.0 (2024-09-23) + * Add `Request::jsonBody` and `Request::jsonBodyArray` for easy access to JSON request bodies. * Add `Response::setJSON` and `Controller::respondJSON` as helpers for sending JSON responses.