From 3b57b0f00e26a1d8533bcd9389a93be56cb5a4da Mon Sep 17 00:00:00 2001 From: Jessica Dussault Date: Wed, 13 Dec 2017 09:23:58 -0600 Subject: [PATCH 1/3] hack to get semicolon through to API --- lib/api_bridge/helpers.rb | 12 ++++++++---- lib/api_bridge/query.rb | 4 +--- lib/api_bridge/version.rb | 2 +- test/helper_test.rb | 13 ++++++------- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/lib/api_bridge/helpers.rb b/lib/api_bridge/helpers.rb index 8faf111..cf1515e 100644 --- a/lib/api_bridge/helpers.rb +++ b/lib/api_bridge/helpers.rb @@ -2,10 +2,6 @@ module ApiBridge - def self.encode_html string - return URI.encode(string) - end - def self.calculate_page count, rows if count && rows return (count.to_f/rows.to_i).ceil @@ -14,6 +10,14 @@ def self.calculate_page count, rows end end + def self.encode url + encoded = URI.encode(url) + # semicolons are not caught in the above step + encoded = encoded.gsub(";", "%3B") + # encoded semicolons may accidentally have had the % double encoded + encoded.gsub("%253B", "%3B") + end + def self.escape_values options # TODO what kind of escaping or sanitizing do we # want to do here? diff --git a/lib/api_bridge/query.rb b/lib/api_bridge/query.rb index 7887e94..65f9b6c 100644 --- a/lib/api_bridge/query.rb +++ b/lib/api_bridge/query.rb @@ -51,9 +51,7 @@ def create_items_url options={} end query_string = query.join("&") url = "#{@url}/items?#{query_string}" - # TODO apparently things URI.encode uses are deprecated - # even though their docs don't mention it guhhhh - encoded = URI.encode(url) + encoded = ApiBridge.encode(url) if ApiBridge.is_url?(encoded) return encoded else diff --git a/lib/api_bridge/version.rb b/lib/api_bridge/version.rb index 0b5ff28..0b6b440 100644 --- a/lib/api_bridge/version.rb +++ b/lib/api_bridge/version.rb @@ -1,5 +1,5 @@ module ApiBridge - VERSION = "0.0.2" + VERSION = "0.0.3" def self.version VERSION diff --git a/test/helper_test.rb b/test/helper_test.rb index d801c19..427f93f 100644 --- a/test/helper_test.rb +++ b/test/helper_test.rb @@ -3,13 +3,6 @@ class HelperTest < Minitest::Test - def test_encode_html - # TODO look into why the ampersand and quotation marks are not encoded? - unencoded = "this is a 'string' & some stuff" - encoded = ApiBridge.encode_html(unencoded) - assert_equal encoded, "this%20is%20a%20'string'%20&%20some%20stuff" - end - def test_calculate_page assert_equal ApiBridge.calculate_page(101, 20), 6 assert_equal ApiBridge.calculate_page(1, 20), 1 @@ -17,6 +10,12 @@ def test_calculate_page assert_equal ApiBridge.calculate_page(57, 5), 12 end + def test_encode + assert_equal "Cather%3B%20Pound", ApiBridge.encode("Cather; Pound") + assert_equal "Cather,%20Pound", ApiBridge.encode("Cather, Pound") + assert_equal "Cather%3B%20Pound", ApiBridge.encode("Cather%3B Pound") + end + def test_escape_values # pending end From 6db6c3413f43c759892a687cc41aac80f7ddc1e8 Mon Sep 17 00:00:00 2001 From: Greg Tunink Date: Fri, 15 Dec 2017 15:12:15 -0600 Subject: [PATCH 2/3] Replace obsolete URI.encode; Update tests Supported encoding methods encode far more characters Split URL passed into individual components Encode only param values Skip encoding param names because the [] encoding breaks API requests Must pass a full URL in tests to use method which splits components Update tests to reflect different encoding output - Spaces encode as '+', commas as '%2C' Remove test for pre-encoded semi-colons Update assert_equal argument order for modified tests --- lib/api_bridge/helpers.rb | 26 +++++++++++++++++++++----- test/helper_test.rb | 9 ++++++--- test/query_test.rb | 10 +++++----- 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/lib/api_bridge/helpers.rb b/lib/api_bridge/helpers.rb index cf1515e..cf4db01 100644 --- a/lib/api_bridge/helpers.rb +++ b/lib/api_bridge/helpers.rb @@ -11,11 +11,17 @@ def self.calculate_page count, rows end def self.encode url - encoded = URI.encode(url) - # semicolons are not caught in the above step - encoded = encoded.gsub(";", "%3B") - # encoded semicolons may accidentally have had the % double encoded - encoded.gsub("%253B", "%3B") + # Split components of URL so we may encode only the query string, index 7 + components = URI.split(url).map.with_index { |component, i| + i != 7 || component == "" ? component : encode_query_params(component) + } + + # Assume URLs are hierarchical, i.e. (scheme)://... not opaque (scheme):... + url = "#{components[0]}://" << components[1..5].join("") + url << "?#{components[7]}" if components[7] != "" + url << components[8] if components[8] + + url end def self.escape_values options @@ -66,4 +72,14 @@ def self.set_page page end return new_page end + + private + + def self.encode_query_params(query_string) + query_string.split(/[&]/).map { |param| + name, value = param.split("=") + + "#{name}=#{URI.encode_www_form_component(value)}" + }.join("&") + end end diff --git a/test/helper_test.rb b/test/helper_test.rb index 427f93f..85e0791 100644 --- a/test/helper_test.rb +++ b/test/helper_test.rb @@ -11,9 +11,12 @@ def test_calculate_page end def test_encode - assert_equal "Cather%3B%20Pound", ApiBridge.encode("Cather; Pound") - assert_equal "Cather,%20Pound", ApiBridge.encode("Cather, Pound") - assert_equal "Cather%3B%20Pound", ApiBridge.encode("Cather%3B Pound") + fake_url = "http://something.unl.edu/?param=" + + assert_equal "#{fake_url}Cather%3B+Pound", + ApiBridge.encode("#{fake_url}Cather; Pound") + assert_equal "#{fake_url}Cather%2C+Pound", + ApiBridge.encode("#{fake_url}Cather, Pound") end def test_escape_values diff --git a/test/query_test.rb b/test/query_test.rb index 8805ae1..d2dc8a7 100644 --- a/test/query_test.rb +++ b/test/query_test.rb @@ -16,12 +16,12 @@ def test_create_item_url def test_create_items_url # basic url = @api.create_items_url() - assert_equal url, "#{@fake_url}/items?" + assert_equal url, "#{@fake_url}/items" # with f[] and q opts = { "f" => ["thing1", "thing2"], "q" => "water" } url = @api.create_items_url(opts) - assert_equal url, "#{@fake_url}/items?f[]=thing1&f[]=thing2&q=water" + assert_equal "#{@fake_url}/items?f[]=thing1&f[]=thing2&q=water", url # with f[], facet[], start, and num opts = { @@ -30,17 +30,17 @@ def test_create_items_url "start" => 21, "num" => 20 } url = @api.create_items_url(opts) - assert_equal url, "#{@fake_url}/items?f[]=subcategory%7Cworks&facet[]=title&facet[]=name&start=21&num=20" + assert_equal "#{@fake_url}/items?f[]=subcategory%7Cworks&facet[]=title&facet[]=name&start=21&num=20", url # with nested f[] opts = { "f" => ["creator.name|Willa, Cather"] } url = @api.create_items_url(opts) - assert_equal url, "#{@fake_url}/items?f[]=creator.name%7CWilla,%20Cather" + assert_equal "#{@fake_url}/items?f[]=creator.name%7CWilla%2C+Cather", url # with multiple sorts opts = { "sort" => ["title|asc", "name|desc"] } url = @api.create_items_url(opts) - assert_equal url, "#{@fake_url}/items?sort[]=title%7Casc&sort[]=name%7Cdesc" + assert_equal "#{@fake_url}/items?sort[]=title%7Casc&sort[]=name%7Cdesc", url end def test_reset_options From 5c9b42b87c365a87242d1782b258350c172b2749 Mon Sep 17 00:00:00 2001 From: Jessica Dussault Date: Mon, 18 Dec 2017 11:04:40 -0600 Subject: [PATCH 3/3] simplifies url encoding and adds a few tests --- lib/api_bridge/helpers.rb | 18 +++++++----------- test/helper_test.rb | 28 ++++++++++++++++++++++------ 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/lib/api_bridge/helpers.rb b/lib/api_bridge/helpers.rb index cf4db01..dd4bf18 100644 --- a/lib/api_bridge/helpers.rb +++ b/lib/api_bridge/helpers.rb @@ -11,17 +11,13 @@ def self.calculate_page count, rows end def self.encode url - # Split components of URL so we may encode only the query string, index 7 - components = URI.split(url).map.with_index { |component, i| - i != 7 || component == "" ? component : encode_query_params(component) - } - - # Assume URLs are hierarchical, i.e. (scheme)://... not opaque (scheme):... - url = "#{components[0]}://" << components[1..5].join("") - url << "?#{components[7]}" if components[7] != "" - url << components[8] if components[8] - - url + # Split components of URL so we may encode only the query string + request_url, query = url.split("?") + if query + encoded = encode_query_params(query) + request_url << "?#{encoded}" + end + request_url end def self.escape_values options diff --git a/test/helper_test.rb b/test/helper_test.rb index 85e0791..e5164ac 100644 --- a/test/helper_test.rb +++ b/test/helper_test.rb @@ -11,12 +11,28 @@ def test_calculate_page end def test_encode - fake_url = "http://something.unl.edu/?param=" + fake_url = "http://something.unl.edu/" + + # semicolons + assert_equal "#{fake_url}?param=Cather%3B+Pound", + ApiBridge.encode("#{fake_url}?param=Cather; Pound") + assert_equal "#{fake_url}?param=Cather%2C+Pound", + ApiBridge.encode("#{fake_url}?param=Cather, Pound") + # multiple types of characters + assert_equal "#{fake_url}?f[]=works%7CTitle+%282012%29+Author", + ApiBridge.encode("#{fake_url}?f[]=works|Title (2012) Author") + # multiple parameters + query_in = "f[]=works|Title (1827)&f[]=works|Title2, Author; Author2&q=*" + query_out = "f[]=works%7CTitle+%281827%29&f[]=works%7CTitle2%2C+Author%3B+Author2&q=*" + assert_equal "#{fake_url}?#{query_out}", + ApiBridge.encode("#{fake_url}?#{query_in}") + # no parameters + assert_equal fake_url, ApiBridge.encode(fake_url) + end - assert_equal "#{fake_url}Cather%3B+Pound", - ApiBridge.encode("#{fake_url}Cather; Pound") - assert_equal "#{fake_url}Cather%2C+Pound", - ApiBridge.encode("#{fake_url}Cather, Pound") + def test_encode_query_params + assert_equal "authors=Cather%3B+Pound&f[]=works%7CSomething", + ApiBridge.encode_query_params("authors=Cather; Pound&f[]=works|Something") end def test_escape_values @@ -39,7 +55,7 @@ def test_is_url? end def test_override_options - + # TODO end end