Constants in Addressable::URI sometimes get mutated

[QuinnWilton]

This is an issue I ran into when upgrading to 2.5, but I think other versions are affected in similar ways. Sometimes, when parsing a URL, some of the components of the parsed URL will be set directly to one of the constants defined in Addressable::URI. For example, if the path is "/", the instance variable will be set to Addressable::URI::SLASH.

If the parsed URL is then normalized, the encoding of @normalized_path is forced to be Encoding::UTF_8. This mutates the constant. This snippet demonstrates the behavior:

[1] pry(main)> Addressable::URI.parse("http://google.com").normalize
=> #<Addressable::URI:0x3fd9d0f0fe44 URI:http://google.com/>
[2] pry(main)> Addressable::URI::SLASH.freeze
=> "/"
[3] pry(main)> Addressable::URI.parse("http://google.com").normalize
RuntimeError: can't modify frozen String

The changed behavior is from this commit: 8f4fee0

This is currently blocking my team from upgrading to 2.5, because we've found value in using the IceNine gem to deep_freeze! our parsed URLs from Addressable: on more than one occasion it has caught other dependencies of ours mutating URLs in unexpected ways. I understand that this is a non-standard usage of your gem, and understand if as a result, you do not consider this to be a bug.

In the meantime, we will most likely continue using 2.4, but it might be worth thinking about possible solutions here, if only because mutating constants is unexpected.

I think a possible fix would be to either check if the components are already UTF-8 encoded before calling force_encoding on them, or to move the force_encoding call into the ||= begin block at the start of each of the normalized_* methods.

[dentarg]

If not before, this has to be fixed to be compatible with Ruby 3. :)

$ RUBYOPT="-w --enable-frozen-string-literal" irb
irb(main):001:0> require 'addressable/uri'
=> true
irb(main):002:0> Addressable::URI.parse("http://google.com").normalize
RuntimeError: can't modify frozen String
	from /Users/dentarg/.gem/ruby/2.3.3/gems/addressable-2.5.0/lib/addressable/uri.rb:439:in `force_encoding'
	from /Users/dentarg/.gem/ruby/2.3.3/gems/addressable-2.5.0/lib/addressable/uri.rb:439:in `unencode'
	from /Users/dentarg/.gem/ruby/2.3.3/gems/addressable-2.5.0/lib/addressable/uri.rb:536:in `normalize_component'
	from /Users/dentarg/.gem/ruby/2.3.3/gems/addressable-2.5.0/lib/addressable/uri.rb:852:in `normalized_scheme'
	from /Users/dentarg/.gem/ruby/2.3.3/gems/addressable-2.5.0/lib/addressable/uri.rb:2123:in `normalize'
	from (irb):2
	from /Users/dentarg/.rubies/ruby-2.3.3/bin/irb:11:in `<main>'
irb(main):003:0> RUBY_VERSION
=> "2.3.3"

[QuinnWilton]

That's a great point, I completely forgot about that!

@sporkmonger With your permission, I'm happy to fix this issue if you point me towards where you think the proper solution is. Happy to hop into an IRC chatroom or something too, if you want to discuss it at some point.

[sporkmonger]

This has occasionally come up. I think this is a perfectly reasonable usage of Addressable. We have a number of freeze tests already in the test suite, but clearly it's not a complete list. Seems to me that maybe we should just freeze all the constants, run the test suite and see what fails.

[bloomdido]

Just hit this myself when calling origin(). Looks like I'm going to have to revert to 2.4 until this gets addressed. 😃

[dentarg]

@ShaneWilton @bloomdido what is the reason you can't use Addressable with Ruby 2.5? is it something in regards to IceNine + Ruby 2.5 that changed versus IceNine + Ruby 2.4?

I've tried to understand (browsed news and blogs) what changed between Ruby 2.4 och 2.5, related to this, but so far I've come up empty.

[QuinnWilton]

@dentarg Sorry, my original post wasn't clear. I ran into issues upgrading to Addressable version 2.5, not Ruby 2.5. As of Addressable 2.5, many of the accessor methods mutate the underlying field where before they didn't.

You can see that in most of the normalized_* functions, as of this commit: 8f4fee0#diff-33998b1f6fff21cf34d63816274234b3R1781

I agree that it's probably worth ensuring normalized values are UTF-8 encoded, however force_encoding mutates the string even if the string is already marked as being UTF-8 encoded.