From db3a60eb92707912461c76895492f6b018b63cc5 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 28 Mar 2008 20:01:21 +0000 Subject: Added support for regexp flags like ignoring case in the :requirements part of routes declarations (closes #11421) [NeilW] git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9115 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/CHANGELOG | 2 + actionpack/lib/action_controller/routing.rb | 18 +++ .../lib/action_controller/routing/builder.rb | 9 +- actionpack/lib/action_controller/routing/route.rb | 4 +- .../lib/action_controller/routing/segments.rb | 18 ++- actionpack/test/controller/routing_test.rb | 146 ++++++++++++++++++++- 6 files changed, 190 insertions(+), 7 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 9a72737c37..40921b8526 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Added support for regexp flags like ignoring case in the :requirements part of routes declarations #11421 [NeilW] + * Fixed that ActionController::Base#read_multipart would fail if boundary was exactly 10240 bytes #10886 [ariejan] * Fixed HTML::Tokenizer (used in sanitize helper) didn't handle unclosed CDATA tags #10071 [esad, packagethief] diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb index 4cc5647cd9..3cb8b7b93d 100644 --- a/actionpack/lib/action_controller/routing.rb +++ b/actionpack/lib/action_controller/routing.rb @@ -160,6 +160,24 @@ module ActionController # map.geocode 'geocode/:postalcode', :controller => 'geocode', # :action => 'show', :requirements => { :postalcode => /\d{5}(-\d{4})?/ } # + # Formats can include the 'ignorecase' and 'extended syntax' regular + # expression modifiers: + # + # map.geocode 'geocode/:postalcode', :controller => 'geocode', + # :action => 'show', :postalcode => /hx\d\d\s\d[a-z]{2}/i + # + # map.geocode 'geocode/:postalcode', :controller => 'geocode', + # :action => 'show',:requirements => { + # :postalcode => /# Postcode format + # \d{5} #Prefix + # (-\d{4})? #Suffix + # /x + # } + # + # Using the multiline match modifier will raise an ArgumentError. + # Encoding regular expression modifiers are silently ignored. The + # match will always use the default encoding or ASCII. + # # == Route globbing # # Specifying *[string] as part of a rule like: diff --git a/actionpack/lib/action_controller/routing/builder.rb b/actionpack/lib/action_controller/routing/builder.rb index 69943d4230..50064055f4 100644 --- a/actionpack/lib/action_controller/routing/builder.rb +++ b/actionpack/lib/action_controller/routing/builder.rb @@ -16,6 +16,10 @@ module ActionController Regexp.new "(.*?)(#{separators.source}|$)" end + def multiline_regexp?(expression) + expression.options & Regexp::MULTILINE == Regexp::MULTILINE + end + # Accepts a "route path" (a string defining a route), and returns the array # of segments that corresponds to it. Note that the segment array is only # partially initialized--the defaults and requirements, for instance, need @@ -99,6 +103,9 @@ module ActionController if requirement.source =~ %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} raise ArgumentError, "Regexp anchor characters are not allowed in routing requirements: #{requirement.inspect}" end + if multiline_regexp?(requirement) + raise ArgumentError, "Regexp multiline option not allowed in routing requirements: #{requirement.inspect}" + end segment.regexp = requirement else route_requirements[key] = requirement @@ -194,4 +201,4 @@ module ActionController end end end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/routing/route.rb b/actionpack/lib/action_controller/routing/route.rb index 94134a284e..a83a599e35 100644 --- a/actionpack/lib/action_controller/routing/route.rb +++ b/actionpack/lib/action_controller/routing/route.rb @@ -62,7 +62,7 @@ module ActionController def generation_requirements requirement_conditions = requirements.collect do |key, req| if req.is_a? Regexp - value_regexp = Regexp.new "\\A#{req.source}\\Z" + value_regexp = Regexp.new "\\A#{req.to_s}\\Z" "hash[:#{key}] && #{value_regexp.inspect} =~ options[:#{key}]" else "hash[:#{key}] == #{req.inspect}" @@ -237,4 +237,4 @@ module ActionController end end -end \ No newline at end of file +end diff --git a/actionpack/lib/action_controller/routing/segments.rb b/actionpack/lib/action_controller/routing/segments.rb index efac2df1cb..24ea8c7f2d 100644 --- a/actionpack/lib/action_controller/routing/segments.rb +++ b/actionpack/lib/action_controller/routing/segments.rb @@ -171,10 +171,19 @@ module ActionController end def value_regexp - Regexp.new "\\A#{regexp.source}\\Z" if regexp + Regexp.new "\\A#{regexp.to_s}\\Z" if regexp end + def regexp_chunk - regexp ? "(#{regexp.source})" : "([^#{Routing::SEPARATORS.join}]+)" + if regexp + if regexp_has_modifiers? + "(#{regexp.to_s})" + else + "(#{regexp.source})" + end + else + "([^#{Routing::SEPARATORS.join}]+)" + end end def build_pattern(pattern) @@ -183,6 +192,7 @@ module ActionController pattern = "#{chunk}#{pattern}" optional? ? Regexp.optionalize(pattern) : pattern end + def match_extraction(next_capture) # All non code-related keys (such as :id, :slug) are URI-unescaped as # path parameters. @@ -201,6 +211,10 @@ module ActionController [:action, :id].include? key end + def regexp_has_modifiers? + regexp.options & (Regexp::IGNORECASE | Regexp::EXTENDED) != 0 + end + end class ControllerSegment < DynamicSegment #:nodoc: diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index bd6fae6e0b..5756c05e2e 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -936,6 +936,16 @@ class DynamicSegmentTest < Test::Unit::TestCase a_segment.key = :action assert a_segment.optionality_implied? end + + def test_modifiers_must_be_handled_sensibly + a_segment = ROUTING::DynamicSegment.new + a_segment.regexp = /david|jamis/i + assert_equal "((?i-mx:david|jamis))stuff", a_segment.build_pattern('stuff') + a_segment.regexp = /david|jamis/x + assert_equal "((?x-mi:david|jamis))stuff", a_segment.build_pattern('stuff') + a_segment.regexp = /david|jamis/ + assert_equal "(david|jamis)stuff", a_segment.build_pattern('stuff') + end end class ControllerSegmentTest < Test::Unit::TestCase @@ -1723,7 +1733,7 @@ class RouteSetTest < Test::Unit::TestCase end end end - + def test_non_path_route_requirements_match_all set.draw do |map| map.connect 'page/37s', :controller => 'pages', :action => 'show', :name => /(jamis|david)/ @@ -2110,6 +2120,138 @@ class RouteSetTest < Test::Unit::TestCase end end end + + def test_route_requirements_with_unsupported_regexp_options_must_error + assert_raises ArgumentError do + set.draw do |map| + map.connect 'page/:name', :controller => 'pages', + :action => 'show', + :requirements => {:name => /(david|jamis)/m} + end + end + end + + def test_route_requirements_with_supported_options_must_not_error + assert_nothing_raised do + set.draw do |map| + map.connect 'page/:name', :controller => 'pages', + :action => 'show', + :requirements => {:name => /(david|jamis)/i} + end + end + assert_nothing_raised do + set.draw do |map| + map.connect 'page/:name', :controller => 'pages', + :action => 'show', + :requirements => {:name => / # Desperately overcommented regexp + ( #Either + david #The Creator + | #Or + jamis #The Deployer + )/x} + end + end + end + + def test_route_requirement_recognize_with_ignore_case + set.draw do |map| + map.connect 'page/:name', :controller => 'pages', + :action => 'show', + :requirements => {:name => /(david|jamis)/i} + end + assert_equal({:controller => 'pages', :action => 'show', :name => 'jamis'}, set.recognize_path('/page/jamis')) + assert_raises ActionController::RoutingError do + set.recognize_path('/page/davidjamis') + end + assert_equal({:controller => 'pages', :action => 'show', :name => 'DAVID'}, set.recognize_path('/page/DAVID')) + end + + def test_route_requirement_generate_with_ignore_case + set.draw do |map| + map.connect 'page/:name', :controller => 'pages', + :action => 'show', + :requirements => {:name => /(david|jamis)/i} + end + url = set.generate({:controller => 'pages', :action => 'show', :name => 'david'}) + assert_equal "/page/david", url + assert_raises ActionController::RoutingError do + url = set.generate({:controller => 'pages', :action => 'show', :name => 'davidjamis'}) + end + url = set.generate({:controller => 'pages', :action => 'show', :name => 'JAMIS'}) + assert_equal "/page/JAMIS", url + end + + def test_route_requirement_recognize_with_extended_syntax + set.draw do |map| + map.connect 'page/:name', :controller => 'pages', + :action => 'show', + :requirements => {:name => / # Desperately overcommented regexp + ( #Either + david #The Creator + | #Or + jamis #The Deployer + )/x} + end + assert_equal({:controller => 'pages', :action => 'show', :name => 'jamis'}, set.recognize_path('/page/jamis')) + assert_equal({:controller => 'pages', :action => 'show', :name => 'david'}, set.recognize_path('/page/david')) + assert_raises ActionController::RoutingError do + set.recognize_path('/page/david #The Creator') + end + assert_raises ActionController::RoutingError do + set.recognize_path('/page/David') + end + end + + def test_route_requirement_generate_with_extended_syntax + set.draw do |map| + map.connect 'page/:name', :controller => 'pages', + :action => 'show', + :requirements => {:name => / # Desperately overcommented regexp + ( #Either + david #The Creator + | #Or + jamis #The Deployer + )/x} + end + url = set.generate({:controller => 'pages', :action => 'show', :name => 'david'}) + assert_equal "/page/david", url + assert_raises ActionController::RoutingError do + url = set.generate({:controller => 'pages', :action => 'show', :name => 'davidjamis'}) + end + assert_raises ActionController::RoutingError do + url = set.generate({:controller => 'pages', :action => 'show', :name => 'JAMIS'}) + end + end + + def test_route_requirement_generate_with_xi_modifiers + set.draw do |map| + map.connect 'page/:name', :controller => 'pages', + :action => 'show', + :requirements => {:name => / # Desperately overcommented regexp + ( #Either + david #The Creator + | #Or + jamis #The Deployer + )/xi} + end + url = set.generate({:controller => 'pages', :action => 'show', :name => 'JAMIS'}) + assert_equal "/page/JAMIS", url + end + + def test_route_requirement_recognize_with_xi_modifiers + set.draw do |map| + map.connect 'page/:name', :controller => 'pages', + :action => 'show', + :requirements => {:name => / # Desperately overcommented regexp + ( #Either + david #The Creator + | #Or + jamis #The Deployer + )/xi} + end + assert_equal({:controller => 'pages', :action => 'show', :name => 'JAMIS'}, set.recognize_path('/page/JAMIS')) + end + end @@ -2190,7 +2332,7 @@ class RoutingTest < Test::Unit::TestCase ActionController::Routing::Routes.install_helpers c assert c.ancestors.include?(h) end - + end uses_mocha 'route loading' do -- cgit v1.2.3