fix: parseKeyValue function does not allow additional equals signs #12351
Conversation
In some cases people will not follow all URL standards and may have unescaped = characters in their GET parameter values. Currently $location will not parse them correctly (or rather too correctly) and in combination with the routing will make a pushState that removes them from the URL. My proposed fix will just join everything after the key back together with the '=' character it is split by before.
|
Well I guess the build failing wasn't rly my fault. |
|
I am a bit torn about this. On the surface don't really like this idea since we are undermining the standard. But then again I see that accepting While I can see that it would make life much easier in the cases where |
|
If there is an unencoded & in the string, something is rly wrong and that can't be solved. The case I needed this for are base64 encoded strings. |
| @@ -1129,9 +1129,9 @@ function parseKeyValue(/**string*/keyValue) { | |||
| forEach((keyValue || "").split('&'), function(keyValue) { | |||
| if ( keyValue ) { | |||
| key_value = keyValue.replace(/\+/g,'%20').split('='); | |||
There was a problem hiding this comment.
I know it is slightly more expensive, but you can save some of the weirdness later in the code if by changing .split('=') with .split(/=(.*)/, 2)
There was a problem hiding this comment.
I think it will be nicer just to use indexOf:
var key, value, splitPoint;
...
if ( keyValue ) {
key = keyValue = keyValue.replace(/\+/g,'%20');
splitPoint = keyValue.indexOf('=');
if (splitPoint != -1) {
key = keyValue.substring(0, splitPoint);
value = keyValue.substring(splitPoint+1);
}
key = tryDecodeURIComponent(key);
if ( isDefined(key) ) {
value = tryDecodeURIComponent(value) || true;
if (!hasOwnProperty.call(obj, key)) {
obj[key] = val;
} else if(isArray(obj[key])) {
obj[key].push(val);
} else {
obj[key] = [obj[key],val];
}
}
}
|
Even when I agree with @petebacondarwin that this does not follow the spec, the behavior in the PR is how most servers handle unencoded |
|
OK, if @lgalfaso is happy then I am happy. I think we can improve the algorithm and then land this. |
|
Sounds great, will this be available in any 1.2.* version? That's the version my project currently runs on. |
|
We are only putting security fixes into 1.2 but I guess since you asked so nicely we could consider backporting this in. |
|
The version I posted or the version from @petebacondarwin works fine with me. @jniebuhr can you please update the PR? |
In some cases people will not follow all URL standards and may have unescaped = characters in their GET parameter values. Currently $location will not parse them correctly dropping everything after the unescaped =. This change includes all characters after the first `=` up to the next `&`. Closes angular#12351
In some cases people will not follow all URL standards and may have unescaped = characters in their GET parameter values. Currently $location will not parse them correctly dropping everything after the unescaped =. This change includes all characters after the first `=` up to the next `&`. Closes angular#12351
In some cases people will not follow all URL standards and may have unescaped = characters in their GET parameter values. Currently $location will not parse them correctly dropping everything after the unescaped =. This change includes all characters after the first `=` up to the next `&`. Closes angular#12351
In some cases people will not follow all URL standards and may have unescaped = characters in their GET parameter values. Currently $location will not parse them correctly (or rather too correctly) and in combination with the routing will make a pushState that removes them from the URL.
My proposed fix will just join everything after the key back together with the '=' character it is split by before.