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

Fixing a bug in Message::isJSON #4081

Merged
merged 2 commits into from
Jan 10, 2021

Conversation

dcaswel
Copy link
Contributor

@dcaswel dcaswel commented Jan 8, 2021

Fixes the bug pointed out in a comment on #3719.

Description
Fixed an issue in isJSON that was breaking it if the content type header had the encoding in it as well.

Checklist:

  • Securely signed commits
  • [ ]N/A Component(s) with PHPdocs
  • Unit testing, with >80% coverage
  • [ ]N/A User guide updated
  • Conforms to style guide

@dcaswel
Copy link
Contributor Author

dcaswel commented Jan 8, 2021

@MGatner I did notice that you put a deprecated flag on this function. I'm wondering why that is when you were the one that suggested the creation of the function. Given the change needed in this PR, I am wondering if it might be a better idea to keep it?

@MGatner
Copy link
Member

MGatner commented Jan 8, 2021

I'm okay with this PR since it is a bugfix, but we did decide to deprecate that. I know I recommended it but since I have spent a lot of time with the HTTP layer. One of the goals is to incorporate a PSR-7 HTTP layer into the framework, and a corollary to that is relocating functionality to their proper locations.

Comment on lines +150 to +153
$header = $this->header('Content-Type')->getValue();
$parts = explode(';', $header);

return in_array('application/json', $parts, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$header = $this->header('Content-Type')->getValue();
$parts = explode(';', $header);
return in_array('application/json', $parts, true);
return in_array('application/json', explode(';', $this->header('Content-Type')->getValue()), true);

use one line

Copy link
Member

Choose a reason for hiding this comment

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

@mostafakhudair that's harder to read/understand the flow.

@samsonasik samsonasik merged commit 6861a58 into codeigniter4:develop Jan 10, 2021
@samsonasik
Copy link
Member

Thank you @caswell-wc

@dcaswel
Copy link
Contributor Author

dcaswel commented Jan 13, 2021

You're welcome @samsonasik.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants