Skip to content

Commit dfc5e80

Browse files
committed
Get rid of Jounry::Utils.unescape
The `cgi` default gem provide the same functionality but much more optimized. Might as well just use that. We can also pre check if the string need escaping, assuming most don't, we save an allocation.
1 parent afcf3fc commit dfc5e80

File tree

3 files changed

+13
-20
lines changed

3 files changed

+13
-20
lines changed

actionpack/lib/action_dispatch/journey/router.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
# :markup: markdown
44

5+
require "cgi"
56
require "action_dispatch/journey/router/utils"
67
require "action_dispatch/journey/routes"
78
require "action_dispatch/journey/formatter"
@@ -123,8 +124,13 @@ def find_routes(req)
123124
path_parameters = {}
124125
index = 1
125126
match_data.names.each do |name|
126-
val = match_data[index]
127-
path_parameters[name.to_sym] = Utils.unescape_uri(val) if val
127+
if val = match_data[index]
128+
path_parameters[name.to_sym] = if val.include?("%")
129+
CGI.unescapeURIComponent(val)
130+
else
131+
val
132+
end
133+
end
128134
index += 1
129135
end
130136
yield [match_data, path_parameters, r]

actionpack/lib/action_dispatch/journey/router/utils.rb

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,6 @@ def escape_segment(segment)
6161
escape(segment, SEGMENT)
6262
end
6363

64-
def unescape_uri(uri)
65-
encoding = uri.encoding == US_ASCII ? UTF_8 : uri.encoding
66-
uri.gsub(ESCAPED) { |match| [match[1, 2].hex].pack("C") }.force_encoding(encoding)
67-
end
68-
6964
private
7065
def escape(component, pattern)
7166
component.gsub(pattern) { |unsafe| percent_encode(unsafe) }.force_encoding(US_ASCII)
@@ -91,14 +86,6 @@ def self.escape_segment(segment)
9186
def self.escape_fragment(fragment)
9287
ENCODER.escape_fragment(fragment.to_s)
9388
end
94-
95-
# Replaces any escaped sequences with their unescaped representations.
96-
#
97-
# uri = "/topics?title=Ruby%20on%20Rails"
98-
# unescape_uri(uri) #=> "/topics?title=Ruby on Rails"
99-
def self.unescape_uri(uri)
100-
ENCODER.unescape_uri(uri)
101-
end
10289
end
10390
end
10491
end

actionpack/test/journey/router/utils_test.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ def test_fragment_escape
1818
assert_equal "a/b%20c+d%25?e", Utils.escape_fragment("a/b c+d%?e")
1919
end
2020

21-
def test_uri_unescape
22-
assert_equal "a/b c+d", Utils.unescape_uri("a%2Fb%20c+d")
21+
def test_CGI_unescapeURIComponent
22+
assert_equal "a/b c+d", CGI.unescapeURIComponent("a%2Fb%20c+d")
2323
end
2424

25-
def test_uri_unescape_with_utf8_string
26-
assert_equal "Šašinková", Utils.unescape_uri((+"%C5%A0a%C5%A1inkov%C3%A1").force_encoding(Encoding::US_ASCII))
25+
def test_CGI_unescapeURIComponent_with_utf8_string
26+
assert_equal "Šašinková", CGI.unescapeURIComponent((+"%C5%A0a%C5%A1inkov%C3%A1").force_encoding(Encoding::US_ASCII))
2727
end
2828

2929
def test_normalize_path_not_greedy
@@ -36,7 +36,7 @@ def test_normalize_path_uppercase
3636

3737
def test_normalize_path_maintains_string_encoding
3838
path = "/foo%AAbar%AAbaz".b
39-
assert_equal Encoding::ASCII_8BIT, Utils.normalize_path(path).encoding
39+
assert_equal Encoding::BINARY, Utils.normalize_path(path).encoding
4040
end
4141

4242
def test_normalize_path_with_nil

0 commit comments

Comments
 (0)