Merge pull request #883 from Shopify/flavorjones-ruby-3-0-support

Support for ruby 3.0

.github/workflows/build.yml

@@ -12,17 +12,20 @@ jobs:
     strategy:
       matrix:
         version:
-          - 2.4
-          - 2.5
-          - 2.6
-          - 2.7
+          - "2.4"
+          - "2.5"
+          - "2.6"
+          - "2.7"
+          - "3.0"
         gemfile:
           - Gemfile_ar41
           - Gemfile_ar50
           - Gemfile_ar51
           - Gemfile_ar_master
         exclude:
-          - version: 2.7
+          - version: "2.7"
+            gemfile: Gemfile_ar41
+          - version: "3.0"
             gemfile: Gemfile_ar41
     steps:
       - uses: actions/checkout@v2

Gemfile.lock

@@ -5,6 +5,7 @@ PATH
       activeresource (>= 4.1.0, < 6.0.0)
       graphql-client
       rack
+      webrick
 
 GEM
   remote: https://rubygems.org/
@@ -78,7 +79,7 @@ GEM
       rb-inotify (~> 0.9, >= 0.9.10)
     mercenary (0.4.0)
     method_source (1.0.0)
-    minitest (5.14.1)
+    minitest (5.14.4)
     mocha (1.11.2)
     parallel (1.19.2)
     parser (2.7.2.0)
@@ -129,6 +130,7 @@ GEM
       addressable (>= 2.3.6)
       crack (>= 0.3.2)
       hashdiff (>= 0.4.0, < 2.0.0)
+    webrick (1.7.0)
     zeitwerk (2.3.0)
 
 PLATFORMS
@@ -137,7 +139,7 @@ PLATFORMS
 DEPENDENCIES
   activeresource (~> 5.1)
   jekyll
-  minitest (>= 4.0)
+  minitest (>= 5.14)
   mocha (>= 1.4.0)
   pry
   pry-byebug

dev.yml

@@ -3,7 +3,7 @@ name: shopify-api
 type: ruby
 
 up:
-  - ruby: 2.7.1
+  - ruby: "3.0"
   - bundler
 
 commands:

lib/shopify_api.rb

@@ -21,6 +21,7 @@ require 'shopify_api/metafields'
 require 'shopify_api/countable'
 require 'shopify_api/resources'
 require 'shopify_api/session'
+require 'shopify_api/hmac_params'
 require 'shopify_api/api_access'
 require 'shopify_api/message_enricher'
 require 'shopify_api/connection'

lib/shopify_api/hmac_params.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+require 'webrick/httputils'
+
+module ShopifyAPI
+  module HmacParams
+    extend WEBrick::HTTPUtils
+
+    def self.encode(params)
+      params
+        .except(:signature, :hmac, :action, :controller)
+        .map { |k,v| sprintf("%s=%s", encode_key(k), encode_value(v)) }
+        .sort.join("&")
+    end
+
+    def self.encode_key(key)
+      _escape(key.to_s, _make_regex('&=%'))
+    end
+
+    def self.encode_value(value)
+      _escape(value.to_s, _make_regex('&%'))
+    end
+  end
+end

lib/shopify_api/session.rb

@@ -71,7 +71,7 @@ module ShopifyAPI
         return false unless (signature = params[:hmac])
 
         calculated_signature = OpenSSL::HMAC.hexdigest(
-          OpenSSL::Digest.new('SHA256'), secret, encoded_params_for_signature(params)
+          OpenSSL::Digest.new('SHA256'), secret, ShopifyAPI::HmacParams.encode(params)
         )
 
         Rack::Utils.secure_compare(calculated_signature, signature)
@@ -79,11 +79,6 @@ module ShopifyAPI
 
       private
 
-      def encoded_params_for_signature(params)
-        params = params.except(:signature, :hmac, :action, :controller)
-        params.map { |k, v| "#{URI.escape(k.to_s, '&=%')}=#{URI.escape(v.to_s, '&%')}" }.sort.join('&')
-      end
-
       def extract_current_session
         site = ShopifyAPI::Base.site.to_s
         token = ShopifyAPI::Base.headers['X-Shopify-Access-Token']
@@ -188,7 +183,7 @@ module ShopifyAPI
     end
 
     def parameterize(params)
-      URI.escape(params.collect { |k, v| "#{k}=#{v}" }.join('&'))
+      URI.encode_www_form(params)
     end
 
     def access_token_request(code)

shopify_api.gemspec

@@ -35,10 +35,11 @@ Gem::Specification.new do |s|
   s.add_runtime_dependency("activeresource", ">= 4.1.0", "< 6.0.0")
   s.add_runtime_dependency("rack")
   s.add_runtime_dependency("graphql-client")
+  s.add_runtime_dependency("webrick")
 
   s.add_development_dependency("mocha", ">= 1.4.0")
   s.add_development_dependency("webmock")
-  s.add_development_dependency("minitest", ">= 4.0")
+  s.add_development_dependency("minitest", ">= 5.14")
   s.add_development_dependency("rake")
   s.add_development_dependency("timecop")
   s.add_development_dependency("rubocop-shopify")

test/fulfillment_order_test.rb

@@ -315,7 +315,7 @@ class FulFillmentOrderTest < Test::Unit::TestCase
           fulfillment_order_line_items: [{ id: 1, quantity: 1 }],
           message: "Fulfill this FO, please.",
         }
-        response_fulfillment_orders = fulfillment_order.request_fulfillment(params)
+        response_fulfillment_orders = fulfillment_order.request_fulfillment(**params)
 
         assert_equal('closed', fulfillment_order.status)
         assert_equal(3, response_fulfillment_orders.size)
@@ -367,7 +367,7 @@ class FulFillmentOrderTest < Test::Unit::TestCase
           fulfillment_order_line_items: [{ id: 1, quantity: 1 }],
           message: "Fulfill this FO, please.",
         }
-        response_fulfillment_orders = fulfillment_order.request_fulfillment(params)
+        response_fulfillment_orders = fulfillment_order.request_fulfillment(**params)
 
         assert_equal('closed', fulfillment_order.status)
         assert_equal(3, response_fulfillment_orders.size)

test/hmac_params_test.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+require 'test_helper'
+
+class HmacParamsTest < Test::Unit::TestCase
+  test "cgi param keys are prepared for hmac validation by encoding equals, ampersand, and percent characters" do
+    assert_equal(
+      "abcd%26%3D%251234",
+      ShopifyAPI::HmacParams.encode_key("abcd&=%1234")
+    )
+  end
+
+  test "cgi param values are prepared for hmac validation by encoding ampersand and percent characters" do
+    assert_equal(
+      "abcd%26=%251234",
+      ShopifyAPI::HmacParams.encode_value("abcd&=%1234")
+    )
+  end
+
+  test "cgi params are encoded properly for hmac validation" do
+    assert_equal(
+      "abcd%26%3D%251234=abcd%26=%251234",
+      ShopifyAPI::HmacParams.encode({"abcd&=%1234" => "abcd&=%1234"})
+    )
+  end
+end

test/meta_test.rb

@@ -40,8 +40,8 @@ class ApiVersionTest < Test::Unit::TestCase
         "display_name": "unstable",
         "supported": false,
       },
-    ].to_json
+    ].as_json
 
-    assert_equal versions, ShopifyAPI::Meta.admin_versions.to_json
+    assert_equal versions, ShopifyAPI::Meta.admin_versions.as_json
   end
 end

test/session_test.rb

@@ -94,7 +94,7 @@ class SessionTest < Test::Unit::TestCase
   end
 
   test "ignore everything but the subdomain in the shop" do
-    assert_equal(
+    assert_equal_uri(
       "https://testshop.myshopify.com",
       ShopifyAPI::Session.new(
         domain: "http://user:pass@testshop.notshopify.net/path",
@@ -105,7 +105,7 @@ class SessionTest < Test::Unit::TestCase
   end
 
   test "append the myshopify domain if not given" do
-    assert_equal(
+    assert_equal_uri(
       "https://testshop.myshopify.com",
       ShopifyAPI::Session.new(domain: "testshop", token: "any-token", api_version: any_api_version).site
     )
@@ -283,7 +283,7 @@ class SessionTest < Test::Unit::TestCase
     )
     scope = ["write_products"]
     permission_url = session.create_permission_url(scope, "http://my_redirect_uri.com")
-    assert_equal(
+    assert_equal_uri(
       "https://localhost.myshopify.com/admin/oauth/authorize?client_id=My_test_key&" \
         "scope=write_products&redirect_uri=http://my_redirect_uri.com",
       permission_url
@@ -299,7 +299,7 @@ class SessionTest < Test::Unit::TestCase
     )
     scope = ["write_products", "write_customers"]
     permission_url = session.create_permission_url(scope, "http://my_redirect_uri.com")
-    assert_equal(
+    assert_equal_uri(
       "https://localhost.myshopify.com/admin/oauth/authorize?client_id=My_test_key&" \
         "scope=write_products,write_customers&redirect_uri=http://my_redirect_uri.com",
       permission_url
@@ -315,7 +315,7 @@ class SessionTest < Test::Unit::TestCase
     )
     scope = []
     permission_url = session.create_permission_url(scope, "http://my_redirect_uri.com")
-    assert_equal(
+    assert_equal_uri(
       "https://localhost.myshopify.com/admin/oauth/authorize?client_id=My_test_key&" \
         "scope=&redirect_uri=http://my_redirect_uri.com",
       permission_url
@@ -331,9 +331,9 @@ class SessionTest < Test::Unit::TestCase
     )
     scope = []
     permission_url = session.create_permission_url(scope, "http://my_redirect_uri.com", state: "My nonce")
-    assert_equal(
+    assert_equal_uri(
       "https://localhost.myshopify.com/admin/oauth/authorize?client_id=My_test_key&" \
-        "scope=&redirect_uri=http://my_redirect_uri.com&state=My%20nonce",
+        "scope=&redirect_uri=http://my_redirect_uri.com&state=My+nonce",
       permission_url
     )
   end
@@ -347,7 +347,7 @@ class SessionTest < Test::Unit::TestCase
     )
     scope = []
     permission_url = session.create_permission_url(scope, "http://my_redirect_uri.com", grant_options: "per-user")
-    assert_equal(
+    assert_equal_uri(
       "https://localhost.myshopify.com/admin/oauth/authorize?client_id=My_test_key&" \
         "scope=&redirect_uri=http://my_redirect_uri.com&grant_options[]=per-user",
       permission_url
@@ -380,7 +380,7 @@ class SessionTest < Test::Unit::TestCase
       token: "any-token",
       api_version: any_api_version
     )
-    assert_equal("https://testshop.myshopify.com", session.site)
+    assert_equal_uri("https://testshop.myshopify.com", session.site)
   end
 
   test "return_token_if_signature_is_valid" do
@@ -618,6 +618,10 @@ class SessionTest < Test::Unit::TestCase
 
   private
 
+  def assert_equal_uri(expected, actual)
+    assert_equal(Addressable::URI.parse(expected), Addressable::URI.parse(actual))
+  end
+
   def make_sorted_params(params)
     params.with_indifferent_access.except(
       :signature, :hmac, :action, :controller