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
Ensure unescaped commas are used for periods in url param #16784
Conversation
I did some digging and I think this can also be fixed by handling %2C or urldecoding in Range::parseDateRange. (The other alternative fix is to urldecode every time 'date' is accessed via Common::getRequestVar, but that seems excessive.) |
@diosmosis I tried some other stuff as well, but couldn't get it working for all cases. If you have a better solution feel free to simply adjust the PR |
The problem with replacing the comma is that it will only be done client side in this one place, though we of course change the URL in many places, sometimes setting location.href directly, and it could cause problems w/ segments that contain commas given the extra confusing segment encoding logic. I just checked multisites, but what are the other cases? I can try to fix those as well. |
@diosmosis When we want to "support" an escaped comma in the url parameter we need to be sure, that it works on all pages. IIrc it didn't work for various reports |
Replacing the comma in every part of the URL will have side effects. |
… initially because broadcast does not urldecode but $.param expects urldecoded value)
Found the root cause, it wasn't server side. When the url contains an empty hash, the hash is created using the main url values, but it double encodes them. This was due to |
Description:
Might be a bit hackish solution, but Matomo simply can't handle an encoded date range in the url at various places. So might be the simplest solution to avoid having an encoded value instead of searching for all places that might make problem...
fixes #15238
Review