Problem with ajaxBeforeSend and setRequestHeader()in Zepto 1.1.1 #878

Closed
alvinchow86 opened this Issue Dec 9, 2013 · 5 comments

Comments

Projects
None yet
2 participants
@alvinchow86

I upgraded to the latest Zepto (1.1.1), was previously using 1.03. I have some code that adds a CSRF token header to every AJAX request that is now showing this error (running Chrome 31, OS X)

Uncaught InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': the object's state must be OPENED. 

Code

 $(document).on('ajaxSend', function(e, xhr, options) {
    var csrfToken = getCsrfToken();
    if (csrfToken) {
      if (!/^(GET|HEAD|OPTIONS|TRACE)$/i.test(options.type)) {
        xhr.setRequestHeader('X-CSRFToken', csrfToken);
      }
    }
  };);

Any idea what's going on here? This code worked fine in 1.0.3. Is there some other way to add AJAX request headers that will work?

@alvinchow86

This comment has been minimized.

Show comment Hide comment
@alvinchow86

alvinchow86 Dec 9, 2013

Looked at the source code (src/ajax.js), it seems like my issue is because ajaxBeforeSend() is called BEFORE xhr.open() in the code block below. In previous version it was called after. Moving to that block to after xhr.open() fixes my problem, but I don't know if there was some other reason it was moved in the first place.

  if (ajaxBeforeSend(xhr, settings) === false) {
      xhr.abort()
      ajaxError(null, 'abort', xhr, settings, deferred)
      return xhr
    }

    if (settings.xhrFields) for (name in settings.xhrFields) xhr[name] = settings.xhrFields[name]

    var async = 'async' in settings ? settings.async : true
    xhr.open(settings.type, settings.url, async, settings.username, settings.password)

Looked at the source code (src/ajax.js), it seems like my issue is because ajaxBeforeSend() is called BEFORE xhr.open() in the code block below. In previous version it was called after. Moving to that block to after xhr.open() fixes my problem, but I don't know if there was some other reason it was moved in the first place.

  if (ajaxBeforeSend(xhr, settings) === false) {
      xhr.abort()
      ajaxError(null, 'abort', xhr, settings, deferred)
      return xhr
    }

    if (settings.xhrFields) for (name in settings.xhrFields) xhr[name] = settings.xhrFields[name]

    var async = 'async' in settings ? settings.async : true
    xhr.open(settings.type, settings.url, async, settings.username, settings.password)
@alvinchow86

This comment has been minimized.

Show comment Hide comment
@alvinchow86

alvinchow86 Dec 9, 2013

In the meantime, my workaround is to set the headers directly. It might be limited if you want to set headers conditionally/dynamically though. FWIW, official jQuery documentation does mention that 'beforeSend' should be able to set custom request headers.

  $.extend($.ajaxSettings, {
    headers: {
      'X-CSRFToken': myCSRFToken
    }
  });

In the meantime, my workaround is to set the headers directly. It might be limited if you want to set headers conditionally/dynamically though. FWIW, official jQuery documentation does mention that 'beforeSend' should be able to set custom request headers.

  $.extend($.ajaxSettings, {
    headers: {
      'X-CSRFToken': myCSRFToken
    }
  });
@mislav

This comment has been minimized.

Show comment Hide comment
@mislav

mislav Dec 10, 2013

Collaborator

This might be a regression I introduced in v1.1. I'll work on fixing that. In the meantime, use this as a workaround:

  $(document).on('ajaxSend', function(e, xhr, options) {
    var csrfToken = getCsrfToken();
    if (csrfToken) {
      if (!/^(GET|HEAD|OPTIONS|TRACE)$/i.test(options.type)) {
        options.headers['X-CSRFToken'] = csrfToken;
      }
    }
  });
Collaborator

mislav commented Dec 10, 2013

This might be a regression I introduced in v1.1. I'll work on fixing that. In the meantime, use this as a workaround:

  $(document).on('ajaxSend', function(e, xhr, options) {
    var csrfToken = getCsrfToken();
    if (csrfToken) {
      if (!/^(GET|HEAD|OPTIONS|TRACE)$/i.test(options.type)) {
        options.headers['X-CSRFToken'] = csrfToken;
      }
    }
  });
@mislav

This comment has been minimized.

Show comment Hide comment
@mislav

mislav Dec 10, 2013

Collaborator

And, I'm pretty sure the header's name (at least in Rails) is X-CSRF-Token (notice the final dash)

Collaborator

mislav commented Dec 10, 2013

And, I'm pretty sure the header's name (at least in Rails) is X-CSRF-Token (notice the final dash)

@alvinchow86

This comment has been minimized.

Show comment Hide comment
@alvinchow86

alvinchow86 Dec 10, 2013

Awesome, that workaround works for me, thank you.

(That token is actually correct for the web framework I'm using)

Awesome, that workaround works for me, thank you.

(That token is actually correct for the web framework I'm using)

mislav added a commit that referenced this issue Dec 12, 2013

Fix `xhr.setRequestHeader()` in Ajax `beforeSend`
This got broken in 23d1879, which
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)
- `xhr.setRequestHeader()` is available, although technically
  `xhr.open()` hasn't been called yet

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

Fixes #878

@mislav mislav closed this in 0604621 Dec 12, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment