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

Allow setting Net::HTTP#response_body_encoding #1028

Open
hallelujah opened this issue May 25, 2023 · 2 comments
Open

Allow setting Net::HTTP#response_body_encoding #1028

hallelujah opened this issue May 25, 2023 · 2 comments

Comments

@hallelujah
Copy link

Hello, I would like to make a feature request

It would be great if we could use response.body_encoding = true below this line following https://bugs.ruby-lang.org/issues/2567#note-39

response = ::Net::HTTPResponse.read_new(socket)

Test case (using rspec in my project) that fails:

  it "converts body based on charset" do
    body = "hello\u1234"
    http_response = <<-HTTP_RESPONSE.strip_heredoc.gsub("\n", "\r\n")
      HTTP/1.1 400 Bad Request
      Date: Wed, 24 May 2023 08:52:57 GMT
      Content-Type: text/plain; charset=utf-8
      Content-Length: #{body.bytesize}
      Connection: keep-alive
      cache-control: no-cache

      #{body}
    HTTP_RESPONSE

    io = StringIO.new(http_response)
    response = WebMock::ResponseFactory.response_for(io)
    expect(response.body.encoding).to eq Encoding.find("utf-8")
  end

Test fails with:

  1)  WebMock::ResponseFactory.response_for converts body based on charset
     Failure/Error: expect(response.body.encoding).to eq Encoding.find("utf-8")

       expected: #<Encoding:UTF-8>
            got: #<Encoding:ASCII-8BIT>

       (compared using ==)

       Diff:
       @@ -1 +1 @@
       -#<Encoding:UTF-8>
       +#<Encoding:ASCII-8BIT>

Suggestion

Adding options to response_for(io, body_encoding: false) false by default

@bblimke
Copy link
Owner

bblimke commented Aug 20, 2023

@hallelujah Thank you for reporting. You have suggested using body_encoding: false by default. Do you see any negative side effects of setting response.body_encoding = true ? I assume lack of response.body_encoding = true is a bug, but perhaps there are cases where it can be desired to have response.body_encoding = false ?

@hallelujah
Copy link
Author

@bblimke I was thinking not changing the current behaviour but on second thought I think you are right, it might actually be a bug.

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

No branches or pull requests

2 participants