Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"karma-qunit": "4.1.2",
"karma-webkit-launcher": "2.1.0",
"load-grunt-tasks": "5.1.0",
"multiparty": "4.2.3",
"native-promise-only": "0.8.1",
"playwright-webkit": "1.29.2",
"promises-aplus-tests": "2.1.2",
Expand Down
6 changes: 3 additions & 3 deletions src/ajax.js
Original file line number Diff line number Diff line change
Expand Up @@ -562,14 +562,14 @@ jQuery.extend( {
}
}

// Apply prefilters
inspectPrefiltersOrTransports( prefilters, s, options, jqXHR );

// Convert data if not already a string
if ( s.data && s.processData && typeof s.data !== "string" ) {
s.data = jQuery.param( s.data, s.traditional );
}

// Apply prefilters
inspectPrefiltersOrTransports( prefilters, s, options, jqXHR );
Comment on lines +565 to -571
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think moving prefilters ahead of string coercion is a breaking change with respect to custom prefilters, cf. https://api.jquery.com/jQuery.ajaxPrefilter/ (emphasis mine):

originalOptions are the options as provided to the $.ajax() method, unmodified and, thus, without defaults from ajaxSettings

Is there a way to instead establish something like prefilter phases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line that now happens after the prefilters is only setting data, not options. Is there any change that I'm missing?

BTW, I would also consider this a breaking change regardless of the above, that's why I'm targeting it at v4.


// If request was aborted inside a prefilter, stop there
if ( completed ) {
return jqXHR;
Expand Down
17 changes: 17 additions & 0 deletions src/ajax/binary.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import jQuery from "../core.js";

import "../ajax.js";

jQuery.ajaxPrefilter( function( s ) {

// Binary data needs to be passed to XHR as-is without stringification.
if ( typeof s.data !== "string" && !jQuery.isPlainObject( s.data ) ) {
s.processData = false;
}

// `Content-Type` for requests with `FormData` bodies needs to be set
// by the browser as it needs to append the `boundary` it generated.
if ( s.data instanceof window.FormData ) {
s.contentType = false;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: check if this could be overridden in options if this is set to false for all binary data, not just FormData.

Copy link
Member Author

@mgol mgol Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this cannot be changed via settings as of now. 😕 I'll need another patch, I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR: #5205

}
} );
1 change: 1 addition & 0 deletions src/jquery.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import "./ajax.js";
import "./ajax/xhr.js";
import "./ajax/script.js";
import "./ajax/jsonp.js";
import "./ajax/binary.js";
import "./ajax/load.js";
import "./core/parseXML.js";
import "./core/parseHTML.js";
Expand Down
11 changes: 11 additions & 0 deletions test/data/mock.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,17 @@ protected function xmlOverJsonp( $req ) {
echo "$cleanCallback($text)\n";
}

protected function formData( $req ) {
$prefix = 'multipart/form-data; boundary=--';
$contentTypeValue = $req->headers[ 'CONTENT-TYPE' ];
if ( substr( $contentTypeValue, 0, strlen( $prefix ) ) === $prefix ) {
echo 'key1 -> ' . $_POST[ 'key1' ] . ', key2 -> ' . $_POST[ 'key2' ];
} else {
echo 'Incorrect Content-Type: ' . $contentTypeValue .
"\nExpected prefix: " . $prefix;
}
}

protected function error( $req ) {
header( 'HTTP/1.0 400 Bad Request' );
if ( isset( $req->query['json'] ) ) {
Expand Down
7 changes: 5 additions & 2 deletions test/data/testinit.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,11 @@ function url( value ) {
}

// Ajax testing helper
this.ajaxTest = function( title, expect, options ) {
QUnit.test( title, function( assert ) {
this.ajaxTest = function( title, expect, options, wrapper ) {
if ( !wrapper ) {
wrapper = QUnit.test;
}
wrapper.call( QUnit, title, function( assert ) {
assert.expect( expect );
var requestOptions;

Expand Down
28 changes: 28 additions & 0 deletions test/middleware-mockserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const url = require( "url" );
const fs = require( "fs" );
const getRawBody = require( "raw-body" );
const multiparty = require( "multiparty" );

let cspLog = "";

Expand Down Expand Up @@ -141,6 +142,19 @@ const mocks = {
resp.writeHead( 200 );
resp.end( `${ cleanCallback( callback ) }(${ JSON.stringify( body ) })\n` );
},
formData: function( req, resp, next ) {
const prefix = "multipart/form-data; boundary=--";
const contentTypeValue = req.headers[ "content-type" ];
resp.writeHead( 200 );
if ( ( prefix || "" ).startsWith( prefix ) ) {
getMultiPartContent( req ).then( function( { fields = {} } ) {
resp.end( `key1 -> ${ fields.key1 }, key2 -> ${ fields.key2 }` );
}, next );
} else {
resp.end( `Incorrect Content-Type: ${ contentTypeValue
}\nExpected prefix: ${ prefix }` );
}
},
error: function( req, resp ) {
if ( req.query.json ) {
resp.writeHead( 400, { "content-type": "application/json" } );
Expand Down Expand Up @@ -363,4 +377,18 @@ function getBody( req ) {
} );
}

function getMultiPartContent( req ) {
return new Promise( function( resolve ) {
if ( req.method !== "POST" ) {
resolve( "" );
return;
}

const form = new multiparty.Form();
form.parse( req, function( _err, fields, files ) {
resolve( { fields, files } );
} );
} );
}

module.exports = MockserverMiddlewareFactory;
43 changes: 43 additions & 0 deletions test/unit/ajax.js
Original file line number Diff line number Diff line change
Expand Up @@ -3049,4 +3049,47 @@ if ( typeof window.ArrayBuffer === "undefined" || typeof new XMLHttpRequest().re
assert.ok( jQuery.active === 0, "ajax active counter should be zero: " + jQuery.active );
} );

ajaxTest( "jQuery.ajax() - FormData", 1, function( assert ) {
var formData = new FormData();
formData.append( "key1", "value1" );
formData.append( "key2", "value2" );

return {
url: url( "mock.php?action=formData" ),
method: "post",
data: formData,
success: function( data ) {
assert.strictEqual( data, "key1 -> value1, key2 -> value2",
"FormData sent correctly" );
}
};
} );

ajaxTest( "jQuery.ajax() - URLSearchParams", 1, function( assert ) {
var urlSearchParams = new URLSearchParams();
urlSearchParams.append( "name", "peter" );

return {
url: url( "mock.php?action=name" ),
method: "post",
data: urlSearchParams,
success: function( data ) {
assert.strictEqual( data, "pan", "URLSearchParams sent correctly" );
}
};
}, QUnit.testUnlessIE );

ajaxTest( "jQuery.ajax() - Blob", 1, function( assert ) {
var blob = new Blob( [ "name=peter" ], { type: "text/plain" } );

return {
url: url( "mock.php?action=name" ),
method: "post",
data: blob,
success: function( data ) {
assert.strictEqual( data, "pan", "Blob sent correctly" );
}
};
} );

} )();