Skip to content

Make charset parameter name comparison case-insensitive - #474

Open
rootkiller6788 wants to merge 1 commit into
google:masterfrom
rootkiller6788:fix-parse-charset-case-insensitive
Open

Make charset parameter name comparison case-insensitive#474
rootkiller6788 wants to merge 1 commit into
google:masterfrom
rootkiller6788:fix-parse-charset-case-insensitive

Conversation

@rootkiller6788

Copy link
Copy Markdown

Problem

HttpHeaderParser.parseCharset() only matches the exact lowercase charset parameter name when parsing a Content-Type header:

if (pair[0].equals("charset")) {
    return pair[1];
}

HTTP header parameter names are case-insensitive (RFC 7230 section 3.2.6). A server that emits e.g. Content-Type: text/plain; Charset=UTF-8 or CHARSET=UTF-8 therefore has its charset silently ignored, and the response body falls back to the default encoding. For StringRequest the default is ISO-8859-1, so any non-ASCII text (e.g. UTF-8 JSON or localized content) is decoded as mojibake.

Fix

Compare the parameter name with equalsIgnoreCase so charset, Charset, and CHARSET are all recognized, matching the case-insensitivity of the rest of header handling in Volley (header names are already looked up case-insensitively via a TreeMap with String.CASE_INSENSITIVE_ORDER).

Verification

  • Reproduced the bug against the unmodified source with a standalone harness: Content-Type: text/plain; Charset=UTF-8 returned ISO-8859-1; after the fix it returns UTF-8. CHARSET=Shift_JIS is also recognized.
  • Added test coverage to HttpHeaderParserTest.parseCharset() for mixed- and upper-case charset parameter names.

HTTP header parameter names are case-insensitive per RFC 7230, so a
Content-Type of "text/plain; Charset=UTF-8" should be recognized the
same as "charset=utf-8". Previously only the exact lowercase form was
matched, causing the charset to be ignored and the response to be
decoded with the default ISO-8859-1 encoding (or the passed-in default),
garbling non-ASCII text in StringRequest responses.
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.

1 participant