Skip to content

Commit

Permalink
Use URI::RFC2396_PARSER instead of URI::DEFAULT_PARSER
Browse files Browse the repository at this point in the history
Ruby 3.4 changes `URI::DEFAULT_PARSER` to `URI::RFC3986_Parser`
and deprecates `URI::RFC3986_PARSER.make_regexp`,`URI::RFC3986_PARSER.escape`, `URI::RFC3986_PARSER.unescape` and `URI::RFC3986_PARSER.extract`.

uri v0.12.2 for Ruby 3.2/3.1 and v0.13.1 for Ruby 3.3 adds `URI::RFC2396_PARSER`.
As of right now there is no way to use uri v0.12.2 for Ruby 3.2/3.1 and v0.13.1 for Ruby 3.3,
This commit uses v0.13.1 or higher version for all supported Ruby versions by Rails main branch.

It also reverts rails#52682 because the original issue has been resolved.

Refer to following URL for the backgrond of this change:
- URI::Generic should use URI::RFC3986_PARSER instead of URI::DEFAULT_PARSER
https://bugs.ruby-lang.org/issues/19266

- Use RFC3986_Parser by default
ruby/uri#107

- Warn compatibility methods in RFC3986_PARSER
ruby/uri#114

- Also warn URI::RFC3986_PARSER.extract
ruby/uri#121

- Define RFC2396_PARSER for Ruby 3.3
ruby/uri#119

- Define RFC2396_PARSER for Ruby 3.2 and 3.1
ruby/uri#120
  • Loading branch information
yahonda committed Sep 2, 2024
1 parent 01a5efc commit dbf3310
Show file tree
Hide file tree
Showing 13 changed files with 19 additions and 15 deletions.
3 changes: 3 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ gem "json", ">= 2.0.0", "!=2.7.0"
# Workaround until Ruby ships with cgi version 0.3.6 or higher.
gem "cgi", ">= 0.3.6", require: false

# Workaround until all supported Ruby versions ship with uri version 0.13.1 or higher.
gem "uri", ">= 0.13.1", require: false

gem "prism"

group :lint do
Expand Down
3 changes: 3 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ PATH
minitest (>= 5.1)
securerandom (>= 0.3)
tzinfo (~> 2.0, >= 2.0.5)
uri (>= 0.13.1)
rails (8.0.0.alpha)
actioncable (= 8.0.0.alpha)
actionmailbox (= 8.0.0.alpha)
Expand Down Expand Up @@ -596,6 +597,7 @@ GEM
concurrent-ruby (~> 1.0)
uber (0.1.0)
unicode-display_width (2.5.0)
uri (0.13.1)
useragent (0.16.10)
w3c_validators (1.3.7)
json (>= 1.8)
Expand Down Expand Up @@ -701,6 +703,7 @@ DEPENDENCIES
trilogy (>= 2.7.0)
turbo-rails
tzinfo-data
uri (>= 0.13.1)
useragent
w3c_validators (~> 1.3.6)
wdm (>= 0.1.0)
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/routing/inspector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def normalize_filter(filter)
{ controller: /#{filter[:controller].underscore.sub(/_?controller\z/, "")}/ }
elsif filter[:grep]
grep_pattern = Regexp.new(filter[:grep])
path = URI::DEFAULT_PARSER.escape(filter[:grep])
path = URI::RFC2396_PARSER.escape(filter[:grep])
normalized_path = ("/" + path).squeeze("/")

{
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/routing/mapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2033,7 +2033,7 @@ def add_route(action, controller, options, _path, to, via, formatted, anchor, op
name_for_action(options.delete(:as), action)
end

path = Mapping.normalize_path URI::DEFAULT_PARSER.escape(path), formatted
path = Mapping.normalize_path URI::RFC2396_PARSER.escape(path), formatted
ast = Journey::Parser.parse path

mapping = Mapping.build(@scope, @set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options)
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/routing/route_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ def recognize_path_with_request(req, path, extras, raise_on_missing: true)
params.each do |key, value|
if value.is_a?(String)
value = value.dup.force_encoding(Encoding::BINARY)
params[key] = URI::DEFAULT_PARSER.unescape(value)
params[key] = URI::RFC2396_PARSER.unescape(value)
end
end
req.path_parameters = params
Expand Down
2 changes: 1 addition & 1 deletion actionpack/test/controller/parameter_encoding_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class ParameterEncodingTest < ActionController::TestCase
end

test "does not raise an error when passed a param declared as ASCII-8BIT that contains invalid bytes" do
get :test_skip_parameter_encoding, params: { "bar" => URI::DEFAULT_PARSER.escape("bar\xE2baz".b) }
get :test_skip_parameter_encoding, params: { "bar" => URI::RFC2396_PARSER.escape("bar\xE2baz".b) }

assert_response :success
assert_equal "ASCII-8BIT", @response.body
Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/controller/routing_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2179,11 +2179,11 @@ def test_extras
end

def test_unicode_path
assert_equal({ controller: "news", action: "index" }, @routes.recognize_path(URI::DEFAULT_PARSER.escape("こんにちは/世界"), method: :get))
assert_equal({ controller: "news", action: "index" }, @routes.recognize_path(URI::RFC2396_PARSER.escape("こんにちは/世界"), method: :get))
end

def test_downcased_unicode_path
assert_equal({ controller: "news", action: "index" }, @routes.recognize_path(URI::DEFAULT_PARSER.escape("こんにちは/世界").downcase, method: :get))
assert_equal({ controller: "news", action: "index" }, @routes.recognize_path(URI::RFC2396_PARSER.escape("こんにちは/世界").downcase, method: :get))
end

private
Expand Down
4 changes: 2 additions & 2 deletions actionview/lib/action_view/helpers/url_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -556,14 +556,14 @@ def current_page?(options = nil, check_parameters: false, **options_as_kwargs)

options ||= options_as_kwargs
check_parameters ||= options.is_a?(Hash) && options.delete(:check_parameters)
url_string = URI::DEFAULT_PARSER.unescape(url_for(options)).force_encoding(Encoding::BINARY)
url_string = URI::RFC2396_PARSER.unescape(url_for(options)).force_encoding(Encoding::BINARY)

# We ignore any extra parameters in the request_uri if the
# submitted URL doesn't have any either. This lets the function
# work with things like ?order=asc
# the behavior can be disabled with check_parameters: true
request_uri = url_string.index("?") || check_parameters ? request.fullpath : request.path
request_uri = URI::DEFAULT_PARSER.unescape(request_uri).force_encoding(Encoding::BINARY)
request_uri = URI::RFC2396_PARSER.unescape(request_uri).force_encoding(Encoding::BINARY)

if %r{^\w+://}.match?(url_string)
request_uri = +"#{request.protocol}#{request.host_with_port}#{request_uri}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def to_hash
attr_reader :uri

def uri_parser
@uri_parser ||= URI::Parser.new
@uri_parser ||= URI::RFC2396_Parser.new
end

# Converts the query parameters of the URI into a hash.
Expand Down
1 change: 1 addition & 0 deletions activesupport/activesupport.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,5 @@ Gem::Specification.new do |s|
s.add_dependency "bigdecimal"
s.add_dependency "logger", ">= 1.4.2"
s.add_dependency "securerandom", ">= 0.3"
s.add_dependency "uri", ">= 0.13.1"
end
3 changes: 0 additions & 3 deletions activesupport/lib/active_support/testing/strict_warnings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ class WarningError < StandardError; end
/Ignoring .*\.yml because it has expired/,
/Failed to validate the schema cache because/,

# TODO: Can be removed if https://github.com/ruby/uri/issues/118 is addressed
/URI::RFC3986_PARSER/,

# TODO: We need to decide what to do with this.
/Status code :unprocessable_entity is deprecated/
)
Expand Down
4 changes: 2 additions & 2 deletions railties/lib/rails/info_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def properties

def routes
if query = params[:query]
query = URI::DEFAULT_PARSER.escape query
query = URI::RFC2396_PARSER.escape query

render json: {
exact: matching_routes(query: query, exact_match: true),
Expand Down Expand Up @@ -61,7 +61,7 @@ def matching_routes(query:, exact_match:)
match ||= (query === route_wrapper.verb)

unless match
controller_action = URI::DEFAULT_PARSER.escape(route_wrapper.reqs)
controller_action = URI::RFC2396_PARSER.escape(route_wrapper.reqs)
match = exact_match ? (query === controller_action) : controller_action.include?(query)
end

Expand Down
2 changes: 1 addition & 1 deletion railties/test/application/assets_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ class User < ActiveRecord::Base; raise 'should not be reached'; end
# Load app env
app "development"

get "/assets/#{URI::DEFAULT_PARSER.escape(asset_path)}"
get "/assets/#{URI::RFC2396_PARSER.escape(asset_path)}"
assert_match "not an image really", last_response.body
assert_file_exists("#{app_path}/public/assets/#{asset_path}")
end
Expand Down

0 comments on commit dbf3310

Please sign in to comment.