thephpleague / uri-src

URI manipulation Library
https://uri.thephpleague.com
MIT License
27 stars 8 forks source link

mergeQuery is case sensitive #6

Closed lludwig closed 2 years ago

lludwig commented 3 years ago

Feature Request

Q A
New Feature yes
BC Break no

Proposal

mergeQuery is case sensitive per RFC spec. I suggest adding a parameter (either at object creation for the URI or when calling mergeQuery to state the 'key' values are not case sensitive. That way

Key=value gets matched with key=newvalue and replaces it in the URL. Currently, the function will append the value so the URL is now Key=value&key=newvalue which in the case of many systems are two values not one.

nyamsprod commented 3 years ago

@lludwig thanks for using the library. I understand your concern but I do not like the idea of having an option to change the expected behaviour of an operation. Would using a combination of removePairs + mergeQuery achieve the same results ? The current modifier are suppose to be building blocks from which you could/should build your own business rules on how to modify your URI. So I would rather have them do only one thing and one thing only.

Also nothing prevents you from creating your own specific modifier that would just do that for your own use case. I for one have never encounter the situation where query keys were design to be case insensitive. I find it strange and would be considered a major area of potential bug and error.

lludwig commented 3 years ago

I've not used the function removePairs so I'll have to test what happens but suspect it will only remove the entry that's matching in exact case (i.e. Key if I use Key but not key)

Yes, it's true I can create my own modifier. The use case is individuals are entering or pasting a URL that can be in different formats, yet I'm only checking for the lowercase version of that key. Some servers/services don't care about the case so their form key and Key are the same entry and would either be two entries OR takes the last entry (depends upon the programming language and the filtering).

I would think this is a more universal problem than just me.

lludwig commented 3 years ago

So in my specific case the key can be:

Each is considered different and with mergeQuery will insert (not replace) with what I define as the variable.

I wind up with two entries in the URI which isn't desirable.

nyamsprod commented 3 years ago

I fully understand your issue but I'm not convinced that adding a case insensitive mergeQuery even with a different name is a correct way of doing it as you introduce an intermediary step where you need to first normalize your query string keys before merging. So instead of a mergeQuery what you are really looking for is a QueryStringKeys(toUpper|toLower)Modifier 🤔 Which to me again seems to be a very specific issue and not a broaden one. But again I could be wrong

lludwig commented 3 years ago

well in my case it's only one key I'm modifying to make standard, though one could argue all of the keys for specific URLs be standardized. Meaning if one is case insensitive they are all case insenstive.

nyamsprod commented 3 years ago

TBH if it's just one case then use removePair before mergeQuery if it is for a unknown number of keys then a specific modifier for your own use should be created. I believe all the building blocks are present already you just need to assemble them in the correct order.