Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#15284 closed (fixed)

CSRF/Ajax/jQuery example could break other site JS

Reported by: LukeMaurer Owned by: nobody
Component: Documentation Version: 1.2
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The example given for setting up jQuery to pass the CSRF token in Ajax requests (http://docs.djangoproject.com/en/1.2/ref/contrib/csrf/#ajax) has a problem: If the site already uses $.ajaxSetup({beforeSend: ...}) for other processing, this code will clobber that other handler (or vice versa).

The jQuery docs suggest using $.ajaxSend() and friends for setting up global callbacks, rather than $.ajaxSetup(), I suspect for this reason.

At any rate, using jQuery's suggestion would make the example look like:

$.ajaxSend(function(event, xhr, settings) {
    function getCookie(name) {
        var cookieValue = null;
        if (document.cookie && document.cookie != '') {
            var cookies = document.cookie.split(';');
            for (var i = 0; i < cookies.length; i++) {
                var cookie = jQuery.trim(cookies[i]);
                // Does this cookie string begin with the name we want?
                if (cookie.substring(0, name.length + 1) == (name + '=')) {
                    cookieValue = decodeURIComponent(cookie.substring(name.length + 1));
                    break;
                }
            }
        }
        return cookieValue;
    }
    if (!(/^http:.*/.test(settings.url) || /^https:.*/.test(settings.url))) {
        // Only send the token to relative URLs i.e. locally.
        xhr.setRequestHeader("X-CSRFToken", getCookie('csrftoken'));
    }
});

(where only the outer function call and the signature of the callback have changed)

Change History (7)

comment:1 Changed 4 years ago by gabrielhurley

  • milestone set to 1.3
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 4 years ago by lukeplant

I was aware of that problem with beforeSend, but I hadn't realised that you could actually alter the XHR request in ajaxSend. This is potentially fragile (the docs only give examples of reading the XHR object, not altering it), but it seems unlikely to break, so I agree that we should change this.

comment:3 Changed 4 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from new to closed

In [15515]:

Fixed #15284 - improved example jQuery code for adding X-CSRF-Token

Using the ajaxSend event is better than beforeSend, because the beforeSend
callback can have only one value, which makes it painful if it is needed by
multiple bits of javascript.

Thanks to LukeMaurer for report and initial patch.

comment:4 Changed 4 years ago by lukeplant

In [15517]:

[1.2.X] Fixed #15284 - improved example jQuery code for adding X-CSRF-Token

Using the ajaxSend event is better than beforeSend, because the beforeSend
callback can have only one value, which makes it painful if it is needed by
multiple bits of javascript.

Thanks to LukeMaurer for report and initial patch.

Backport of [15515] from trunk.

comment:5 Changed 4 years ago by lukeplant

In [15518]:

[1.1.X] Fixed #15284 - improved example jQuery code for adding X-CSRF-Token

Using the ajaxSend event is better than beforeSend, because the beforeSend
callback can have only one value, which makes it painful if it is needed by
multiple bits of javascript.

Thanks to LukeMaurer for report and initial patch.

Backport of [15515] from trunk.

This is backported to 1.1.X because it really belongs with security patch [15466]

comment:6 Changed 4 years ago by schinckel

The example code is broken: $.ajaxSend does not exist.

I use instead:

$(document).ajaxSetup(…)

[Then looked at actual documentation. *headdesk*]

Last edited 4 years ago by schinckel (previous) (diff)

comment:7 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top