Ajax: More checks before replacing %20 to + in data #4123
Conversation
|
|
|
Is there a reason we can't set |
|
@timmywil I preferred to keep existing sequence of sending headers: if |
|
This seems like a bit of a worm tin... what about the JSONP prefilter that has its own logic branch for Having promoted However, if the above is not persuasive, then the best way to implement this fix would be as an early built-in prefilter that overrides |
|
@gibson042 I discovered this issue in the wild, it broke in-prod system. The issue is very hidden and uncovered itself several months after migrating from jQuery 2 to 3. It’s not good that there exist a hidden default value of a not-required option, that might corrupt data unexpectedly. It’s twice not good that jQuery 3 behaves differently than jQuery 2. So it’s not a documentation issue, it’s a dangerous bug corrupting data silently, and it was not easy to root because no one in the world expects hidden values covertly override explicits somewhere in deep internals. However, if you think the patch should be an early override of |
|
If there are different code paths for the same header based on the |
|
@timmywil It might be easy fix for the software not yet written. For the software already deployed and dependent on the behavior demonstrated by 2.x the easiest fix is to roll back to 2.x branch, which was the first thing we did after the issue discovery. In many places we generate headers required for a request by a separate function returning headers object. I suppose it‘s more or less common practice. Moreover, we never use So to delete one specific prop from So from practical POV I‘m sure it’s a bug and not documentation issue. Hope I did all my best to fix it. |
|
Thanks, @ermouth. I reviewed jQuery 2.2.4 and agree that this is indeed a regression. So as I described above, I think we should have a built-in ajaxPrefilter (similar to the one preventing inadvertent XSS) that overwrites |
|
This approach was rejected & we have a newer PR: #4650 so I'm closing this one. |
Summary
Before $.ajax replaces
%20to+in POST-ed data,opts.headers['content-type']is now checked as well asopts.contentType, and explicit header wins if present. Replacement only takes place if resulting content type starts withapplication/x-www-form-urlencoded.Fixes #4119.
Checklist