From 06046213a38407958108901db1f44f775bff8850 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mislav=20Marohni=C4=87?= <mislav.marohnic@gmail.com>
Date: Thu, 12 Dec 2013 14:39:32 +0100
Subject: [PATCH] Fix `xhr.setRequestHeader()` in Ajax `beforeSend`

I broke this in 23d1879d55e89b4a375aaa1aefd71ff2582deabc, 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
---
 src/ajax.js    | 21 +++++++++++++--------
 test/ajax.html | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/src/ajax.js b/src/ajax.js
index 0a106ced6..d71ab31cb 100644
--- a/src/ajax.js
+++ b/src/ajax.js
@@ -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){
@@ -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) {
@@ -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
diff --git a/test/ajax.html b/test/ajax.html
index 6dbd0d127..7fc2d5606 100644
--- a/test/ajax.html
+++ b/test/ajax.html
@@ -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){
@@ -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) {
@@ -1088,10 +1103,10 @@ <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) {
@@ -1099,6 +1114,19 @@ <h1>Zepto Ajax unit tests</h1>
             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')))