Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Sends a X-Requested-With header #26

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants

pvcnt commented Jul 25, 2012

Many Javascript frameworks sends the "X-Requested-With: XMLHttpRequest" header with their ajax requests. It mainly allows server side frameworks to detect ajax requests. It would be great for interoperability to have Javelin send this header too.

Thanks for your feedback!

Owner

epriestley commented Jul 25, 2012

We include an __ajax__ key instead, can you use that?

https://github.com/Vincent-P/javelin/blob/2ec25e4500e678e00920e51092bbd0eefe1fcf57/src/lib/Request.js#L120

We do this instead of a header because it's easier to debug with a browser, since you can just add ?__ajax__=true to a request and see how the server would respond to an Ajax request.

pvcnt commented Jul 25, 2012

The point is that I'm using Javelin with Symfony and the framework comes with a Request::isXMLHttpRequest() method based on this header. I can still verify the ajax jey, it would just be a more practical integration.

Owner

epriestley commented Jul 25, 2012

You can install a global listener for the 'open' event this class emits and add the header there for your install.

Since there are reasonable workarounds, I'm hesitant to accept this into the upstream because headers can have a performance cost way out of line with their apparent size: in particular, there is a performance dropoff between the whole request fitting into one packet and being split into two, and the more unnecessary stuff we add to request headers the more likely we are to trigger that limit. See https://developers.google.com/speed/docs/best-practices/request

Owner

epriestley commented Jul 25, 2012

In Phabricator, we use this snippet to add an 'X-Phabricator-CSRF' header to requests:

// Additionally, add the CSRF token as an HTTP header to every AJAX request.
JX.Request.listen('open', function(r) {
  r.getTransport().setRequestHeader(config.header, current_token);
});

pvcnt commented Jul 25, 2012

That seem's a reasonable workaround for me. Thanks for the input!

Owner

epriestley commented Jul 25, 2012

Cool. Let us know if you run into anything else.

@epriestley epriestley closed this Jul 25, 2012

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