Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix routing regex #614

Merged
merged 2 commits into from

2 participants

@zaki

Hi,

I attempted a fix for sinatra/sinatra#611 by making the parsing regex somewhat more robust.

For now the specs seem to pass, but I'm not entirely sure that this is the best way going forward, so please review it and any comments would be appreciated.

I added a few more specs, such as support for lower-case URL-encoded entities according to http://tools.ietf.org/html/rfc3986#section-2.1

Thanks,
Zaki

zaki added some commits
@zaki zaki [FIX] Make route parsing regex more robust
- fixes sinatra/sinatra/#611
- adds support for case-insensitive URL encoding
f079ce1
@zaki zaki [FIX] Support ruby 1.8 in routing regex d983c72
@rkh rkh merged commit 5a346b7 into sinatra:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 17, 2013
  1. @zaki

    [FIX] Make route parsing regex more robust

    zaki authored
    - fixes sinatra/sinatra/#611
    - adds support for case-insensitive URL encoding
  2. @zaki
This page is out of date. Refresh to see the latest.
Showing with 51 additions and 9 deletions.
  1. +30 −2 lib/sinatra/base.rb
  2. +21 −7 test/compile_test.rb
View
32 lib/sinatra/base.rb
@@ -1325,7 +1325,10 @@ def compile(path)
ignore = ""
pattern = path.to_str.gsub(/[^\?\%\\\/\:\*\w]/) do |c|
ignore << escaped(c).join if c.match(/[\.@]/)
- encoded(c)
+ patt = encoded(c)
+ patt.gsub(/%[\da-fA-F]{2}/) do |match|
+ match.split(//).map {|char| char =~ /[A-Z]/ ? "[#{char}#{char.tr('A-Z', 'a-z')}]" : char}.join
+ end
end
pattern.gsub!(/((:\w+)|\*)/) do |match|
if match == "*"
@@ -1333,7 +1336,9 @@ def compile(path)
"(.*?)"
else
keys << $2[1..-1]
- "([^#{ignore}/?#]+)"
+ ignore_pattern = safe_ignore(ignore)
+
+ ignore_pattern
end
end
[/\A#{pattern}\z/, keys]
@@ -1361,6 +1366,29 @@ def escaped(char, enc = URI.escape(char))
[Regexp.escape(enc), URI.escape(char, /./)]
end
+ def safe_ignore(ignore)
+ unsafe_ignore = []
+ ignore = ignore.gsub(/%[\da-fA-F]{2}/) do |hex|
+ unsafe_ignore << hex[1..2]
+ ''
+ end
+ unsafe_patterns = unsafe_ignore.map do |unsafe|
+ chars = unsafe.split(//).map do |char|
+ if char =~ /[A-Z]/
+ char <<= char.tr('A-Z', 'a-z')
+ end
+ char
+ end
+
+ "|(?:%[^#{chars[0]}].|%[#{chars[0]}][^#{chars[1]}])"
+ end
+ if unsafe_patterns.length > 0
+ "((?:[^#{ignore}/?#%]#{unsafe_patterns.join()})+)"
+ else
+ "([^#{ignore}/?#]+)"
+ end
+ end
+
public
# Makes the methods defined in the block and in the Modules given
# in `extensions` available to the handlers and templates
View
28 test/compile_test.rb
@@ -58,7 +58,7 @@ def compiled pattern
fails "/:foo", "/"
fails "/:foo", "/foo/"
- converts "/föö", %r{\A/f%C3%B6%C3%B6\z}
+ converts "/föö", %r{\A/f%[Cc]3%[Bb]6%[Cc]3%[Bb]6\z}
parses "/föö", "/f%C3%B6%C3%B6", {}
converts "/:foo/:bar", %r{\A/([^/?#]+)/([^/?#]+)\z}
@@ -87,7 +87,7 @@ def compiled pattern
converts "/test$/", %r{\A/test(?:\$|%24)/\z}
parses "/test$/", "/test$/", {}
- converts "/te+st/", %r{\A/te(?:\+|%2B)st/\z}
+ converts "/te+st/", %r{\A/te(?:\+|%2[Bb])st/\z}
parses "/te+st/", "/te+st/", {}
fails "/te+st/", "/test/"
fails "/te+st/", "/teeest/"
@@ -95,7 +95,7 @@ def compiled pattern
converts "/test(bar)/", %r{\A/test(?:\(|%28)bar(?:\)|%29)/\z}
parses "/test(bar)/", "/test(bar)/", {}
- converts "/path with spaces", %r{\A/path(?:%20|(?:\+|%2B))with(?:%20|(?:\+|%2B))spaces\z}
+ converts "/path with spaces", %r{\A/path(?:%20|(?:\+|%2[Bb]))with(?:%20|(?:\+|%2[Bb]))spaces\z}
parses "/path with spaces", "/path%20with%20spaces", {}
parses "/path with spaces", "/path%2Bwith%2Bspaces", {}
parses "/path with spaces", "/path+with+spaces", {}
@@ -110,22 +110,22 @@ def compiled pattern
parses "/*/foo/*/*", "/bar/foo/bling/baz/boom", "splat" => ["bar", "bling", "baz/boom"]
fails "/*/foo/*/*", "/bar/foo/baz"
- converts "/test.bar", %r{\A/test(?:\.|%2E)bar\z}
+ converts "/test.bar", %r{\A/test(?:\.|%2[Ee])bar\z}
parses "/test.bar", "/test.bar", {}
fails "/test.bar", "/test0bar"
- converts "/:file.:ext", %r{\A/([^\.%2E/?#]+)(?:\.|%2E)([^\.%2E/?#]+)\z}
+ converts "/:file.:ext", %r{\A/((?:[^\./?#%]|(?:%[^2].|%[2][^Ee]))+)(?:\.|%2[Ee])((?:[^\./?#%]|(?:%[^2].|%[2][^Ee]))+)\z}
parses "/:file.:ext", "/pony.jpg", "file" => "pony", "ext" => "jpg"
parses "/:file.:ext", "/pony%2Ejpg", "file" => "pony", "ext" => "jpg"
fails "/:file.:ext", "/.jpg"
- converts "/:name.?:format?", %r{\A/([^\.%2E/?#]+)(?:\.|%2E)?([^\.%2E/?#]+)?\z}
+ converts "/:name.?:format?", %r{\A/((?:[^\./?#%]|(?:%[^2].|%[2][^Ee]))+)(?:\.|%2[Ee])?((?:[^\./?#%]|(?:%[^2].|%[2][^Ee]))+)?\z}
parses "/:name.?:format?", "/foo", "name" => "foo", "format" => nil
parses "/:name.?:format?", "/foo.bar", "name" => "foo", "format" => "bar"
parses "/:name.?:format?", "/foo%2Ebar", "name" => "foo", "format" => "bar"
fails "/:name.?:format?", "/.bar"
- converts "/:user@?:host?", %r{\A/([^@%40/?#]+)(?:@|%40)?([^@%40/?#]+)?\z}
+ converts "/:user@?:host?", %r{\A/((?:[^@/?#%]|(?:%[^4].|%[4][^0]))+)(?:@|%40)?((?:[^@/?#%]|(?:%[^4].|%[4][^0]))+)?\z}
parses "/:user@?:host?", "/foo@bar", "user" => "foo", "host" => "bar"
parses "/:user@?:host?", "/foo.foo@bar", "user" => "foo.foo", "host" => "bar"
parses "/:user@?:host?", "/foo@bar.bar", "user" => "foo", "host" => "bar.bar"
@@ -136,4 +136,18 @@ def compiled pattern
# parses "/:name(.:format)?", "/foo", "name" => "foo", "format" => nil
# parses "/:name(.:format)?", "/foo.bar", "name" => "foo", "format" => "bar"
fails "/:name(.:format)?", "/foo."
+
+ parses "/:id/test.bar", "/3/test.bar", {"id" => "3"}
+ parses "/:id/test.bar", "/2/test.bar", {"id" => "2"}
+ parses "/:id/test.bar", "/2E/test.bar", {"id" => "2E"}
+ parses "/:id/test.bar", "/2e/test.bar", {"id" => "2e"}
+ fails "/:id/test.bar", "/%2E/test.bar"
+
+ parses "/:file.:ext", "/pony%2ejpg", "file" => "pony", "ext" => "jpg"
+ parses "/:file.:ext", "/pony%E6%AD%A3%2Ejpg", "file" => "pony%E6%AD%A3", "ext" => "jpg"
+ parses "/:file.:ext", "/pony%e6%ad%a3%2ejpg", "file" => "pony%e6%ad%a3", "ext" => "jpg"
+ parses "/:file.:ext", "/pony正%2Ejpg", "file" => "pony正", "ext" => "jpg"
+ parses "/:file.:ext", "/pony正%2ejpg", "file" => "pony正", "ext" => "jpg"
+ fails "/:file.:ext", "/pony正..jpg"
+ fails "/:file.:ext", "/pony正.%2ejpg"
end
Something went wrong with that request. Please try again.