Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Matching spaces with regex routes are broken in 1.3.1. This is a fix. #390

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions lib/sinatra/base.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -798,10 +798,14 @@ def route_eval
# Returns pass block. # Returns pass block.
def process_route(pattern, keys, conditions, block = nil, values = []) def process_route(pattern, keys, conditions, block = nil, values = [])
@original_params ||= @params @original_params ||= @params
route = @request.path_info
# The %2F shuffle seems ugly. But '/' just happens to be our parameter separator.
# It deserves the extra treatment.
route = Rack::Utils.unescape @request.path_info.gsub('%2F', '%252F')

route = '/' if route.empty? and not settings.empty_path_info? route = '/' if route.empty? and not settings.empty_path_info?
if match = pattern.match(route) if match = pattern.match(route)
values += match.captures.to_a.map { |v| force_encoding URI.decode(v) if v } values += match.captures.to_a.map { |v| force_encoding v.gsub('%2F','/') if v }
params = params =
if keys.any? if keys.any?
keys.zip(values).inject({}) do |hash,(k,v)| keys.zip(values).inject({}) do |hash,(k,v)|
Expand Down Expand Up @@ -1218,7 +1222,7 @@ def compile!(verb, path, block, options = {})
def compile(path) def compile(path)
keys = [] keys = []
if path.respond_to? :to_str if path.respond_to? :to_str
pattern = path.to_str.gsub(/[^\?\%\\\/\:\*\w]/) { |c| encoded(c) } pattern = path.to_str.gsub(/[^\?\%\\\/\:\*\w\ ]/) { |c| "(?:#{Regexp.escape c})" }
pattern.gsub!(/((:\w+)|\*)/) do |match| pattern.gsub!(/((:\w+)|\*)/) do |match|
if match == "*" if match == "*"
keys << 'splat' keys << 'splat'
Expand All @@ -1240,13 +1244,6 @@ def compile(path)
end end
end end


def encoded(char)
enc = URI.encode(char)
enc = "(?:#{Regexp.escape enc}|#{URI.encode char, /./})" if enc == char
enc = "(?:#{enc}|#{encoded('+')})" if char == " "
enc
end

public public
# Makes the methods defined in the block and in the Modules given # Makes the methods defined in the block and in the Modules given
# in `extensions` available to the handlers and templates # in `extensions` available to the handlers and templates
Expand Down
37 changes: 37 additions & 0 deletions test/routing_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -459,6 +459,43 @@ class RoutingTest < Test::Unit::TestCase
assert_equal 'Hello World', body assert_equal 'Hello World', body
end end


it "URL works with spaces within regexps" do
mock_app {
get %r{\A/regexp_space/([\w ]+)} do
assert_equal ['how are you'], params['captures']
nil
end
}

get '/regexp_space/how%20are%20you'
assert ok?
end

# pending:
# it "URL works with slashes within regexps" do
# mock_app {
# get %r{\A/regexp_slash/([\w\/]+)} do
# assert_equal ['how/are/you'], params['captures']
# nil
# end
# }
#
# get '/regexp_slash/how%2Fare%2Fyou'
# assert ok?
# end

it "URL works with umlauts within regexps" do
mock_app {
get %r{\A/regexp_umlaut/([äöü])}u do # the 'u' is only needed for ruby 1.8.7
assert_equal ['ö'], params['captures']
nil
end
}

get '/regexp_umlaut/%C3%B6'
assert ok?
end

it 'makes regular expression captures available in params[:captures]' do it 'makes regular expression captures available in params[:captures]' do
mock_app { mock_app {
get(/^\/fo(.*)\/ba(.*)/) do get(/^\/fo(.*)\/ba(.*)/) do
Expand Down