Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for query strings that contain duplicate keys #5

Merged
merged 3 commits into from
Apr 12, 2023

Conversation

aran112000
Copy link
Contributor

@aran112000 aran112000 commented Apr 12, 2023

Currently when a query string is parsed, PHP's parse_str() will remove any duplicate keys and will only preserve the final value.

Example

?key=value1&key=value2 = key: "value2"

Expected value

?key=value1&key=value2 = key: ["value1", "value2"]

This PR normalises these formats before processing so these strings will become array notation which is then correctly parsed by PHP's parse_str().

* 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.
@otsch
Copy link
Member

otsch commented Apr 12, 2023

Already added it to the changelog, but to also make it visible here:

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.

@otsch otsch merged commit 9ae249c into crwlrsoft:main Apr 12, 2023
@aran112000
Copy link
Contributor Author

Amazing, thanks for your support and quick turnaround on this @otsch

@aran112000 aran112000 deleted the duplicate-query-string-keys branch April 12, 2023 20:30
@otsch
Copy link
Member

otsch commented Apr 12, 2023

Of course. Thank you for the contribution @aran112000 ! 🙏🏻🙂👏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants