Restore URI.escape

Using CGI.escape was causing issues and regressions with special characters. Ref: #179, #180
enriclluelles · Jan 12, 2018 · 4b104a5 · 4b104a5
1 parent f5c07a8
commit 4b104a5
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 12 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -5,6 +5,9 @@ AllCops:
   Exclude:
     - 'vendor/bundle/**/*'
 
+Lint/UriEscapeUnescape:
+  Enabled: false
+
 Metrics/AbcSize:
   Max: 23.15 # TODO: Lower to 15
   Exclude:

diff --git a/lib/route_translator/translator/path/segment.rb b/lib/route_translator/translator/path/segment.rb
@@ -35,8 +35,7 @@ def translate_string(str, locale, scope)
             sanitized_locale = RouteTranslator::LocaleSanitizer.sanitize(locale)
             translated_resource = translate_resource(str, sanitized_locale, scope)
 
-            # restore URI.escape behaviour to avoid breaking change
-            CGI.escape(translated_resource).gsub('%2F', '/')
+            URI.escape translated_resource
           end
         end
 

diff --git a/test/integration/host_locale_path_verify_consistency_test.rb b/test/integration/host_locale_path_verify_consistency_test.rb
@@ -22,14 +22,14 @@ def test_host_path_consistency
     get '/dummy'
     assert_response :success
 
-    get "/#{CGI.escape('манекен')}"
+    get URI.escape('/манекен')
     assert_response :not_found
 
     host! 'ru.testapp.com'
     get '/dummy'
     assert_response :not_found
 
-    get "/#{CGI.escape('манекен')}"
+    get URI.escape('/манекен')
     assert_response :success
   end
 end
diff --git a/test/integration/host_locales_test.rb b/test/integration/host_locales_test.rb
@@ -38,13 +38,13 @@ def test_explicit_path
 
     ## ru route on es com
     host! 'www.testapp.es'
-    get "/ru/#{CGI.escape('манекен')}"
+    get URI.escape('/ru/манекен')
     assert_equal 'ru', @response.body
     assert_response :success
 
     ## native ru route on ru com
     host! 'ru.testapp.com'
-    get "/#{CGI.escape('манекен')}"
+    get URI.escape('/манекен')
     assert_equal 'ru', @response.body
     assert_response :success
 
@@ -65,7 +65,7 @@ def test_generated_path
     ## native ru route on ru com
     host! 'ru.testapp.com'
     get '/native'
-    assert_equal "/#{CGI.escape('показывать')}", @response.body
+    assert_equal URI.escape('/показывать'), @response.body
     assert_response :success
   end
 

diff --git a/test/routing_test.rb b/test/routing_test.rb
@@ -146,7 +146,7 @@ def test_utf8_characters
       end
     end
 
-    assert_routing "/ru/#{CGI.escape('люди')}", controller: 'people', action: 'index', locale: 'ru'
+    assert_routing URI.escape('/ru/люди'), controller: 'people', action: 'index', locale: 'ru'
   end
 
   def test_resources_with_only
@@ -176,8 +176,8 @@ def test_namespaced_controllers
 
     assert_routing '/gente/fans', controller: 'people/products', action: 'favourites', locale: 'es'
     assert_routing '/favoritos', controller: 'products', action: 'favourites', locale: 'es'
-    assert_routing "/ru/#{CGI.escape('люди')}/#{CGI.escape('кандидаты')}", controller: 'people/products', action: 'favourites', locale: 'ru'
-    assert_routing "/ru/#{CGI.escape('избранное')}", controller: 'products', action: 'favourites', locale: 'ru'
+    assert_routing URI.escape('/ru/люди/кандидаты'), controller: 'people/products', action: 'favourites', locale: 'ru'
+    assert_routing URI.escape('/ru/избранное'), controller: 'products', action: 'favourites', locale: 'ru'
   end
 
   def test_unnamed_root_route_without_prefix
@@ -615,7 +615,7 @@ def test_config_available_locales
       end
     end
 
-    assert_routing "/ru/#{CGI.escape('люди')}", controller: 'people', action: 'index', locale: 'ru'
+    assert_routing URI.escape('/ru/люди'), controller: 'people', action: 'index', locale: 'ru'
     assert_routing '/people', controller: 'people', action: 'index', locale: 'en'
     assert_unrecognized_route '/es/gente', controller: 'people', action: 'index', locale: 'es'
   end
@@ -629,7 +629,7 @@ def test_config_available_locales_handles_strings
       end
     end
 
-    assert_routing "/ru/#{CGI.escape('люди')}", controller: 'people', action: 'index', locale: 'ru'
+    assert_routing URI.escape('/ru/люди'), controller: 'people', action: 'index', locale: 'ru'
     assert_routing '/people', controller: 'people', action: 'index', locale: 'en'
     assert_unrecognized_route '/es/gente', controller: 'people', action: 'index', locale: 'es'
   end