Skip to content

fix(templates): not add default charset when upstream response doesn't contain it #9905

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

Merged
merged 5 commits into from
Jan 4, 2023

Conversation

catbro666
Copy link
Contributor

@catbro666 catbro666 commented Dec 7, 2022

Currently, Kong adds charset=UTF-8 to Content-Type header of response if the content-type is one of charset_types and contains no charset.

Kong doesn't know the charset of upstream response and can't assume it is UTF-8. Also we shouldn't change the response unless explicitly required e.g. by the response transformer plugin.

FTI-4551

@catbro666 catbro666 force-pushed the charset-utf8-to-off branch from 3635200 to c4c83a1 Compare December 7, 2022 09:12
@catbro666 catbro666 changed the title fix(conf): not add default charset when upstream response doesn't contain it fix(template): not add default charset when upstream response doesn't contain it Dec 7, 2022
@catbro666 catbro666 force-pushed the charset-utf8-to-off branch from c4c83a1 to b82e49e Compare December 7, 2022 09:15
@catbro666 catbro666 changed the title fix(template): not add default charset when upstream response doesn't contain it fix(templates): not add default charset when upstream response doesn't contain it Dec 7, 2022
@pull-request-size pull-request-size bot added size/M and removed size/XS labels Dec 8, 2022
@catbro666 catbro666 force-pushed the charset-utf8-to-off branch from 9ca98d8 to 39eabbb Compare December 8, 2022 08:07
@catbro666 catbro666 added pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) and removed pr/do not merge labels Dec 8, 2022
@catbro666 catbro666 force-pushed the charset-utf8-to-off branch from 39eabbb to 8fa6472 Compare December 8, 2022 08:10
@catbro666 catbro666 removed the pr/ready (but hold merge) No more concerns, but do not merge yet (probably a conflict of interest with another PR or release) label Dec 12, 2022
catbro666 and others added 4 commits January 3, 2023 10:48
…t contain it

Currently, Kong adds `charset=UTF-8` to `Content-Type` header of response
if the content-type is one of `charset_types` and contains no charset.

Kong doesn't know the charset of upstream response and can't assume it
is UTF-8. Also we shouldn't change the response unless explicitly required
e.g. by the response transformer plugin.

FTI-4551
Co-authored-by: Datong Sun <datong.sun@konghq.com>
@catbro666 catbro666 force-pushed the charset-utf8-to-off branch from 9e993b7 to d9ee248 Compare January 3, 2023 02:50
@catbro666 catbro666 force-pushed the charset-utf8-to-off branch from 7723103 to dcfa845 Compare January 3, 2023 02:57
@dndx dndx merged commit a428800 into master Jan 4, 2023
@dndx dndx deleted the charset-utf8-to-off branch January 4, 2023 04:54
windmgc pushed a commit that referenced this pull request Jan 13, 2023
…'t provide it (#9905)

Currently Kong adds `charset=UTF-8` to `Content-Type` header of response
if upstream provided `Content-Type` contains no `charset`.

Kong should not be assuming the charset of upstream response
is `UTF-8`. Also we shouldn't change the response charset unless
explicitly configured by the response transformer plugin.

FTI-4551

Co-authored-by: Datong Sun <datong.sun@konghq.com>
bungle added a commit that referenced this pull request Jan 13, 2023
… configurable

### Summary

`charset` directive that was previously hard coded in template with a value of `UTF-8`
was removed completely with this PR:
#9905

Previous discussion from 3 years ago pointed that this is breaking change (not a fix):
#5045

So here is another fix to make the directive still default to `UTF-8`. which is a quite
fine default, but allow users to configure it. E.g.:

```
KONG_NGINX_HTTP_CHARSET=off kong start
```
bungle added a commit that referenced this pull request Jan 16, 2023
… configurable

### Summary

`charset` directive that was previously hard coded in template with a value of `UTF-8`
was removed completely with this PR:
#9905

Previous discussion from 3 years ago pointed that this is breaking change (not a fix):
#5045

So here is another fix to make the directive still default to `UTF-8`. which is a quite
fine default, but allow users to configure it. E.g.:

```
KONG_NGINX_HTTP_CHARSET=off kong start
```
bungle added a commit that referenced this pull request Jan 17, 2023
… configurable (#10111)

### Summary

`charset` directive that was previously hard coded in template with a value of `UTF-8`
was removed completely with this PR:
#9905

Previous discussion from 3 years ago pointed that this is breaking change (not a fix):
#5045

So here is another fix to make the directive still default to `UTF-8`. which is a quite
fine default, but allow users to configure it. E.g.:

```
KONG_NGINX_HTTP_CHARSET=off kong start
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants