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

URL parameters case sensitive. #992

Closed
leigh-pointer opened this issue Dec 10, 2020 · 4 comments
Closed

URL parameters case sensitive. #992

leigh-pointer opened this issue Dec 10, 2020 · 4 comments

Comments

@leigh-pointer
Copy link
Contributor

Using the PageState.QueryString["id"] the "iD" will throw an error due to a miss match. Personally adding case sensitivity to params is just another debugging headache. Is this by design?

@hishamco
Copy link
Contributor

@sbwalker did you fix this in the last PR

@sbwalker
Copy link
Member

sbwalker commented Dec 10, 2020

@hishamco this was not one of the case sensitivity items I addressed in my last PR

Note that the spec says:

"The scheme and host are case-insensitive and normally provided in lowercase; all other components are compared in a case-sensitive manner. Characters other than those in the "reserved" set are equivalent to their percent-encoded octets: the normal form is to not encode them (see Sections 2.1 and 2.2 of [RFC3986])."

So querystring parameters are supposed to be case sensitive. As a result, the QueryString property as part of the Oqtane PageState class was defined as a Dictionary ( which is case sensitive by default ).

That being said, after research it appears that Microsoft does not follow the spec. The QueryString property on the Request object in C# is a System.Collections.Specialized.NameValueCollection - which uses a custom case sensitivity. It treats the Keys as case insensitive and the Values as case sensitive.

For consistency Oqtane should support the same behavior that developers are familiar with when working with other Microsoft frameworks. However changing the type of the PageState.QueryString property from Dictionary to System.Collections.Specialized.NameValueCollection would be a breaking change. I am not sure if it would be possible to add a IEqualityComparer to the existing Dictionary without resulting in a breaking change ( research would be required ). Another property could be added to PageState ie. QueryParameters... and QueryString could be deprecated. Note that developers can also access the PageState.Uri property ( which unfortunately does not have a query string collection of its own - which is why the QueryString property was added to PageState ) and parse the Query property themselves - if you look in the Utilities class you will find some helper methods for this ( however I am not sure how they handle case sensitivity ).

@sbwalker
Copy link
Member

sbwalker commented Dec 11, 2020

I am going to test the following, which may be a simple fix and not a breaking change:

Dictionary<string, string> dict = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);

@sbwalker
Copy link
Member

This works and is backwards compatible

sbwalker added a commit that referenced this issue Dec 12, 2020
make QueryString parameter keys case insensitive - resolves #992
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

No branches or pull requests

3 participants