Skip to content

Commit

Permalink
Fix xhr.setRequestHeader() in Ajax beforeSend
Browse files Browse the repository at this point in the history
I broke this in 23d1879, where I
promised to make more Ajax settings configurable in `beforeSend`.
However, it drifted away from jQuery compatibility.

In both jQuery 1.10 and 2.0, the state in `beforeSend` is:

- `settings.data` is `undefined` for GET requests, as it was already
  serialized and added as query params to the URL.
- `settings.headers` is `undefined` if an object was not originally
  specified in the request, but otherwise adding or changing the values
  in it has no effect. This might be a jQuery bug, since its
  documentatation clearly indicates that you should be able to
  add/change header values this way.
- `xhr.setRequestHeader()` is available, although technically
  `xhr.open()` hasn't been called yet.

Since the native `setRequestHeader()` can't be called before `open()`,
I replaced it with our custom method that collects the header values
until they're ready to be applied to the XHR object.

Fixes #878
  • Loading branch information
mislav committed Dec 12, 2013
1 parent c074a94 commit 0604621
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 10 deletions.
21 changes: 13 additions & 8 deletions src/ajax.js
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@
if (options.processData && options.data && $.type(options.data) != "string")
options.data = $.param(options.data, options.traditional)
if (options.data && (!options.type || options.type.toUpperCase() == 'GET'))
options.url = appendQuery(options.url, options.data), options.data = null
options.url = appendQuery(options.url, options.data), options.data = undefined
}

$.ajax = function(options){
Expand All @@ -205,21 +205,26 @@
}

var mime = settings.accepts[dataType],
baseHeaders = { },
headers = { },
setHeader = function(name, value) { headers[name.toLowerCase()] = [name, value] },
protocol = /^([\w-]+:)\/\//.test(settings.url) ? RegExp.$1 : window.location.protocol,
xhr = settings.xhr(), abortTimeout
xhr = settings.xhr(),
nativeSetHeader = xhr.setRequestHeader,
abortTimeout

if (deferred) deferred.promise(xhr)

if (!settings.crossDomain) baseHeaders['X-Requested-With'] = 'XMLHttpRequest'
baseHeaders['Accept'] = mime || '*/*'
if (!settings.crossDomain) setHeader('X-Requested-With', 'XMLHttpRequest')
setHeader('Accept', mime || '*/*')
if (mime = settings.mimeType || mime) {
if (mime.indexOf(',') > -1) mime = mime.split(',', 2)[0]
xhr.overrideMimeType && xhr.overrideMimeType(mime)
}
if (settings.contentType || (settings.contentType !== false && settings.data && settings.type.toUpperCase() != 'GET'))
baseHeaders['Content-Type'] = (settings.contentType || 'application/x-www-form-urlencoded')
settings.headers = $.extend(baseHeaders, settings.headers || {})
setHeader('Content-Type', settings.contentType || 'application/x-www-form-urlencoded')

if (settings.headers) for (name in settings.headers) setHeader(name, settings.headers[name])
xhr.setRequestHeader = setHeader

xhr.onreadystatechange = function(){
if (xhr.readyState == 4) {
Expand Down Expand Up @@ -256,7 +261,7 @@
var async = 'async' in settings ? settings.async : true
xhr.open(settings.type, settings.url, async, settings.username, settings.password)

for (name in settings.headers) xhr.setRequestHeader(name, settings.headers[name])
for (name in headers) nativeSetHeader.apply(xhr, headers[name])

if (settings.timeout > 0) abortTimeout = setTimeout(function(){
xhr.onreadystatechange = empty
Expand Down
32 changes: 30 additions & 2 deletions test/ajax.html
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,20 @@ <h1>Zepto Ajax unit tests</h1>
$.get('echo', { sample: 'plain' }, 'text')
},

testAjaxBeforeSendSetRequestHeader: function(t){
t.pause()
$.ajax({
url: 'echo',
beforeSend: function(xhr, settings){
xhr.setRequestHeader('Accept', 'text/plain')
t.assertUndefined(settings.headers)
},
success: t.reg.resumeHandler('success', function(response){
t.assertLine("accept: text/plain", response)
})
})
},

testAjaxPost: function(t){
t.pause()
var xhr = $.post('echo', t.reg.resumeHandler('success', function(response){
Expand Down Expand Up @@ -739,6 +753,7 @@ <h1>Zepto Ajax unit tests</h1>
this.async = async
},
setRequestHeader: function(name, value) {
if (!this.method) throw "setRequestHeader() called before open()"
this.headers.push({ name: name, value: value })
},
getResponseHeader: function(name) {
Expand Down Expand Up @@ -1088,17 +1103,30 @@ <h1>Zepto Ajax unit tests</h1>
}
})
t.assertEqual('/?a=b', url)
t.assertNull(data)
t.assertUndefined(data)
},

testBeforeSendCanChangeAndAddHeaders: function(t) {
testBeforeSendCannotChangeOrAddHeadersViaObject: function(t) {
var xhr = $.ajax({
headers: { a:'b', c:'d' },
beforeSend: function(x, s) {
s.headers['a'] = 'B'
s.headers['e'] = 'f'
}
})
t.assert(xhr.headers.some(matchHeader('a', 'b')))
t.assert(xhr.headers.some(matchHeader('c', 'd')))
t.refute(xhr.headers.some(matchHeader('e', 'f')))
},

testBeforeSendCanChangeAndAddHeadersViaXhr: function(t) {
var xhr = $.ajax({
headers: { a:'b', c:'d' },
beforeSend: function(x, s) {
x.setRequestHeader('a', 'B')
x.setRequestHeader('e', 'f')
}
})
t.assert(xhr.headers.some(matchHeader('a', 'B')))
t.assert(xhr.headers.some(matchHeader('c', 'd')))
t.assert(xhr.headers.some(matchHeader('e', 'f')))
Expand Down

0 comments on commit 0604621

Please sign in to comment.