From 9ae249c6d4c3ef5f78d65d4e4fb99c95571ff3d3 Mon Sep 17 00:00:00 2001
From: otsch <otsch@users.noreply.github.com>
Date: Wed, 12 Apr 2023 22:16:54 +0200
Subject: [PATCH] Some minor improvements

* Rename method `normalizeArrays` to `fixDuplicateKeysInString` to be
  more specific.
* `strstr` to `str_contains`.
* If preg_replace should somehow fail, keep the $query (also phpstan
  complained).
* Move test case to the FromStringTest file.
* Add changelog.
---
 CHANGELOG.md                 |  7 +++++++
 src/Query.php                | 12 +++++++-----
 tests/FromStringTest.php     |  8 ++++++++
 tests/SanitizeArraysTest.php | 11 -----------
 4 files changed, 22 insertions(+), 16 deletions(-)
 delete mode 100644 tests/SanitizeArraysTest.php

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b30f7ea..c766596 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [1.0.2] - 2023-04-12
+### Fixed
+- Duplicate keys without array notation (like `foo=bar&foo=baz`) are now also interpreted as array (like `foo[]=bar&foo[]=baz`). This is considered a bugfix because:
+  - Previously `foo=bar&foo=baz` became `foo=baz`, which is most likely unwanted behavior.
+  - Of course, such query strings should preferably be written using the array notation (`[]`), but if such a query is written without the brackets, the intention is either that it should be an array, or it's a mistake. If it is a mistake, the probability to notice it is higher, when the result contains all values instead of only the last one.
+  - As this library is also part of the crwlr crawling library and there are servers intentionally using the syntax without brackets (e.g. google https://fonts.googleapis.com/css2?family=Baloo&family=Roboto) it's better to interpret it as an array.
+
 ## [1.0.1] - 2023-01-30
 ### Fixed
 - When creating a `Query` from string, and it contains empty key value parts, like `&foo=bar`, `foo=bar&` or `foo=bar&&baz=quz`, the unnecessary `&` characters are removed in the string version now. For example, `&foo=bar` previously lead to `$instance->toString()` returning `&foo=bar` and now it returns `foo=bar`.
diff --git a/src/Query.php b/src/Query.php
index 0412622..55ca894 100644
--- a/src/Query.php
+++ b/src/Query.php
@@ -555,8 +555,8 @@ private function sanitize(string $query): string
         }
 
         $query = preg_replace('/&+/', '&', $query) ?? $query;
-        
-        $query = $this->normalizeArrays($query);
+
+        $query = $this->fixDuplicateKeysInString($query);
 
         return $this->arrayToString($this->stringToArray($query));
     }
@@ -1004,6 +1004,8 @@ private function spaceCharacter(): string
     }
 
     /**
+     * Fix duplicate keys in query strings without brackets
+     *
      * Arrays can be present in query strings in two formats, as follows:
      *  1.) ?key[]=value1&key[]=value2
      *  2.) ?key=value1&key=value2
@@ -1011,19 +1013,19 @@ private function spaceCharacter(): string
      * Both *should* be parsed in the same way, however, PHP's parse_str() doesn't handle version #1. This function
      * normalizes version #2 structure query strings to #1 so that we can parse both accordingly.
      */
-    private function normalizeArrays(string $query): string
+    private function fixDuplicateKeysInString(string $query): string
     {
         // Count the occurrences of all request keys to check for duplicates
         $keyOccurrences = array_count_values(array_map(fn ($val) => explode('=', $val, 2)[0], explode('&', $query)));
 
         foreach ($keyOccurrences as $key => $count) {
-            if ($count > 1 && !strstr($key, '[')) {
+            if ($count > 1 && !str_contains($key, '[')) {
                 // Duplicate query string key without array notation, convert to {keyName}[] structure
                 $query = preg_replace(
                     '#(^|[?&])(' . preg_quote($key) . ')=#',
                     '$1$2[]=',
                     $query,
-                );
+                ) ?? $query;
             }
         }
 
diff --git a/tests/FromStringTest.php b/tests/FromStringTest.php
index b2f0eed..580eee4 100644
--- a/tests/FromStringTest.php
+++ b/tests/FromStringTest.php
@@ -178,3 +178,11 @@ function (array $array, string $normalized, string $raw) {
         'foo.bar[0]=v1&foo.bar_extra[0]=v2&foo.bar.extra[0]=v3',
     ],
 ]);
+
+test('Duplicate query string keys are converted to arrays', function () {
+    expect(Query::fromString('test=1&test2=2&test=2&test[]=3&test[test]=4')->toArray())
+        ->toEqual([
+            'test' => [1, 2, 3, 'test' => 4],
+            'test2' => 2,
+        ]);
+});
diff --git a/tests/SanitizeArraysTest.php b/tests/SanitizeArraysTest.php
deleted file mode 100644
index eabef4a..0000000
--- a/tests/SanitizeArraysTest.php
+++ /dev/null
@@ -1,11 +0,0 @@
-<?php
-
-use Crwlr\QueryString\Query;
-
-test('Duplicate query string keys are correctly converted to arrays', function () {
-    expect(Query::fromString('test=1&test2=2&test=2&test[]=3')->toArray())
-        ->toEqual([
-            'test' => [1, 2, 3],
-            'test2' => 2,
-        ]);
-});