malhuda/CodeIgniter4
1
0
Fork 0
mirror of https://github.com/codeigniter4/CodeIgniter4.git synced 2025-02-20 11:44:28 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #8490 from kenjis/fix-ResponseTrait-format

Browse Source 
fix: API\ResponseTrait can't return string as JSON
This commit is contained in:
kenjis 2024-02-05 08:57:04 +09:00 committed by GitHub
commit 61269504a8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 220 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
system/API/ResponseTrait.php
View File

@ -22,6 +22,9 @@ use Config\Services;
* Provides common, more readable, methods to provide
* consistent HTTP responses under a variety of common
* situations when working as an API.
*
* @property bool $stringAsHtml Whether to treat string data as HTML in JSON response.
* Setting `true` is only for backward compatibility.
*/
trait ResponseTrait
{
@ -64,10 +67,11 @@ trait ResponseTrait
/**
* How to format the response data.
* Either 'json' or 'xml'. If blank will be
* determined through content negotiation.
* Either 'json' or 'xml'. If null is set, it will be determined through
* content negotiation.
*
* @var string
* @var string|null
* @phpstan-var 'html'|'json'|'xml'|null
*/
protected $format = 'json';
@ -294,7 +298,7 @@ trait ResponseTrait
// --------------------------------------------------------------------
/**
* Handles formatting a response. Currently makes some heavy assumptions
* Handles formatting a response. Currently, makes some heavy assumptions
* and needs updating! :)
*
* @param array|string|null $data
@ -303,20 +307,10 @@ trait ResponseTrait
*/
protected function format($data = null)
{
// If the data is a string, there's not much we can do to it...
if (is_string($data)) {
// The content type should be text/... and not application/...
$contentType = $this->response->getHeaderLine('Content-Type');
$contentType = str_replace('application/json', 'text/html', $contentType);
$contentType = str_replace('application/', 'text/', $contentType);
$this->response->setContentType($contentType);
$this->format = 'html';
return $data;
}
$format = Services::format();
$mime = "application/{$this->format}";
$mime = ($this->format === null) ? $format->getConfig()->supportedResponseFormats[0]
: "application/{$this->format}";
// Determine correct response type through content negotiation if not explicitly declared
if (
@ -338,6 +332,23 @@ trait ResponseTrait
$this->formatter = $format->getFormatter($mime);
}
$asHtml = $this->stringAsHtml ?? false;
// Returns as HTML.
if (
($mime === 'application/json' && $asHtml && is_string($data))
|| ($mime !== 'application/json' && is_string($data))
) {
// The content type should be text/... and not application/...
$contentType = $this->response->getHeaderLine('Content-Type');
$contentType = str_replace('application/json', 'text/html', $contentType);
$contentType = str_replace('application/', 'text/', $contentType);
$this->response->setContentType($contentType);
$this->format = 'html';
return $data;
}
if ($mime !== 'application/json') {
// Recursively convert objects into associative arrays
// Conversion not required for JSONFormatter
@ -350,11 +361,14 @@ trait ResponseTrait
/**
* Sets the format the response should be in.
*
* @param string|null $format Response format
* @phpstan-param 'json'|'xml' $format
*
* @return $this
*/
protected function setResponseFormat(?string $format = null)
{
$this->format = strtolower($format);
$this->format = ($format === null) ? null : strtolower($format);
return $this;
}

2
system/RESTful/ResourceController.php
View File

@ -106,7 +106,7 @@ class ResourceController extends BaseResource
/**
* Set/change the expected response representation for returned objects
*
* @param string $format json/xml
* @param string $format Response format
* @phpstan-param 'json'|'xml' $format
*
* @return void

217
tests/system/API/ResponseTraitTest.php
View File

@ -44,7 +44,7 @@ final class ResponseTraitTest extends CIUnitTestCase
$this->formatter = new JSONFormatter();
}
protected function makeController(array $userConfig = [], string $routePath = '', array $userHeaders = [])
private function createAppConfig(): App
{
$config = new App();
@ -60,6 +60,11 @@ final class ResponseTraitTest extends CIUnitTestCase
$config->{$key} = $value;
}
return $config;
}
private function createCookieConfig(): Cookie
{
$cookie = new Cookie();
foreach ([
@ -74,8 +79,21 @@ final class ResponseTraitTest extends CIUnitTestCase
}
Factories::injectMock('config', 'Cookie', $cookie);
return $cookie;
}
private function createRequestAndResponse(string $routePath = '', array $userHeaders = []): void
{
$config = $this->createAppConfig();
$this->createCookieConfig();
if ($this->request === null) {
$this->request = new MockIncomingRequest($config, new SiteURI($config, $routePath), null, new UserAgent());
$this->request = new MockIncomingRequest(
$config,
new SiteURI($config, $routePath),
null,
new UserAgent()
);
$this->response = new MockResponse($config);
}
@ -87,10 +105,12 @@ final class ResponseTraitTest extends CIUnitTestCase
foreach ($headers as $key => $value) {
$this->request->setHeader($key, $value);
if (($key === 'Accept') && ! is_array($value)) {
$this->response->setContentType($value);
}
}
}
protected function makeController(string $routePath = '', array $userHeaders = [])
{
$this->createRequestAndResponse($routePath, $userHeaders);
// Create the controller class finally.
return new class ($this->request, $this->response, $this->formatter) {
@ -100,7 +120,7 @@ final class ResponseTraitTest extends CIUnitTestCase
protected $response;
protected $formatter;
public function __construct(&$request, &$response, &$formatter)
public function __construct($request, $response, $formatter)
{
$this->request = $request;
$this->response = $response;
@ -117,11 +137,14 @@ final class ResponseTraitTest extends CIUnitTestCase
public function testNoFormatterJSON(): void
{
$this->formatter = null;
$controller = $this->makeController([], '', ['Accept' => 'application/json']);
$controller = $this->makeController(
'',
['Accept' => 'application/json']
);
$this->invoke($controller, 'respondCreated', [['id' => 3], 'A Custom Reason']);
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(201, $this->response->getStatusCode());
$expected = <<<'EOH'
@ -135,19 +158,53 @@ final class ResponseTraitTest extends CIUnitTestCase
public function testNoFormatter(): void
{
$this->formatter = null;
$controller = $this->makeController([], '', ['Accept' => 'application/json']);
$controller = $this->makeController(
'',
['Accept' => 'application/json']
);
$this->invoke($controller, 'respondCreated', ['A Custom Reason']);
$this->assertSame('"A Custom Reason"', $this->response->getBody());
}
public function testNoFormatterWithStringAsHtmlTrue(): void
{
$this->formatter = null;
$this->createRequestAndResponse('', ['Accept' => 'application/json']);
$controller = new class ($this->request, $this->response, $this->formatter) {
use ResponseTrait;
protected $request;
protected $response;
protected $formatter;
protected bool $stringAsHtml = true;
public function __construct($request, $response, $formatter)
{
$this->request = $request;
$this->response = $response;
$this->formatter = $formatter;
}
};
$this->invoke($controller, 'respondCreated', ['A Custom Reason']);
$this->assertSame('A Custom Reason', $this->response->getBody());
$this->assertStringStartsWith(
'text/html',
$this->response->getHeaderLine('Content-Type')
);
}
public function testAssociativeArrayPayload(): void
{
$this->formatter = null;
$controller = $this->makeController();
$payload = ['answer' => 42];
$payload = ['answer' => 42];
$this->invoke($controller, 'respond', [$payload]);
$expected = <<<'EOH'
@ -162,12 +219,12 @@ final class ResponseTraitTest extends CIUnitTestCase
{
$this->formatter = null;
$controller = $this->makeController();
$payload = [
$payload = [
1,
2,
3,
];
$this->invoke($controller, 'respond', [$payload]);
$expected = <<<'EOH'
@ -184,9 +241,10 @@ final class ResponseTraitTest extends CIUnitTestCase
{
$this->formatter = null;
$controller = $this->makeController();
$payload = new stdClass();
$payload->name = 'Tom';
$payload->id = 1;
$payload = new stdClass();
$payload->name = 'Tom';
$payload->id = 1;
$this->invoke($controller, 'respond', [(array) $payload]);
@ -225,10 +283,41 @@ final class ResponseTraitTest extends CIUnitTestCase
$this->invoke($controller, 'respond', ['something', 201]);
$this->assertSame(201, $this->response->getStatusCode());
$this->assertSame('"something"', $this->response->getBody());
$this->assertStringStartsWith(
'application/json',
$this->response->getHeaderLine('Content-Type')
);
$this->assertSame('Created', $this->response->getReasonPhrase());
}
public function testRespondSetsCorrectBodyAndStatusWithStringAsHtmlTrue(): void
{
$this->createRequestAndResponse();
$controller = new class ($this->request, $this->response, $this->formatter) {
use ResponseTrait;
protected $request;
protected $response;
protected $formatter;
protected bool $stringAsHtml = true;
public function __construct($request, $response, $formatter)
{
$this->request = $request;
$this->response = $response;
$this->formatter = $formatter;
}
};
$this->invoke($controller, 'respond', ['something', 201]);
$this->assertSame(201, $this->response->getStatusCode());
$this->assertSame('something', $this->response->getBody());
$this->assertStringStartsWith('text/html', $this->response->getHeaderLine('Content-Type'));
$this->assertSame('Created', $this->response->getReason());
$this->assertSame('Created', $this->response->getReasonPhrase());
}
public function testRespondWithCustomReason(): void
@ -238,7 +327,7 @@ final class ResponseTraitTest extends CIUnitTestCase
$this->invoke($controller, 'respond', ['something', 201, 'A Custom Reason']);
$this->assertSame(201, $this->response->getStatusCode());
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
}
public function testFailSingleMessage(): void
@ -257,7 +346,7 @@ final class ResponseTraitTest extends CIUnitTestCase
];
$this->assertSame($this->formatter->format($expected), $this->response->getBody());
$this->assertSame(500, $this->response->getStatusCode());
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
}
public function testCreated(): void
@ -266,7 +355,7 @@ final class ResponseTraitTest extends CIUnitTestCase
$this->invoke($controller, 'respondCreated', [['id' => 3], 'A Custom Reason']);
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(201, $this->response->getStatusCode());
$this->assertSame($this->formatter->format(['id' => 3]), $this->response->getBody());
}
@ -277,7 +366,7 @@ final class ResponseTraitTest extends CIUnitTestCase
$this->invoke($controller, 'respondDeleted', [['id' => 3], 'A Custom Reason']);
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(200, $this->response->getStatusCode());
$this->assertSame($this->formatter->format(['id' => 3]), $this->response->getBody());
}
@ -288,7 +377,7 @@ final class ResponseTraitTest extends CIUnitTestCase
$this->invoke($controller, 'respondUpdated', [['id' => 3], 'A Custom Reason']);
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(200, $this->response->getStatusCode());
$this->assertSame($this->formatter->format(['id' => 3]), $this->response->getBody());
}
@ -306,7 +395,7 @@ final class ResponseTraitTest extends CIUnitTestCase
'error' => 'Nope',
],
];
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(401, $this->response->getStatusCode());
$this->assertSame($this->formatter->format($expected), $this->response->getBody());
}
@ -324,7 +413,7 @@ final class ResponseTraitTest extends CIUnitTestCase
'error' => 'Nope',
],
];
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(403, $this->response->getStatusCode());
$this->assertSame($this->formatter->format($expected), $this->response->getBody());
}
@ -336,7 +425,7 @@ final class ResponseTraitTest extends CIUnitTestCase
$this->invoke($controller, 'respondNoContent', ['']);
$this->assertStringStartsWith('application/json', $this->response->getHeaderLine('Content-Type'));
$this->assertSame('No Content', $this->response->getReason());
$this->assertSame('No Content', $this->response->getReasonPhrase());
$this->assertSame(204, $this->response->getStatusCode());
}
@ -353,7 +442,7 @@ final class ResponseTraitTest extends CIUnitTestCase
'error' => 'Nope',
],
];
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(404, $this->response->getStatusCode());
$this->assertSame($this->formatter->format($expected), $this->response->getBody());
}
@ -371,7 +460,7 @@ final class ResponseTraitTest extends CIUnitTestCase
'error' => 'Nope',
],
];
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(400, $this->response->getStatusCode());
$this->assertSame($this->formatter->format($expected), $this->response->getBody());
}
@ -380,7 +469,11 @@ final class ResponseTraitTest extends CIUnitTestCase
{
$controller = $this->makeController();
$this->invoke($controller, 'failValidationErrors', [['foo' => 'Nope', 'bar' => 'No way'], 'FAT CHANCE', 'A Custom Reason']);
$this->invoke(
$controller,
'failValidationErrors',
[['foo' => 'Nope', 'bar' => 'No way'], 'FAT CHANCE', 'A Custom Reason']
);
$expected = [
'status' => 400,
@ -390,7 +483,7 @@ final class ResponseTraitTest extends CIUnitTestCase
'bar' => 'No way',
],
];
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(400, $this->response->getStatusCode());
$this->assertSame($this->formatter->format($expected), $this->response->getBody());
}
@ -408,7 +501,7 @@ final class ResponseTraitTest extends CIUnitTestCase
'error' => 'Nope',
],
];
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(409, $this->response->getStatusCode());
$this->assertSame($this->formatter->format($expected), $this->response->getBody());
}
@ -426,7 +519,7 @@ final class ResponseTraitTest extends CIUnitTestCase
'error' => 'Nope',
],
];
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(410, $this->response->getStatusCode());
$this->assertSame($this->formatter->format($expected), $this->response->getBody());
}
@ -444,7 +537,7 @@ final class ResponseTraitTest extends CIUnitTestCase
'error' => 'Nope',
],
];
$this->assertSame('A Custom Reason', $this->response->getReason());
$this->assertSame('A Custom Reason', $this->response->getReasonPhrase());
$this->assertSame(429, $this->response->getStatusCode());
$this->assertSame($this->formatter->format($expected), $this->response->getBody());
}
@ -455,7 +548,7 @@ final class ResponseTraitTest extends CIUnitTestCase
$this->invoke($controller, 'failServerError', ['Nope.', 'FAT-CHANCE', 'A custom reason.']);
$this->assertSame('A custom reason.', $this->response->getReason());
$this->assertSame('A custom reason.', $this->response->getReasonPhrase());
$this->assertSame(500, $this->response->getStatusCode());
$this->assertSame($this->formatter->format([
'status' => 500,
@ -486,11 +579,19 @@ final class ResponseTraitTest extends CIUnitTestCase
$original = $_SERVER;
$_SERVER['CONTENT_TYPE'] = $mimeType;
$this->makeController([], '', ['Accept' => $mimeType]);
$this->assertSame($mimeType, $this->request->getHeaderLine('Accept'), 'Request header...');
$this->makeController('', ['Accept' => $mimeType]);
$this->assertSame(
$mimeType,
$this->request->getHeaderLine('Accept'),
'Request header...'
);
$this->response->setContentType($contentType);
$this->assertSame($contentType, $this->response->getHeaderLine('Content-Type'), 'Response header pre-response...');
$this->assertSame(
$contentType,
$this->response->getHeaderLine('Content-Type'),
'Response header pre-response...'
);
$_SERVER = $original;
}
@ -529,33 +630,8 @@ final class ResponseTraitTest extends CIUnitTestCase
public function testFormatByRequestNegotiateIfFormatIsNotJsonOrXML(): void
{
$config = new App();
foreach ([
'baseURL' => 'http://example.com/',
'uriProtocol' => 'REQUEST_URI',
'defaultLocale' => 'en',
'negotiateLocale' => false,
'supportedLocales' => ['en'],
'CSPEnabled' => false,
'proxyIPs' => [],
] as $key => $value) {
$config->{$key} = $value;
}
$cookie = new Cookie();
foreach ([
'prefix' => '',
'domain' => '',
'path' => '/',
'secure' => false,
'httponly' => false,
'samesite' => 'Lax',
] as $key => $value) {
$cookie->{$key} = $value;
}
Factories::injectMock('config', 'Cookie', $cookie);
$config = $this->createAppConfig();
$this->createCookieConfig();
$request = new MockIncomingRequest($config, new SiteURI($config), null, new UserAgent());
$response = new MockResponse($config);
@ -566,7 +642,7 @@ final class ResponseTraitTest extends CIUnitTestCase
protected $request;
protected $response;
public function __construct(&$request, &$response)
public function __construct($request, $response)
{
$this->request = $request;
$this->response = $response;
@ -577,7 +653,10 @@ final class ResponseTraitTest extends CIUnitTestCase
$this->invoke($controller, 'respondCreated', [['id' => 3], 'A Custom Reason']);
$this->assertStringStartsWith(config('Format')->supportedResponseFormats[0], $response->getHeaderLine('Content-Type'));
$this->assertStringStartsWith(
config('Format')->supportedResponseFormats[0],
$response->getHeaderLine('Content-Type')
);
}
public function testResponseFormat(): void
@ -588,13 +667,19 @@ final class ResponseTraitTest extends CIUnitTestCase
$this->invoke($controller, 'setResponseFormat', ['json']);
$this->invoke($controller, 'respond', [$data, 201]);
$this->assertStringStartsWith('application/json', $this->response->getHeaderLine('Content-Type'));
$this->assertStringStartsWith(
'application/json',
$this->response->getHeaderLine('Content-Type')
);
$this->assertSame($this->formatter->format($data), $this->response->getJSON());
$this->invoke($controller, 'setResponseFormat', ['xml']);
$this->invoke($controller, 'respond', [$data, 201]);
$this->assertStringStartsWith('application/xml', $this->response->getHeaderLine('Content-Type'));
$this->assertStringStartsWith(
'application/xml',
$this->response->getHeaderLine('Content-Type')
);
}
public function testXMLResponseFormat(): void

6
tests/system/Test/ControllerTestTraitTest.php
View File

@ -175,9 +175,9 @@ final class ControllerTestTraitTest extends CIUnitTestCase
->controller(Popcorn::class)
->execute('weasel');
$body = $result->response()->getBody(); // empty
$this->assertEmpty($body);
$this->assertFalse($result->isOK());
$body = $result->response()->getBody(); // empty string as JSON
$this->assertSame('""', $body);
$this->assertTrue($result->isOK());
}
public function testRedirect(): void

7
user_guide_src/source/changelogs/v4.5.0.rst
View File

@ -51,6 +51,13 @@ Due to a bug fix, the behavior has changed so that options passed to the outer
``group()`` are merged with the options of the inner ``group()``.
See :ref:`Upgrading Guide <upgrade-450-nested-route-groups-and-options>` for details.
API\\ResponseTrait
------------------
Now when a response format is JSON, if you pass string data, the framework returns
a JSON response. In previous versions, it returned a HTML response.
See :ref:`Upgrading Guide <upgrade-450-api-response-trait>` for details.
Factories class
---------------

14
user_guide_src/source/installation/upgrade_450.rst
View File

@ -155,6 +155,20 @@ reversed.
Previous: route1 → route2 → filter1 → filter2
Now: route2 → route1 → filter2 → filter1
.. _upgrade-450-api-response-trait:
API\\ResponseTrait and String Data
==================================
In previous versions, if you pass string data to a trait method, the framework
returned an HTML response, even if the response format was determined to be JSON.
Now if you pass string data, it returns a JSON response correctly. See also
:ref:`api-response-trait-handling-response-types`.
You can keep the behavior in previous versions if you set the ``$stringAsHtml``
property to ``true`` in your controller.
FileLocator::findQualifiedNameFromPath()
========================================

17
user_guide_src/source/outgoing/api_responses.rst
View File

@ -24,6 +24,8 @@ exist for the most common use cases:
.. literalinclude:: api_responses/002.php
.. _api-response-trait-handling-response-types:
***********************
Handling Response Types
***********************
@ -31,12 +33,17 @@ Handling Response Types
When you pass your data in any of these methods, they will determine the data type to format the results as based on
the following criteria:
* If data is a string, it will be treated as HTML to send back to the client.
* If data is an array, it will be formatted according to the controller's ``$this->format`` value. If that is empty,
it will try to negotiate the content type with what the client asked for, defaulting to JSON
if nothing else has been specified within **Config/Format.php**, the ``$supportedResponseFormats`` property.
* The format is determined according to the controller's ``$this->format`` value.
If that is ``null``, it will try to negotiate the content type with what the
client asked for, defaulting to the first element (JSON by default) in the
``$supportedResponseFormats`` property within **app/Config/Format.php**.
* The data will be formatted according to the format. If the format is not JSON
and data is a string, it will be treated as HTML to send back to the client.
To define the formatter that is used, edit **Config/Format.php**. The ``$supportedResponseFormats`` contains a list of
.. note:: Prior to v4.5.0, due to a bug, if data is a string, it will be treated
as HTML even if the format is JSON.
To define the formatter that is used, edit **app/Config/Format.php**. The ``$supportedResponseFormats`` contains a list of
mime types that your application can automatically format the response for. By default, the system knows how to
format both XML and JSON responses: