Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 17 commits into from
Feb 4, 2024

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Jan 31, 2024

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
@michalsn
Copy link
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
Copy link
Member Author

kenjis commented Feb 1, 2024

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
Copy link
Member

michalsn commented Feb 1, 2024

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

Copy link

@trampgeek trampgeek left a comment

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
Copy link
Member Author

kenjis commented Feb 2, 2024

Added changelog and upgrade guide.

@kenjis kenjis merged commit 6126950 into codeigniter4:4.5 Feb 4, 2024
47 checks passed
@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
4.5 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