Project

General

Profile

Bug #13926

Non UTF response headers raise an Argument error since 2.4.2p198

Added by petehamilton (Pete Hamilton) almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.4.2p198 (2017-09-14 revision 59899) [x86_64-darwin16]
[ruby-core:82922]

Description

When setting headers using Net::HTTPHeader#add_field or Net::HTTPHeader#[]= in v2.4.2, an ArgumentError (invalid byte sequence in UTF-8) is raised.

In 2.4.1, this behaviour didn't exist and it looks like it was introduced in one of the revisions associated with https://bugs.ruby-lang.org/issues/13852, where the header value is matched against a regular expression to prevent newlines.

Previously, Net::HTTP would accept non-UTF8 header values and just return them as invalid UTF8 strings. It was then on the user of Net::HTTP to handle this. With this change, there's now no way for the user to handle the case where they receive non-UTF8 header values as Net::HTTP raises an error.

RFC2616 allowed an HTTP header field content to be made up of any non-whitespace octets. Because of this RFC7230 makes an allowance for all characters in the ISO-8859-1 charset (both lower and extended ASCII characters).

Specifically, this section of RFC7230 suggests that although ideally response header values would be compatible with UTF-8, we can't assume this to be the case.

Historically, HTTP has allowed field content with text in the
ISO-8859-1 charset [ISO-8859-1], supporting other charsets only
through use of [RFC2047] encoding. In practice, most HTTP header
field values use only a subset of the US-ASCII charset [USASCII].
Newly defined header fields SHOULD limit their field values to
US-ASCII octets. A recipient SHOULD treat other octets in field
content (obs-text) as opaque data.

Not entirely sure where to go from here or what the fix is but given this is a behaviour change, it'd be great to hear your thoughts.


Files

net_http_utf8_tests.patch (1.14 KB) net_http_utf8_tests.patch Patch for the tests which pass on 2.4.1 and are now failing on 2.4.2 petehamilton (Pete Hamilton), 09/21/2017 05:20 PM

Associated revisions

Revision 56f91c6e
Added by naruse (Yui NARUSE) almost 2 years ago

HTTPHeader#add_field should allow binary [Bug #13926]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@60021 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 60021
Added by naruse (Yui NARUSE) almost 2 years ago

HTTPHeader#add_field should allow binary [Bug #13926]

Revision 60021
Added by naruse (Yui NARUSE) almost 2 years ago

HTTPHeader#add_field should allow binary [Bug #13926]

Revision 60021
Added by naruse (Yui NARUSE) almost 2 years ago

HTTPHeader#add_field should allow binary [Bug #13926]

Revision d2b5c16e
Added by nagachika (Tomoyuki Chikanaga) over 1 year ago

merge revision(s) 60021: [Backport #13926]

    HTTPHeader#add_field should allow binary [Bug #13926]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_4@61373 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 61373
Added by nagachika (Tomoyuki Chikanaga) over 1 year ago

merge revision(s) 60021: [Backport #13926]

HTTPHeader#add_field should allow binary [Bug #13926]

Revision ecb7182f
Added by usa (Usaku NAKAMURA) over 1 year ago

merge revision(s) 60021: [Backport #13926]

    HTTPHeader#add_field should allow binary [Bug #13926]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_2_3@62133 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 62133
Added by usa (Usaku NAKAMURA) over 1 year ago

merge revision(s) 60021: [Backport #13926]

HTTPHeader#add_field should allow binary [Bug #13926]

History

Updated by shevegen (Robert A. Heiler) almost 2 years ago

Can you add a link to Net::HTTPHeader#add_header?

I was trying to find it but I can not find it at https://ruby-doc.org/stdlib/libdoc/net/http/rdoc/Net/HTTPHeader.html - I do find []= though https://ruby-doc.org/stdlib-2.4.2/libdoc/net/http/rdoc/Net/HTTPHeader.html#method-i-5B-5D-3D and the documentation does not mention anything about Encoding.

I think either way, the behaviour in regards to Encoding should be noted down in the documentation as well. I assume that the change introduced a regression but I have absolutely no idea; the documentation should mention this somewhere though, either at the method, or at the main page or Net::HTTPHeader imo.

Updated by petehamilton (Pete Hamilton) almost 2 years ago

shevegen (Robert A. Heiler) wrote:

Can you add a link to Net::HTTPHeader#add_header?

I was trying to find it but I can not find it at https://ruby-doc.org/stdlib/libdoc/net/http/rdoc/Net/HTTPHeader.html

Apologies, I meant Net::HTTPHeader#add_field (https://ruby-doc.org/stdlib-2.4.2/libdoc/net/http/rdoc/Net/HTTPHeader.html#method-i-add_field). I will update the ticket description.

#3

Updated by petehamilton (Pete Hamilton) almost 2 years ago

  • Description updated (diff)
#4

Updated by naruse (Yui NARUSE) almost 2 years ago

  • Status changed from Open to Closed

Applied in changeset trunk|r60021.


HTTPHeader#add_field should allow binary [Bug #13926]

#5

Updated by naruse (Yui NARUSE) almost 2 years ago

  • Backport changed from 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN to 2.2: DONTNEED, 2.3: REQUIRED, 2.4: REQUIRED

Updated by nagachika (Tomoyuki Chikanaga) over 1 year ago

  • Backport changed from 2.2: DONTNEED, 2.3: REQUIRED, 2.4: REQUIRED to 2.2: DONTNEED, 2.3: REQUIRED, 2.4: DONE

ruby_2_4 r61373 merged revision(s) 60021.

Updated by usa (Usaku NAKAMURA) over 1 year ago

  • Backport changed from 2.2: DONTNEED, 2.3: REQUIRED, 2.4: DONE to 2.2: DONTNEED, 2.3: DONE, 2.4: DONE

ruby_2_3 r62133 merged revision(s) 60021.

Also available in: Atom PDF