Skip to content

fix: API\ResponseTrait can't return string as JSON#8490

Merged
kenjis merged 17 commits into
codeigniter4:4.5from
kenjis:fix-ResponseTrait-format
Feb 4, 2024
Merged

fix: API\ResponseTrait can't return string as JSON#8490
kenjis merged 17 commits into
codeigniter4:4.5from
kenjis:fix-ResponseTrait-format

Conversation

@kenjis

@kenjis kenjis commented Jan 31, 2024

Copy link
Copy Markdown
Member

Description
Fixes #8397

  • When you pass a string, $this->respond() returns text/html. But it should return application/json.
    • When you pass an int, $this->respond() returns application/json.
    • When you pass a boolean, $this->respond() returns application/json.

Before:

$ curl -v http://localhost:8080/
...
< HTTP/1.1 200 OK
< Host: localhost:8080
< Date: Wed, 31 Jan 2024 07:57:40 GMT
< Connection: close
< X-Powered-By: PHP/8.2.15
< Cache-Control: no-store, max-age=0, no-cache
< Content-Type: text/html; charset=UTF-8
< 
* Closing connection
message

After:

$ curl -v http://localhost:8080/
...
< HTTP/1.1 200 OK
< Host: localhost:8080
< Date: Wed, 31 Jan 2024 07:59:11 GMT
< Connection: close
< X-Powered-By: PHP/8.2.15
< Cache-Control: no-store, max-age=0, no-cache
< Content-Type: application/json; charset=UTF-8
< 
* Closing connection
"message"
<?php

namespace App\Controllers;

use CodeIgniter\API\ResponseTrait;

class Home extends BaseController
{
    use ResponseTrait;

    public function index()
    {
        return $this->respond('message', 200);
    }
}

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added bug Verified issues on the current code behavior or pull requests that will fix them breaking change Pull requests that may break existing functionalities 4.5 labels Jan 31, 2024
@kenjis kenjis force-pushed the fix-ResponseTrait-format branch from a286a7d to 625628c Compare January 31, 2024 08:08
@michalsn

Copy link
Copy Markdown
Member

Unless we will be able to parse the same JSON on the input, I think I will not be in favor.

IMO we should support the same JSON parsing options for both: Request and Response.

@kenjis

kenjis commented Feb 1, 2024

Copy link
Copy Markdown
Member Author

We can parse the string JSON "message".

$routes->post('/', 'Home::index');
<?php

namespace App\Controllers;

class Home extends BaseController
{
    public function index(): string
    {
        $json = $this->request->getJSON();
        echo $json; exit;
    }
}
$ curl -H 'Content-Type: application/json' -d '"message"' localhost:8080/
message

@michalsn

michalsn commented Feb 1, 2024

Copy link
Copy Markdown
Member

Sorry, looks like I was mistaken. I thought we had a problem with that.

@trampgeek trampgeek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Many thanks, Kenjis.

@kenjis kenjis force-pushed the fix-ResponseTrait-format branch from e686a30 to f8b41ab Compare February 2, 2024 01:19
@kenjis

kenjis commented Feb 2, 2024

Copy link
Copy Markdown
Member Author

Added changelog and upgrade guide.

@kenjis kenjis force-pushed the fix-ResponseTrait-format branch from f8b41ab to cd2b2d2 Compare February 2, 2024 06:44
@kenjis kenjis force-pushed the fix-ResponseTrait-format branch from cd2b2d2 to fac0fe2 Compare February 4, 2024 02:02
@kenjis kenjis merged commit 6126950 into codeigniter4:4.5 Feb 4, 2024
@kenjis kenjis deleted the fix-ResponseTrait-format branch February 4, 2024 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Pull requests that may break existing functionalities bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants