postrank-labs / postrank-uri

URI normalization, c14n, escaping, and extraction
MIT License
301 stars 52 forks source link

Remove Addressable monkey patch? #49

Open dentarg opened 1 year ago

dentarg commented 1 year ago

From Addressable 2.8.2 it no longer works:

$ ruby -rbundler/inline -e 'gemfile do; source "https://rubygems.org"; gem "postrank-uri"; gem "addressable", "2.8.2"; end; Addressable::URI.parse("google.com").normalize'
/Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:1636:in `query=': Can't convert Object into String. (TypeError)

        raise TypeError, "Can't convert #{new_query.class} into String."
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    from /Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:851:in `block in initialize'
    from /Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:2392:in `defer_validation'
    from /Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:840:in `initialize'
    from /Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:2168:in `new'
    from /Users/dentarg/.arm64_rubies/3.2.2/lib/ruby/gems/3.2.0/gems/addressable-2.8.2/lib/addressable/uri.rb:2168:in `normalize'
    from -e:1:in `<main>'

Seeing the patch is soon 12 years old, https://github.com/postrank-labs/postrank-uri/commit/110ed0bb92f5c1cd258970b76ce8dfacd1a3f60c, is it still needed?

Was originally reported in https://github.com/sporkmonger/addressable/issues/506

igrigorik commented 1 year ago

Yes, probably due for an update! Would you be willing to stage a PR?

dentarg commented 1 year ago

I'm not using postrank but maybe @danielnolan or @rickyc or @ck250186 is up for it! (users that reported the problem in the first place)

ericfirth commented 10 months ago

I am running into this issue and its making difficult to update a bunch of gems, so I attempted a fix here: https://github.com/postrank-labs/postrank-uri/pull/50