httprb / http

HTTP (The Gem! a.k.a. http.rb) - a fast Ruby HTTP client with a chainable API, streaming support, and timeouts
MIT License
3.01k stars 321 forks source link

Should responses with content-type application/json have UTF-8 encoding by default? #714

Closed drwl closed 2 years ago

drwl commented 2 years ago

Currently, responses without a specified charset encoding in the header are stored internally with binary/ascii 8bit encoding. According to the spec, the specified charset should be redundant. Should the default for json responses by utf-8 instead?

From RFC8259:

JSON text exchanged between systems that are not part of a closed ecosystem MUST be encoded using UTF-8 [RFC3629].

Previous specifications of JSON have not required the use of UTF-8 when transmitting JSON text. However, the vast majority of JSON- based software implementations have chosen to use the UTF-8 encoding, to the extent that it is the only encoding that achieves interoperability.

Also in RFC8259, at the end of Section 11 (or just above of Section 12):

Note: No "charset" parameter is defined for this registration. Adding one really has no effect on compliant recipients.

tarcieri commented 2 years ago

That seems like a reasonable default, yes

drwl commented 2 years ago

That seems like a reasonable default, yes

I can try to put up a PR. Where do you suggest this code change take place?

My initial thought is to change HTTP::Response initializer:

        encoding   = opts[:encoding] || charset

        if encoding.nil?
          if mime_type == 'application/json'
            encoding = Encoding::UTF_8
          else
            encoding = Encoding::BINARY
          end
        end

thoughts?

tarcieri commented 2 years ago

Sounds good to me /cc @ixti

ixti commented 2 years ago

@tarcieri I like the idea. Reviewed the PR.

drwl commented 2 years ago

@ixti @tarcieri thanks for quick turn around. Not to rush, but do yall have any idea when I can expect a release with this change?

I came across this related to work and so I'm figuring out if I should make changes to support this internally or if I can wait and update the gem (when it's out).

ixti commented 2 years ago

Released as 5.1.0

ixti commented 2 years ago

I can cut release layer today

On Thu, 16 Jun 2022, 19:45 Andrew W. Lee, @.***> wrote:

@ixti https://github.com/ixti @tarcieri https://github.com/tarcieri thanks for quick turn around. Not to rush, but do yall have any idea when I can expect a release with this change?

I came across this related to work and so I'm figuring out if I should make changes to support this internally or if I can wait and update the gem (when it's out).

— Reply to this email directly, view it on GitHub https://github.com/httprb/http/issues/714#issuecomment-1157959960, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAXEPVV24CMIAFOIQS2ESLVPNR2DANCNFSM5YV3IZWQ . You are receiving this because you were mentioned.Message ID: @.***>