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

Remove charset=utf-8 parameter from application/json content type #3645

Closed
2 of 4 tasks
MrPropre opened this issue Mar 6, 2021 · 4 comments · Fixed by #3946
Closed
2 of 4 tasks

Remove charset=utf-8 parameter from application/json content type #3645

MrPropre opened this issue Mar 6, 2021 · 4 comments · Fixed by #3946
Assignees
Labels
effort: [XS] < 1 day of estimated development time good first issue Indicates a good issue for first-time contributors module: critical path css priority: low Issues that can wait severity: minor Defect that does not affect functionality type: bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@MrPropre
Copy link

MrPropre commented Mar 6, 2021

Before submitting an issue please check that you’ve completed the following steps:

  • Made sure you’re on the latest version
  • Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug

As per IANA notes on media types https://www.iana.org/assignments/media-types/application/json the charset parameter has no effect on the content type application/json.

A user reported that Chrome started throwing CORB (Cross-Origin-Read-Blocking) warnings when the content type was set to application/json; charset=utf-8

slimphp/Slim#2629 (comment)

Expected behavior

We need to remove ; charset=utf-8

The affected files are:

xhttp.setRequestHeader( 'Content-Type', 'application/json;charset=UTF-8' );

AddCharset UTF-8 .atom .css .js .json .rss .vtt .xml

$rules .= "AddCharset $charset .atom .css .js .json .rss .vtt .xml" . PHP_EOL;

Backlog Grooming (for WP Media dev team use only)

  • Reproduce the problem
  • Identify the root cause
  • Scope a solution
  • Estimate the effort
@Tabrisrp
Copy link
Contributor

Hi @MrPropre,

Did you encounter the CORB warning when using the plugin?

@MrPropre
Copy link
Author

Hello @Tabrisrp,

Not personally (I use Firefox) but I know that in most cases the warning can safely be ignored. Here is the documentation from Google: https://www.chromium.org/Home/chromium-security/corb-for-developers

A response served with a "X-Content-Type-Options: nosniff" response header and an incorrect "Content-Type" response header, may be blocked.

In most cases, the blocked response should not affect the web page's behavior and the CORB error message can be safely ignored.

It seems that adding ;charset=utf-8 to an application/json content type makes it incorrect due to Chrome's content type enhancement. Additionally, by checking IANA's site, we can see no charset parameter was ever actually defined in any RFC for the application/json type. It would be logical to remove it to stop the CORB warning on chrome, and to lighten the response size a bit.

Thank you.

@Tabrisrp Tabrisrp added type: bug Indicates an unexpected problem or unintended behavior severity: minor Defect that does not affect functionality priority: low Issues that can wait and removed module: htaccess labels Mar 30, 2021
@Tabrisrp
Copy link
Contributor

Thank you for your feedback.

I'm thinking we can safely remove this from the code in wpr-cpcss.js.

I have more doubts for the .htaccess file directives, so I would not touch this one for now.

@Tabrisrp
Copy link
Contributor

Scope a solution ✅

In assets/js/wpr-cpcss.js

  • Remove the ;charset=UTF-8 on line 88

Estimate the effort ✅

Effort [XS]

@Tabrisrp Tabrisrp added effort: [XS] < 1 day of estimated development time and removed needs: grooming labels Mar 30, 2021
@Tabrisrp Tabrisrp added the good first issue Indicates a good issue for first-time contributors label Apr 7, 2021
@mostafa-hisham mostafa-hisham self-assigned this May 25, 2021
@Tabrisrp Tabrisrp added this to the 3.9.0.1 milestone May 27, 2021
@Tabrisrp Tabrisrp mentioned this issue Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [XS] < 1 day of estimated development time good first issue Indicates a good issue for first-time contributors module: critical path css priority: low Issues that can wait severity: minor Defect that does not affect functionality type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants