Avoid modifying frozen strings

[Morriar]

Calling the force_encoding method on frozen strings can be a problem.
Especially for people using the # frozen_string_literal: true annotation.

To protect it, I added a call to dup before all calls to force_encoding.
If we decide the bump the ruby version to something newer we could use the +.

Signed-off-by: Alexandre Terrasa alexandre@moz-code.org

[jnunemaker]

I thought # frozen_string_literal: true only applied to the contents of that file, not all other libraries used in that file. What was the error you hit that led you down this path?

[TheSmartnik]

Well, if someone to run ruby with RUBYOPT="--enable-frozen-string-literal" it may be a problem. Unfortunately, httparty depends on a library that mutates strings anyway, so it won't be of much help 😞
sferik/multi_xml#64

[rafaelfranca]

I don't think those issues are related. # frozen_string_literal: true only applies to the content of that file, but let's say one of those contents are the body that is passed to httparty. In that case body will be frozen and httparty will try to mutate that string. This is what is happening here. We are not using RUBYOPT="--enable-frozen-string-literal".

You can reproduce the problem with HTTParty.post(url, "foo".freeze).

[TheSmartnik]

@rafaelfranca thanks for clarifying, you're right

@Morriar sorry it look so long to review and thatnks for the pr

[TheSmartnik]

@rafaelfranca @Morriar actually, I think I merged this too soon. The thing is HTTParty::TextEncoder is used only for handling response from server, which can't really be frozen with frozen_string_literal: true

You can reproduce the problem with HTTParty.post(url, "foo".freeze)

Second parameter accepts hash, not a string. So I can't really reproduce it 😕
Could you please provide a better reproduction example?

[rafaelfranca]

This is a combination between stubs using webmock and using httparty as client.

In a test like:

stub_request(:post, url, status: 200, body: '{"data": 1}'.freeze) # or with this stub being defined in a file with frozen_string_literal comment

HTTParty.post(url, some_hash)

this would fail.

[TheSmartnik]

Ah, I see. Didn't think about this use-case. Thanks for explaining @rafaelfranca