diff options
author | Andrew White <andrew.white@unboxed.co> | 2017-04-18 11:02:05 +0100 |
---|---|---|
committer | Andrew White <andrew.white@unboxed.co> | 2017-04-18 11:02:05 +0100 |
commit | 8776a7139757d0b264785c774d4e7f37d4bc1ac7 (patch) | |
tree | 63f4912b39ad1c9a11fa65576cfbea5470884bd4 /actionpack | |
parent | 0d208e02f6b90fd3d61da60e58854b9fdd8eeb1d (diff) | |
download | rails-8776a7139757d0b264785c774d4e7f37d4bc1ac7.tar.gz rails-8776a7139757d0b264785c774d4e7f37d4bc1ac7.tar.bz2 rails-8776a7139757d0b264785c774d4e7f37d4bc1ac7.zip |
Use more specific check for :format in route path
The current check for whether to add an optional format to the path
is very lax and will match things like `:format_id` where there are
nested resources, e.g:
resources :formats do
resources :items
end
Fix this by using a more restrictive regex pattern that looks for
the patterns `(.:format)`, `.:format` or `/` at the end of the path.
Note that we need to allow for multiple closing parenthesis since
the route may be of this form:
get "/books(/:action(.:format))", controller: "books"
This probably isn't what's intended since it means that the default
index action route doesn't support a format but we have a test for
it so we need to allow it.
Fixes #28517.
Diffstat (limited to 'actionpack')
-rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 3 | ||||
-rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 18 |
2 files changed, 20 insertions, 1 deletions
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 8ad17504ae..74904e3d45 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -54,6 +54,7 @@ module ActionDispatch class Mapping #:nodoc: ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} + OPTIONAL_FORMAT_REGEX = %r{(?:\(\.:format\)+|\.:format|/)\Z} attr_reader :requirements, :defaults attr_reader :to, :default_controller, :default_action @@ -93,7 +94,7 @@ module ActionDispatch end def self.optional_format?(path, format) - format != false && !path.include?(":format") && !path.end_with?("/") + format != false && path !~ OPTIONAL_FORMAT_REGEX end def initialize(set, ast, defaults, controller, default_action, modyoule, to, formatted, scope_constraints, blocks, via, options_constraints, anchor, options) diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 64818e6ca1..fdc47743fa 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3706,6 +3706,24 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal "/bar", bar_root_path end + def test_nested_routes_under_format_resource + draw do + resources :formats do + resources :items + end + end + + get "/formats/1/items.json" + assert_equal 200, @response.status + assert_equal "items#index", @response.body + assert_equal "/formats/1/items.json", format_items_path(1, :json) + + get "/formats/1/items/2.json" + assert_equal 200, @response.status + assert_equal "items#show", @response.body + assert_equal "/formats/1/items/2.json", format_item_path(1, 2, :json) + end + private def draw(&block) |