No charset in json response - revisted #1631

Closed
chris68 opened this Issue Dec 26, 2013 · 19 comments

Comments

Projects
None yet
3 participants
@chris68

chris68 commented Dec 26, 2013

In #833 the support has been introduced and then has been dropped again since only UTF-8 as char set is correct.

Currently, not working for me. The response was not UTF-8 and I needed to explicitly add the UTF8 as charset in the header.

                case self::FORMAT_JSON:
                    $this->getHeaders()->set('Content-Type', 'application/json; charset= UTF-8');
                    $this->content = Json::encode($this->data);
                    break;
@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Dec 26, 2013

Member

That is expected. Valid JSON should always be in utf-8 and served with a proper header.

Member

samdark commented Dec 26, 2013

That is expected. Valid JSON should always be in utf-8 and served with a proper header.

@samdark samdark closed this Dec 26, 2013

@chris68

This comment has been minimized.

Show comment
Hide comment
@chris68

chris68 Dec 26, 2013

@samdark Exactly. And for that reason you should explicitly set it to UTF-8. Currently my json response without that fix will not return as proper UTF-8!

chris68 commented Dec 26, 2013

@samdark Exactly. And for that reason you should explicitly set it to UTF-8. Currently my json response without that fix will not return as proper UTF-8!

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Dec 26, 2013

Member

OK, adding a header explicitly won't hurt.

Member

samdark commented Dec 26, 2013

OK, adding a header explicitly won't hurt.

@samdark samdark reopened this Dec 26, 2013

@ghost ghost assigned samdark Dec 26, 2013

@chris68

This comment has been minimized.

Show comment
Hide comment
@chris68

chris68 Dec 26, 2013

Sounds reasonable and would be extremely helpful....

chris68 commented Dec 26, 2013

Sounds reasonable and would be extremely helpful....

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 26, 2013

Member

Please check the comment for the reason why the charset header is dropped: 595ac6d#commitcomment-3988192

Member

qiangxue commented Dec 26, 2013

Please check the comment for the reason why the charset header is dropped: 595ac6d#commitcomment-3988192

@chris68

This comment has been minimized.

Show comment
Hide comment
@chris68

chris68 Dec 26, 2013

I saw that change. But that was the application char set. Here it would be a fixed UTF-8 char set header.

chris68 commented Dec 26, 2013

I saw that change. But that was the application char set. Here it would be a fixed UTF-8 char set header.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 26, 2013

Member

How does the header look like?

Member

qiangxue commented Dec 26, 2013

How does the header look like?

@chris68

This comment has been minimized.

Show comment
Hide comment
@chris68

chris68 Dec 26, 2013

HTTP/1.1 200 OK
Date: Thu, 26 Dec 2013 19:21:24 GMT
Server: Apache/2.2.22 (Ubuntu)
X-Powered-By: PHP/5.4.23-1+sury.org~precise+1
Set-Cookie: PHPSESSID=tj8f1j6e7dp7n5cgeh5gh1bbb4; path=/; HttpOnly
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
Pragma: no-cache
Content-Type: application/json

chris68 commented Dec 26, 2013

HTTP/1.1 200 OK
Date: Thu, 26 Dec 2013 19:21:24 GMT
Server: Apache/2.2.22 (Ubuntu)
X-Powered-By: PHP/5.4.23-1+sury.org~precise+1
Set-Cookie: PHPSESSID=tj8f1j6e7dp7n5cgeh5gh1bbb4; path=/; HttpOnly
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
Pragma: no-cache
Content-Type: application/json

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 26, 2013

Member

I don't see the charset header.

Member

qiangxue commented Dec 26, 2013

I don't see the charset header.

@chris68

This comment has been minimized.

Show comment
Hide comment
@chris68

chris68 Dec 26, 2013

There is no charset header in my output!

chris68 commented Dec 26, 2013

There is no charset header in my output!

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 26, 2013

Member

I was asking what charset header you are expecting...

Member

qiangxue commented Dec 26, 2013

I was asking what charset header you are expecting...

@chris68

This comment has been minimized.

Show comment
Hide comment
@chris68

chris68 Dec 26, 2013

I start to see your point: for Content-Type: application/json there is no charset parameter....

However, only when I add the charset parameter my browser (firefox) shows the data correctly as UTF-8 encoded. At least in some occasions. But it works for me now and so a tend to agree that the code is correct.

chris68 commented Dec 26, 2013

I start to see your point: for Content-Type: application/json there is no charset parameter....

However, only when I add the charset parameter my browser (firefox) shows the data correctly as UTF-8 encoded. At least in some occasions. But it works for me now and so a tend to agree that the code is correct.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 26, 2013

Member

I guess it's related with browser. I'm closing this one unless we see other reports of similar problem. Thanks.

Member

qiangxue commented Dec 26, 2013

I guess it's related with browser. I'm closing this one unless we see other reports of similar problem. Thanks.

@qiangxue qiangxue closed this Dec 26, 2013

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Dec 27, 2013

Member

@qiangxue it's fine to be left opened till I'll fix it (github push problems currently). Header is charset: UTF-8. It's not an issue if you serve all the content with utf-8 but can be a problem if all the content is, for example, uses cp1251 but some URLs are serving JSON which should be utf-8.

Member

samdark commented Dec 27, 2013

@qiangxue it's fine to be left opened till I'll fix it (github push problems currently). Header is charset: UTF-8. It's not an issue if you serve all the content with utf-8 but can be a problem if all the content is, for example, uses cp1251 but some URLs are serving JSON which should be utf-8.

@samdark samdark reopened this Dec 27, 2013

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 27, 2013

Member

Is there such a header charset?

Member

qiangxue commented Dec 27, 2013

Is there such a header charset?

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 27, 2013

Member

I also don't quite understand the use case you described "It's not an issue if you serve all the content with utf-8 but can be a problem if all the content is, for example, uses cp1251 but some URLs are serving JSON which should be utf-8."

Do you mean some responses use cp1251 while some use JSON/UTF-8? Why does this case matter here?

Member

qiangxue commented Dec 27, 2013

I also don't quite understand the use case you described "It's not an issue if you serve all the content with utf-8 but can be a problem if all the content is, for example, uses cp1251 but some URLs are serving JSON which should be utf-8."

Do you mean some responses use cp1251 while some use JSON/UTF-8? Why does this case matter here?

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Dec 27, 2013

Member

No, I've meant Content-Type: application/json; charset=utf-8. The issue with 1251 is that when you're setting an encoding for website you're typically doing it sitewide via config. On shared hosting it may be in control panel settings or can be just preset to 1251 or some other country-specific encoding.

So sitewide you're using 1251 for responses but JSON can't be served with anything other than UTF-8. That's why we're sending our own header overriding server config.

Member

samdark commented Dec 27, 2013

No, I've meant Content-Type: application/json; charset=utf-8. The issue with 1251 is that when you're setting an encoding for website you're typically doing it sitewide via config. On shared hosting it may be in control panel settings or can be just preset to 1251 or some other country-specific encoding.

So sitewide you're using 1251 for responses but JSON can't be served with anything other than UTF-8. That's why we're sending our own header overriding server config.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 28, 2013

Member

I see. I'm fine adding it back then.

Member

qiangxue commented Dec 28, 2013

I see. I'm fine adding it back then.

@chris68

This comment has been minimized.

Show comment
Hide comment
@chris68

chris68 Dec 28, 2013

I think it does not do harm to add it but I think according to spec Content-Type: application/json does not have an optional parameter charset. Only e.g. html has it (see http://www.iana.org/assignments/media-types/text/html)

But again: I do not think it does harm but sometimes it even seems to help certain browsers (see my remarks above). So I would added it but maybe put a comment in explaining why we did it.

chris68 commented Dec 28, 2013

I think it does not do harm to add it but I think according to spec Content-Type: application/json does not have an optional parameter charset. Only e.g. html has it (see http://www.iana.org/assignments/media-types/text/html)

But again: I do not think it does harm but sometimes it even seems to help certain browsers (see my remarks above). So I would added it but maybe put a comment in explaining why we did it.

@samdark samdark closed this in 404757b Dec 28, 2013

tonydspaniard added a commit to tonydspaniard/yii2 that referenced this issue Dec 29, 2013

Merge branch 'upstream' into 46-image-helper
* upstream: (35 commits)
  Fixes #1691: added “viewport” meta tag to layout views.
  Fixed the issue that query cache returns the same data for the same SQL but different query methods
  moved section
  subsection added
  typo [skip ci]
  docs about response
  Fixes #1586: `QueryBuilder::buildLikeCondition()` will now escape special characters and use percentage characters by default
  docs improved
  csrf docs added
  Fixes #1685: UrlManager::showScriptName should be set true for tests.
  Fixes #1688: ActiveForm is creating duplicated messages in error summary
  change back the visibility of findTableNames to protected.
  Typo fix.
  Fixes #1681: Added support for automatically adjusting the "for" attribute of label generated by `ActiveField::label()`
  Simplified tests.
  Fixes #1631: Charset is now explicitly set to UTF-8 when serving JSON
  Fixed typo
  added html layout for mail component in basic app
  CS fixes.
  Merge branch 'debug_module_improvements' of github.com:Ragazzo/yii2 into Ragazzo-debug_module_improvements
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment