Skip to content

/ jquery

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

Jump to bottom

Invalid encoding of %20 char sequence in $.ajax post, put body #4119

Closed
ermouth opened this issue Jul 5, 2018 · 6 comments
Closed

Invalid encoding of %20 char sequence in $.ajax post, put body #4119

ermouth opened this issue Jul 5, 2018 · 6 comments
Labels
Ajax Bug Has Pull Request help wanted
Milestone
3.5.0

Comments

@ermouth
Copy link

@ermouth ermouth commented Jul 5, 2018 •
edited

Description

Seems that jQuery 3 replaces %20 char sequence to + in a request body during PUT and POST. A request like

$.ajax({
  method: 'put', 
  url: '/test/doc', 
  dataType: 'json',
  headers: {'content-type':'application/json'},
  data: JSON.stringify({val:'%20'})
})

actually sends {"val":"+"} in 3.2.1 and 3.3.1, both with Migration tools.

jQuery however 2.x behaves correctly.

Update. Seems the reason is this block https://github.com/jquery/jquery/blob/master/src/ajax.js#L617-L621. It assumes s.contentType is the only source of Content-Type header, but there can be s.headers['content-type'], which later (https://github.com/jquery/jquery/blob/master/src/ajax.js#L648-L650) overrides s.contentType value.
@dmethvin
Copy link
Member

@dmethvin dmethvin commented Jul 8, 2018

I agree with your analysis! If we're letting the header override the setting I guess we should honor both. Would you like to file a pull request?
@dmethvin dmethvin added the Bug label Jul 8, 2018
@ermouth
Copy link
Author

@ermouth ermouth commented Jul 9, 2018

Yes, but it may take a time to read all community guides. However hope a fix for this is not urgent )
@timmywil timmywil added this to the Future milestone Jul 9, 2018
ermouth added a commit to ermouth/jquery that referenced this issue Jul 10, 2018
@ermouth
Ajax: More checks before replacing %20 to + in data
f7c39d8 
Fixes jquery#4119
@ermouth ermouth mentioned this issue Jul 10, 2018
Closed
2 of 4 tasks complete
@Gomathy97
Copy link

@Gomathy97 Gomathy97 commented Mar 13, 2020 •
edited

Hi, is there any update on this?
It would be fine if there’s any alternative
This is creating a major breakage in one of works. It would helpful if you assist at the earliy
@mgol
Copy link
Member

@mgol mgol commented Mar 13, 2020

@Gomathy97 The summary is in #4123 (comment) & #4123 (comment). We welcome PRs, that's the easiest way to make the fix be available sooner.
wenz added a commit to wenz/jquery that referenced this issue Mar 29, 2020
@wenz
Component: ajax
fa8db2b 
Overwrite s.contentType with content-type header value, if any

Fixes jquery#4119
@wenz wenz mentioned this issue Mar 29, 2020
Merged
3 of 4 tasks complete
@wenz
Copy link
Contributor

@wenz wenz commented Mar 29, 2020

I've sent a PR that implements the functionality as suggested in the #4123 discussion; based on the research and code by @ermouth
wenz added a commit to wenz/jquery that referenced this issue Mar 30, 2020
@wenz
Ajax: Overwrite s.contentType with content-type header value, if any
3b7dab1 
Fixes jquery#4119
wenz added a commit to wenz/jquery that referenced this issue Mar 30, 2020
@wenz
Ajax: Overwrite contentType property with header, plus new tests
4501ab4 
Fixes jquery#4119
wenz added a commit to wenz/jquery that referenced this issue Mar 30, 2020
@wenz
Ajax: Test for contentType override via header
8ee7197 
Fixes jquery#4119
@mgol mgol added the Has Pull Request label Mar 30, 2020
@mgol
Copy link
Member

@mgol mgol commented Mar 30, 2020

The mentioned PR by @wenz: #4650.
@mgol mgol modified the milestones: Future, 3.5.0 Mar 30, 2020
@mgol mgol closed this in #4650 Apr 6, 2020
mgol added a commit that referenced this issue Apr 6, 2020
@wenz @gibson042 @mgol
Ajax: Overwrite s.contentType with content-type header value, if any
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
7fb90a6 
This fixes the issue of "%20" in POST data being replaced with "+"
even for requests with content-type different from
"application/x-www-form-urlencoded", e.g. for "application/json".

Fixes gh-4119
Closes gh-4650

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
Co-authored-by: Michał Gołębiowski-Owczarek <m.goleb@gmail.com>
mgol added a commit that referenced this issue Apr 6, 2020
@wenz @gibson042 @mgol
Ajax: Overwrite s.contentType with content-type header value, if any
Loading status checks…
065143c 
This fixes the issue of "%20" in POST data being replaced with "+"
even for requests with content-type different from
"application/x-www-form-urlencoded", e.g. for "application/json".

Fixes gh-4119
Closes gh-4650

(cherry picked from 7fb90a6)

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
Co-authored-by: Michał Gołębiowski-Owczarek <m.goleb@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Ajax Bug Has Pull Request help wanted
Projects
None yet
Milestone
3.5.0
Linked pull requests

Successfully merging a pull request may close this issue.

Ajax: Overwrite s.contentType with content-type header value, if any
Ajax: More checks before replacing %20 to + in data
6 participants
@wenz @dmethvin @timmywil @ermouth @mgol @Gomathy97