Merge pull request #1672 from iBlackShadow/fix-url-encoding

Fix spaces in a url being encoded as '+' instead of '%20' in lookups that use URL path instead of query string (Mapbox and Bing).
alexreisner · Dec 4, 2024 · ec2f034 · ec2f034
2 parents c1a19f2 + 5c2c6d0
commit ec2f034
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 8 deletions.
diff --git a/lib/geocoder/lookups/bing.rb b/lib/geocoder/lookups/bing.rb
@@ -19,7 +19,7 @@ def required_api_key_parts
     private # ---------------------------------------------------------------
 
     def base_query_url(query)
-      text = CGI.escape(query.sanitized_text.strip)
+      text = ERB::Util.url_encode(query.sanitized_text.strip)
       url = "#{protocol}://dev.virtualearth.net/REST/v1/Locations/"
       if query.reverse_geocode?
         url + "#{text}?"

diff --git a/lib/geocoder/lookups/mapbox.rb b/lib/geocoder/lookups/mapbox.rb
@@ -30,14 +30,14 @@ def query_url_params(query)
     end
 
     def mapbox_search_term(query)
-      require 'cgi' unless defined?(CGI) && defined?(CGI.escape)
+      require 'erb' unless defined?(ERB) && defined?(ERB::Util.url_encode)
       if query.reverse_geocode?
         lat,lon = query.coordinates
-        "#{CGI.escape lon},#{CGI.escape lat}"
+        "#{ERB::Util.url_encode lon},#{ERB::Util.url_encode lat}"
       else
         # truncate at first semicolon so Mapbox doesn't go into batch mode
         # (see Github issue #1299)
-        CGI.escape query.text.to_s.split(';').first.to_s
+        ERB::Util.url_encode query.text.to_s.split(';').first.to_s
       end
     end
 

diff --git a/test/unit/lookups/bing_test.rb b/test/unit/lookups/bing_test.rb
@@ -60,7 +60,7 @@ def test_query_url_escapes_spaces_in_address
       "manchester, lancashire",
       :region => "uk"
     ))
-    assert_match(%r!Locations/uk/\?q=manchester%2C\+lancashire!, url)
+    assert_match(%r!Locations/uk/\?q=manchester%2C%20lancashire!, url)
   end
 
   def test_query_url_strips_trailing_and_leading_spaces
@@ -69,7 +69,7 @@ def test_query_url_strips_trailing_and_leading_spaces
       " manchester, lancashire ",
       :region => "uk"
     ))
-    assert_match(%r!Locations/uk/\?q=manchester%2C\+lancashire!, url)
+    assert_match(%r!Locations/uk/\?q=manchester%2C%20lancashire!, url)
   end
 
   def test_raises_exception_when_service_unavailable

diff --git a/test/unit/lookups/mapbox_test.rb b/test/unit/lookups/mapbox_test.rb
@@ -12,13 +12,13 @@ def setup
   def test_url_contains_api_key
     Geocoder.configure(mapbox: {api_key: "abc123"})
     query = Geocoder::Query.new("Leadville, CO")
-    assert_equal "https://api.mapbox.com/geocoding/v5/mapbox.places/Leadville%2C+CO.json?access_token=abc123", query.url
+    assert_equal "https://api.mapbox.com/geocoding/v5/mapbox.places/Leadville%2C%20CO.json?access_token=abc123", query.url
   end
 
   def test_url_contains_params
     Geocoder.configure(mapbox: {api_key: "abc123"})
     query = Geocoder::Query.new("Leadville, CO", {params: {country: 'CN'}})
-    assert_equal "https://api.mapbox.com/geocoding/v5/mapbox.places/Leadville%2C+CO.json?access_token=abc123&country=CN", query.url
+    assert_equal "https://api.mapbox.com/geocoding/v5/mapbox.places/Leadville%2C%20CO.json?access_token=abc123&country=CN", query.url
   end
 
   def test_result_components