ruby / uri

URI is a module providing classes to handle Uniform Resource Identifiers
https://ruby.github.io/uri/
Other
86 stars 47 forks source link

Provide `URI::RFC2396_PARSER` for Ruby 3.3 or older #118

Closed yahonda closed 2 months ago

yahonda commented 3 months ago

This issue requests to provide URI::RFC2396_PARSER for Ruby 3.3 or older version because Ruby on Rails supports Ruby 3.1.0 and higher and I want to support these Ruby versions without if RUBY_VERSION clauses.

Background

Ruby on Rails runs CI against Ruby master branch and it enables RAILS_STRICT_WARNINGS=1 flag to let Rails CI fail if any warnings appeared.

Since https://github.com/ruby/ruby/commit/b41d79962abefbe8b70d76dee134b44a3a3d3929, Rails Ci against Ruby master branch is getting failed as expected. To address this warning, I have replaced URI::DEFAULT_PARSER.escape with URI::RFC2396_PARSER.escape as suggested. It works against Ruby master branch but it raises uninitialized constant URI::RFC2396_PARSER (NameError) against Ruby 3.3.4 that bundles uri version 0.13.0

Steps to reproduce

$ irb -w

require 'uri'
URI::DEFAULT_PARSER.escape("/:controller(/:action)")
URI::RFC2396_PARSER.escape("/:controller(/:action)")

Expected behavior

Ruby 3.3.4 runs these statements without successfully warnings.

Actual behavior

It raises uninitialized constant URI::RFC2396_PARSER (NameError)

$ ruby -v
ruby 3.3.4 (2024-07-09 revision be1089c8ec) [x86_64-linux]
$ irb -w
irb(main):001> require 'uri'
=> true
irb(main):002> URI::VERSION
=> "0.13.0"
irb(main):003> URI::DEFAULT_PARSER.escape("/:controller(/:action)")
=> "/:controller(/:action)"
irb(main):004> URI::RFC2396_PARSER.escape("/:controller(/:action)")
(irb):4:in `<main>': uninitialized constant URI::RFC2396_PARSER (NameError)

URI::RFC2396_PARSER.escape("/:controller(/:action)")
   ^^^^^^^^^^^^^^^^
Did you mean?  URI::RFC2396_Parser
               URI::RFC3986_Parser
               URI::RFC3986_PARSER
    from <internal:kernel>:187:in `loop'
    from /home/yahonda/.rbenv/versions/3.3.4/lib/ruby/gems/3.3.0/gems/irb-1.14.0/exe/irb:9:in `<top (required)>'
    from /home/yahonda/.rbenv/versions/3.3.4/bin/irb:25:in `load'
    from /home/yahonda/.rbenv/versions/3.3.4/bin/irb:25:in `<main>'
irb(main):005>

Here is the result against Ruby 3.4.0dev, that works as expected.


$ ruby -v
ruby 3.4.0dev (2024-08-22T23:47:40Z master fdba458e85) [x86_64-linux]
$ irb -w
Ignoring io-console-0.7.2 because its extensions are not built. Try: gem pristine io-console --version 0.7.2
Ignoring psych-5.1.2 because its extensions are not built. Try: gem pristine psych --version 5.1.2
Ignoring cgi-0.4.1 because its extensions are not built. Try: gem pristine cgi --version 0.4.1
Ignoring date-3.3.4 because its extensions are not built. Try: gem pristine date --version 3.3.4
Ignoring io-console-0.7.2 because its extensions are not built. Try: gem pristine io-console --version 0.7.2
Ignoring json-2.7.2 because its extensions are not built. Try: gem pristine json --version 2.7.2
Ignoring prism-0.30.0 because its extensions are not built. Try: gem pristine prism --version 0.30.0
Ignoring psych-5.1.2 because its extensions are not built. Try: gem pristine psych --version 5.1.2
irb(main):001> require 'uri'
=> true
irb(main):002> URI::VERSION
=> "0.13.0"
irb(main):003> URI::DEFAULT_PARSER.escape("/:controller(/:action)")
(irb):3: warning: URI::RFC3986_PARSER.escape is obsoleted. Use URI::RFC2396_PARSER.escape explicitly.
=> "/:controller(/:action)"
irb(main):004> URI::RFC2396_PARSER.escape("/:controller(/:action)")
irb(main):005>
hsbt commented 3 months ago

Make sense. I will plan to release to URI-0.12.3 and 0.13.1 with #119 and #120. After that, I will request to backport them to Ruby 3.2 and 3.3.

Is it enough to your use-case?

yahonda commented 3 months ago

Is it enough to your use-case?

Current Rails main branch supports Ruby 3.1.0 and higher, So it would be appreciateed if URI::RFC2396_PARSER is also available for Ruby 3.1.

https://github.com/rails/rails/blob/c6e3336cfdec5ae9c7fb98b03d0196aa56ab30f9/rails.gemspec#L12

Once newer version of uri is available, we can add the uri as Active Support dependency.

https://github.com/rails/rails/blob/main/activesupport/activesupport.gemspec

yahonda commented 3 months ago

I meant to say I'm happy if the newer version of uri that supports Ruby 3.1+ is available at RubyGems, we can add this gem explicitly in the Active Support.

hsbt commented 3 months ago

This feature is not security fix, it's impossible to backport to Ruby 3.1 with our maintenance policy.

But URI-0.12.3 is also working Ruby 3.1. s.add_dependency 'uri', '>= 0.12.3' resolve the original issue.

yahonda commented 3 months ago

It is little bit off-topic for this request itself, I'm wondering how to write gemspec to let each Ruby versions to install appropriate uri versions.

I want to install uri version 0.12.3 on Ruby 3.1.z and Ruby 3.2.z and uri version 0.13.1 on Ruby 3.3.z. Also I'd like to avoid uri version 0.13.1 is installed for Ruby 3.1.z and 3.2.z.

s.add_dependency 'uri', '>= 0.12.3'

It should install the requried uri versions for Ruby 3.1.z and 3.2.z. However it may install the lower versions of uri for Ruby 3.3.z that ships with uri v0.13.0 that does not have the RFC2396_PARSER. Since it is not feasible to have gemspec file per Ruby version.

Earlopain commented 3 months ago

Is there a problem with requiring the latest uri on all rubies? I for example already have uri through faraday => net-http => uri which just pulls in the latest version. Faraday itself supports ruby >= 3.0.

hsbt commented 3 months ago

I released https://github.com/ruby/uri/releases/tag/v0.12.3 and https://github.com/ruby/uri/releases/tag/v0.13.1. They are also released with next stable versions, see https://github.com/ruby/ruby/pull/11465 and https://github.com/ruby/ruby/pull/11466.

Q. Why we deprecate them?

A. I and @yahonda ask that to @nurse in this week. URI.escape and these deprecated methods can't parse multibyte URI correctly. We shouldn't use for that.

yahonda commented 2 months ago

Thank you for releasing the v0.12.3 and v0.13.1. Considering the current RubyGem dependency resolution, I think Rails need to add v0.13.1 or higher version of uri.

yahonda commented 2 months ago

I'll open a pull request to rails/rails and will inform if there are some issues happens.

yahonda commented 2 months ago

I'll open a pull request to rails/rails and will inform if there are some issues happens.

Opened https://github.com/rails/rails/pull/52779

yahonda commented 2 months ago

Closing this issue as https://github.com/rails/rails/pull/52779 has been merged to Rails main branch. Thank you for providing the new version of uri.