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

Ajax always processes data for requests without entity body #3438

Closed
dmethvin opened this issue Dec 8, 2016 · 5 comments
Closed

Ajax always processes data for requests without entity body #3438

dmethvin opened this issue Dec 8, 2016 · 5 comments
Assignees
Labels
Milestone

Comments

@dmethvin
Copy link
Member

@dmethvin dmethvin commented Dec 8, 2016

I'm creating this ticket based on the discussion in the gh-3409 PR from @Litor so we don't forget to review it.

With ajax requests using HTTP methods that do not have a request body, such as GET, any provided data is always processed and removed, even if the caller specified processData: false. This seems contrary to the documentation which says:

The data option can contain either a query string of the form 'key1=value1&key2=value2', or an object of the form {key1: 'value1', key2: 'value2'}. If the latter form is used, the data is converted into a query string using jQuery.param() before it is sent. This processing can be circumvented by setting processData to false.

This behavior was also present before 3.x so it's not any kind of recent regression, but it does seem like a bug. I suppose you could argue that "this processing can be circumvented" refers only to serializing objects but not to appending the data to the URL, but that interpretation doesn't make sense. Why would someone want [object Object] appended to their URL? 😸

If we didn't process data at all, it would be possible for a beforeSend handler to take an object and stringify it to the URL as desired , something like this currently non-working example:

http://jsbin.com/fucigaqure/edit?html,console

@jaubourg
Copy link
Member

@jaubourg jaubourg commented Sep 17, 2017

The real question is : what is the purpose of setting the data option with processData false for a GET request if the data is not to be used in the final URL ?

The data option can be set to a string that is already serialized or an object which class provides a toString method that serializes the object properly.

Conflating the processData: false with "don't use the data" seems very wrong to me.

@gibson042
Copy link
Member

@gibson042 gibson042 commented Sep 18, 2017

processData: false is documented to "prevent this automatic processing [converting data to a query string and, for GET requests, appending it to the URL]". There's an argument to be made that processData should affect the jQuery.param serialization but not the URL manipulation, but that strikes me as confusing (because then processData: false doesn't actually prevent manipulation of data—or url, for that matter) and useless (as Dave points out, "why would someone want [object Object] appended to their URL?").

I have a hard time imagining any plausible purpose for jQuery.ajax(url, { type: "GET", data: nonString, processData: false, … }), but ignoring data (while exposing it to beforeSend) seems most in accord with user intentions.

No matter what, though, it's clear that http://api.jquery.com/jQuery.ajax/ needs an update.

@jaubourg
Copy link
Member

@jaubourg jaubourg commented Sep 18, 2017

I hear you but I simply can't agree with a change that will make the following code not work anymore :

$.ajax( {
    url: "http://my.domain/path/to/script",
    data: "param1=value1&param2=value2",
    processData: false
} );

There are many situations where it's far simpler to serialize url parameters beforehand rather than construct a data structure for the sole purpose of accomodating jQuery's internal serialization (hence my example of an object with a custom toString() that pretty much covers the data: nonString scenario).

I think there clearly is a problem with the documentation if it implies that processData: false is supposed to prevent the addition of data to the url. The data is "not processed" (i.e. not transformed) but it's still used. If you don't want the data added to the url then don't put it in the options in the first place : no data, no problem and no need for an additional option to ignore it.

BTW, processData predates the 1.5 ajax rewrite and always behaved this way.

As an aside, where and when beforeSend is called makes using it an anti-pattern in pretty much any situation, hence the introduction of prefilters where it's safe to mess with the options. If someone wants to handle data manually like you suggest then they just need to create an option of their own together with a prefilter that checks for it and modifies the url accordingly (this won't break the ajax caching mechanism, unlike beforeSend).

@dmethvin
Copy link
Member Author

@dmethvin dmethvin commented Sep 19, 2017

Hi @jaubourg! Always glad to have you here.

The data is "not processed" (i.e. not transformed) but it's still used.

We do want the data to be used. One of the use cases that precipitated the ticket was that someone wanted to serialize the data to JSON rather than form-encode it.

If someone wants to handle data manually like you suggest then they just need to create an option of their own together with a prefilter that checks for it and modifies the url accordingly

Ok, so a prefilter that might take jsonData: { ... } and convert it to a stringified data object. Or alternatively, just a data object and have the prefilter decide whether to convert it based on method, specific endpoint URL, custom convertDataToJSON: true in the options, whatever.

If the jQuery.ajax() caller is treating the URL endpoint as a "black box" and doesn't want to worry about the data format then it seems like the prefilter is a good solution because it can be implemented and any scattered callers don't need to worry. If the caller is implementing the endpoint interface through their own jQuery.ajax() call wrapper then they shouldn't use processData at all and just convert the data to a string before they make the call.

If we're not making code changes I definitely think we need to make docs changes. I'm not sure I see a lot of utility in processData because it only applies to requests with entity bodies and specific transports. It's almost as if we should say "don't use this".

@jaubourg
Copy link
Member

@jaubourg jaubourg commented Sep 19, 2017

Hi @dmethvin! Always glad to argue on tickets ;)

Please note data has already been potentially processed before prefilters are called so a custom option is the only available route here.

Your "don't use this" recommandation is spot on. The only time you need it is when you provide data as a properly serialized query or as an object with a custom toString that handles the serialization. Indeed those cases are very rare. Beside using an option to change the way another option is handled is quite the anti-pattern (a rawData option would have been better but, ahem, "backward compatibility" and it doesn't provide automagical serialization from within the ajax architecture itself).

The following would definitely make processData obsolete:

$.ajaxPrefilter( options => {
    options.data = options.data || options.rawData;
} );
@dmethvin dmethvin closed this in d723789 Jan 16, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants
You can’t perform that action at this time.