From b44740d48d78cda609ffa0ecd2d4f45f661c31f9 Mon Sep 17 00:00:00 2001
From: Woody Peterson <woody.peterson@gmail.com>
Date: Wed, 28 Aug 2019 16:52:12 -0700
Subject: [PATCH] Fix route from "(:a)(foo/:b)" when only given :b

Previously, when generating a root path from a route having nested
optional scopes where second-level or greater scopes had a static part,
and when providing only a second-level option, it would result in an
invalid path leading with "//". Ex. a path generated from the route
"(:a)(foo/:b)", given `b: 'bar'`, would return "//foo/bar".

Commit 7670d60977a introduced this bug while fixing the edge case of
empty strings returned when no options are passed to such routes.

A previous commit fixed this for simpler hierarchical cases like
"(:a)(:b)(:c)", and this fixes it for "(:a)(foo/:b)(bar/:c)".
---
 .../lib/action_dispatch/routing/mapper.rb     |  4 +-
 actionpack/test/dispatch/routing_test.rb      | 47 +++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 8cdf7cd283..95947f326d 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -412,7 +412,9 @@ module ActionDispatch
         path.gsub!(%r{/(\(+)/?}, '\1/')
         # if a path is all optional segments, change the leading "(/" back to
         # "/(" so it evaluates to "/" when interpreted with no options.
-        path.sub!(%r{^(\(+)/}, '/\1') if %r{^(\(+[^)]+\)){1,}$}.match?(path)
+        # Unless, however, at least one secondary segment consists of a static
+        # part, ex. "(/:locale)(/pages/:page)"
+        path.sub!(%r{^(\(+)/}, '/\1') if %r{^(\(+[^)]+\))(\(+/:[^)]+\))*$}.match?(path)
         path
       end
 
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 9ca35837a0..623952e794 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -1412,6 +1412,53 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     assert_equal "projects#index", @response.body
   end
 
+  def test_optional_scoped_root_multiple_choice
+    draw do
+      scope "(:locale)" do
+        scope "(p/:platform)" do
+          scope "(b/:browser)" do
+            root to: "projects#index"
+          end
+        end
+      end
+    end
+
+    # Note, in this particular case where we rely on pattern matching instead
+    # of hierarchy to match parameters in a root path, root_path returns ""
+    # when given no path parameters.
+
+    assert_equal "/en", root_path(locale: "en")
+    assert_equal "/p/osx", root_path(platform: "osx")
+    assert_equal "/en/p/osx", root_path(locale: "en", platform: "osx")
+    assert_equal "/b/chrome", root_path(browser: "chrome")
+    assert_equal "/en/b/chrome", root_path(locale: "en", browser: "chrome")
+    assert_equal "/p/osx/b/chrome",
+      root_path(platform: "osx", browser: "chrome")
+    assert_equal "/en/p/osx/b/chrome",
+      root_path(locale: "en", platform: "osx", browser: "chrome")
+
+    get "/en"
+    assert_equal "projects#index", @response.body
+
+    get "/p/osx"
+    assert_equal "projects#index", @response.body
+
+    get "/en/p/osx"
+    assert_equal "projects#index", @response.body
+
+    get "/b/chrome"
+    assert_equal "projects#index", @response.body
+
+    get "/en/b/chrome"
+    assert_equal "projects#index", @response.body
+
+    get "/p/osx/b/chrome"
+    assert_equal "projects#index", @response.body
+
+    get "/en/p/osx/b/chrome"
+    assert_equal "projects#index", @response.body
+  end
+
   def test_scope_with_format_option
     draw do
       get "direct/index", as: :no_format_direct, format: false