Refactored query string building to account for array handling (#269)

* Refactored query string building to account for array handling

* Updated changelog

Author: Jan0707

CHANGELOG.md

@@ -7,6 +7,7 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
 
+- [#269](https://github.com/Shopify/shopify-api-php/pull/269) [bugfix] Refactored query string building to account for Shopify-specific array format
 - [#236](https://github.com/Shopify/shopify-api-php/pull/236) [bugfix] Correct requirements in `composer.json`
 - [#262](https://github.com/Shopify/shopify-api-php/pull/262) ⚠️ [Breaking] Added support for PHP 8.2, and removed support for PHP 7.4
 - [#264](https://github.com/Shopify/shopify-api-php/pull/264) [Patch] Remove support for currently-non-existent versions of PHP (8.3+)

src/Utils.php

@@ -60,6 +60,42 @@ public static function sanitizeShopDomain(string $shop, ?string $myshopifyDomain
         }
     }
 
+    /**
+     * Builds query strings that are compatible with Shopify's format for array handling
+     * Example: IDs = [1,2,3]
+     * PHP would generate:  ids[]=1&IDs[]=2&IDs[]=3
+     * Shopify expects:     ids=["1","2","3"] (URL encoded)
+     *
+     * @param array $params Array of query parameters
+     *
+     * @return string The URL encoded query string ("foo=bar&bar=foo")
+     */
+    public static function buildQueryString(array $params): string
+    {
+        // Exclude HMAC from query string
+        $params = array_filter($params, function ($key) {
+            return $key !== 'hmac';
+        }, ARRAY_FILTER_USE_KEY);
+
+        // Concatenate arrays to conform with Shopify
+        array_walk($params, function (&$value, $key) {
+            if (!is_array($value)) {
+                return;
+            }
+
+            $escapedValues = array_map(function ($value) {
+                return sprintf('"%s"', $value);
+            }, $value);
+            $concatenatedValues = implode(',', $escapedValues);
+            $encapsulatedValues = sprintf('[%s]', $concatenatedValues);
+
+            $value = $encapsulatedValues;
+        });
+
+        // Building the actual query using PHP's native function
+        return http_build_query($params);
+    }
+
     /**
      * Determines if request is valid by processing secret key through an HMAC-SHA256 hash function
      *
@@ -70,12 +106,14 @@ public static function sanitizeShopDomain(string $shop, ?string $myshopifyDomain
      */
     public static function validateHmac(array $params, string $secret): bool
     {
-        $hmac = $params['hmac'] ?? '';
-        unset($params['hmac']);
-
-        $computedHmac = hash_hmac('sha256', http_build_query($params), $secret);
+        if (empty($params['hmac']) || empty($secret)) {
+            return false;
+        }
 
-        return hash_equals($hmac, $computedHmac);
+        return hash_equals(
+            $params['hmac'],
+            hash_hmac('sha256', self::buildQueryString($params), $secret)
+        );
     }
 
     /**

tests/UtilsTest.php

@@ -87,6 +87,58 @@ public function sanitizeShopWithCustomDomainsProvider()
         ];
     }
 
+    /**
+     * @dataProvider buildQueryStringProvider
+     */
+    public function testBuildQueryString($params, $queryString)
+    {
+        $this->assertEquals($queryString, Utils::buildQueryString($params));
+    }
+
+    public function buildQueryStringProvider()
+    {
+        return [
+            [
+                [],
+                ''
+            ],
+            [
+                [
+                    'hmac' => 'ThisShouldNotAppearInTheString',
+                    'foo' => 'ThisShould',
+                    'bar' => 'AsShouldThis',
+                ],
+                'foo=ThisShould&bar=AsShouldThis'
+            ],
+            [
+                [
+                    'hmac' => 'ThisShouldNotAppearInTheString',
+                    'someArray' => [
+                      'foo',
+                      'bar',
+                      1,
+                    ],
+                ],
+                'someArray=%5B%22foo%22%2C%22bar%22%2C%221%22%5D' // URLEncoded for: ["foo","bar","1"]
+            ],
+        ];
+    }
+
+    public function testEmptyHmac()
+    {
+        $this->assertEquals(false, Utils::validateHmac(
+            [],
+            'I AM SECRET'
+        ));
+
+        $this->assertEquals(false, Utils::validateHmac(
+            [
+                'hmac' => 'SomeHash',
+            ],
+            ''
+        ));
+    }
+
     public function testValidHmac()
     {
         // phpcs:ignore