determine the match shorthand target early.

Backport #9361.
Closes #7554.

This patch determines the `controller#action` directly
in the `match` method when the shorthand syntax is used.
this prevents problems with namespaces and scopes.

3 files changed, 54 insertions, 9 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 248e6f2b04..b71d004033 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,20 @@
 ## unreleased ##
 
+*   Determine the controller#action from only the matched path when using the
+    shorthand syntax. Previously the complete path was used, which led
+    to problems with nesting (scopes and namespaces).
+    Fixes #7554.
+    Backport #9361.
+
+    Example:
+
+        # this will route to questions#new
+        scope ':locale' do
+          get 'questions/new'
+        end
+
+    *Yves Senn*
+
 *   Fix `assert_template` with `render :stream => true`.
     Fix #1743.
     Backport #5288.
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 0b71c2ea5c..6b272704ea 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -51,7 +51,6 @@ module ActionDispatch
       class Mapping #:nodoc:
         IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix]
         ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z}
-        SHORTHAND_REGEX = %r{/[\w/]+$}
         WILDCARD_PATH = %r{\*([^/\)]+)\)?$}
 
         def initialize(set, scope, path, options)
@@ -70,12 +69,7 @@ module ActionDispatch
           def normalize_options!
             path_without_format = @path.sub(/\(\.:format\)$/, '')
 
-            if using_match_shorthand?(path_without_format, @options)
-              to_shorthand    = @options[:to].blank?
-              @options[:to] ||= path_without_format.gsub(/\(.*\)/, "")[1..-1].sub(%r{/([^/]*)$}, '#\1')
-            end
-
-            @options.merge!(default_controller_and_action(to_shorthand))
+            @options.merge!(default_controller_and_action)
 
             requirements.each do |name, requirement|
               # segment_keys.include?(k.to_s) || k == :controller
@@ -153,7 +147,7 @@ module ActionDispatch
             end
           end
 
-          def default_controller_and_action(to_shorthand=nil)
+          def default_controller_and_action
             if to.respond_to?(:call)
               { }
             else
@@ -166,7 +160,7 @@ module ActionDispatch
               controller ||= default_controller
               action     ||= default_action
 
-              unless controller.is_a?(Regexp) || to_shorthand
+              unless controller.is_a?(Regexp)
                 controller = [@scope[:module], controller].compact.join("/").presence
               end
 
@@ -1261,6 +1255,11 @@ module ActionDispatch
             paths = [path] + rest
           end
 
+          path_without_format = path.to_s.sub(/\(\.:format\)$/, '')
+          if using_match_shorthand?(path_without_format, options)
+            options[:to] ||= path_without_format.gsub(%r{^/}, "").sub(%r{/([^/]*)$}, '#\1')
+          end
+
           options[:anchor] = true unless options.key?(:anchor)
 
           if options[:on] && !VALID_ON_OPTIONS.include?(options[:on])
@@ -1271,6 +1270,10 @@ module ActionDispatch
           self
         end
 
+        def using_match_shorthand?(path, options)
+          path && (options[:to] || options[:action]).nil? && path =~ %r{/[\w/]+$}
+        end
+
         def decomposed_match(path, options) # :nodoc:
           if on = options.delete(:on)
             send(on) { decomposed_match(path, options) }
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 6ecf011694..b922235944 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -517,6 +517,18 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
       end
 
       get 'search' => 'search'
+
+      scope ':locale' do
+        match 'questions/new', via: [:get]
+      end
+
+      namespace :api do
+        namespace :v3 do
+          scope ':locale' do
+            get "products/list"
+          end
+        end
+      end
     end
   end
 
@@ -1417,6 +1429,21 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_match_shorthand_inside_scope_with_variables_with_controller
+    with_test_routes do
+      get '/de/questions/new'
+      assert_equal 'questions#new', @response.body
+      assert_equal 'de', @request.params[:locale]
+    end
+  end
+
+  def test_match_shorthand_inside_nested_namespaces_and_scopes_with_controller
+    with_test_routes do
+      get '/api/v3/en/products/list'
+      assert_equal 'api/v3/products#list', @response.body
+    end
+  end
+
   def test_dynamically_generated_helpers_on_collection_do_not_clobber_resources_url_helper
     with_test_routes do
       assert_equal '/replies', replies_path