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

HttpUtil.getCharset() fails for charset in double-quotes #11370

Closed
jerrinot opened this issue Jun 7, 2021 · 2 comments · Fixed by #11373
Closed

HttpUtil.getCharset() fails for charset in double-quotes #11370

jerrinot opened this issue Jun 7, 2021 · 2 comments · Fixed by #11373

Comments

@jerrinot
Copy link

jerrinot commented Jun 7, 2021

I understood from RFC 7231 the content-type header can have charset value in double-quotes.

media-type = type "/" subtype *( OWS ";" OWS parameter )
type       = token
subtype    = token

The type/subtype MAY be followed by parameters in the form of name=value pairs.
parameter  = token "=" ( token / quoted-string )

Expected behaviour

When the content-type header is set to text/html; charset="utf-8" then the charset is parsed correctly as utf-8.

Actual behaviour

When the content-type header is set to text/html; charset="utf-8" then the charset is parsed incorrectly as default ISO-8859-1.

Steps to reproduce

Change the testGetCharset() to look like this:

@Test
public void testGetCharset() {
    String NORMAL_CONTENT_TYPE = "text/html; charset=utf-8";
    String UPPER_CASE_NORMAL_CONTENT_TYPE = "TEXT/HTML; CHARSET=UTF-8";
    String CHARSET_IN_DOUBLE_QUOTES = "text/html; charset=\"utf-8\"";

    HttpMessage message = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
    message.headers().set(HttpHeaderNames.CONTENT_TYPE, NORMAL_CONTENT_TYPE);
    assertEquals(CharsetUtil.UTF_8, HttpUtil.getCharset(message));
    assertEquals(CharsetUtil.UTF_8, HttpUtil.getCharset(NORMAL_CONTENT_TYPE));

    message.headers().set(HttpHeaderNames.CONTENT_TYPE, UPPER_CASE_NORMAL_CONTENT_TYPE);
    assertEquals(CharsetUtil.UTF_8, HttpUtil.getCharset(message));
    assertEquals(CharsetUtil.UTF_8, HttpUtil.getCharset(UPPER_CASE_NORMAL_CONTENT_TYPE));

    message.headers().set(HttpHeaderNames.CONTENT_TYPE, CHARSET_IN_DOUBLE_QUOTES);
    assertEquals(CharsetUtil.UTF_8, HttpUtil.getCharset(message));
    assertEquals(CharsetUtil.UTF_8, HttpUtil.getCharset(CHARSET_IN_DOUBLE_QUOTES));
}

Netty version

b0d1bff

@normanmaurer
Copy link
Member

@NiteshKant can you please take a look ?

NiteshKant pushed a commit to NiteshKant/netty that referenced this issue Jun 7, 2021
__Motivation__

As described in netty#11370 we should support quoted charset values

__Modification__

Modify `HttpUtil.getCharset(CharSequence contentTypeValue, Charset defaultCharset)` to trim the double-quotes if present.

__Result__

`HttpUtil.getCharset()` now supports quoted charsets.
@NiteshKant
Copy link
Member

@jerrinot thanks for reporting!

Here is the PR to fix it: #11373

normanmaurer pushed a commit that referenced this issue Jun 9, 2021
__Motivation__

As described in #11370 we should support quoted charset values

__Modification__

Modify `HttpUtil.getCharset(CharSequence contentTypeValue, Charset defaultCharset)` to trim the double-quotes if present.

__Result__

`HttpUtil.getCharset()` now supports quoted charsets. Fixes #11370
normanmaurer pushed a commit that referenced this issue Jun 9, 2021
__Motivation__

As described in #11370 we should support quoted charset values

__Modification__

Modify `HttpUtil.getCharset(CharSequence contentTypeValue, Charset defaultCharset)` to trim the double-quotes if present.

__Result__

`HttpUtil.getCharset()` now supports quoted charsets. Fixes #11370
raidyue pushed a commit to raidyue/netty that referenced this issue Jul 8, 2022
__Motivation__

As described in netty#11370 we should support quoted charset values

__Modification__

Modify `HttpUtil.getCharset(CharSequence contentTypeValue, Charset defaultCharset)` to trim the double-quotes if present.

__Result__

`HttpUtil.getCharset()` now supports quoted charsets. Fixes netty#11370
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 a pull request may close this issue.

3 participants