Opened 7 years ago

Closed 7 years ago

#7060 closed (invalid)

Tutorial skips over a race condition

Reported by: donald.ball@… Owned by: nobody
Component: Documentation Version: master
Severity: Keywords:
Cc: donald.ball@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In the portion of the tutorial where you introduce the template system, you don't make any mention of how to escape output to avoid XSS and similar attacks. Later, when you show code to increment the vote count, you have a classic race condition.

Both of these potential bugs could be considered advanced topics, not suitable for an introductory tutorial, but I am of the view that you should show folks the correct way to do things from the get-to.

Change History (6)

comment:1 follow-up: Changed 7 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

1: All output is HTML escaped by default, so XSS is not a worry unless people explicitly mark data as safe.

2: Way more details on the race condition please.

comment:2 in reply to: ↑ 1 Changed 7 years ago by anonymous

Replying to anonymous:

1: All output is HTML escaped by default, so XSS is not a worry unless people explicitly mark data as safe.

2: Way more details on the race condition please.

Forgive me, the auto-escaping is introduced in the SVN version.

comment:3 Changed 7 years ago by Donald Ball <donald.ball@…>

On the race condition:

def vote(request, poll_id):
    p = get_object_or_404(Poll, pk=poll_id)
    try:
        selected_choice = p.choice_set.get(pk=request.POST['choice'])
    except (KeyError, Choice.DoesNotExist):
        # Redisplay the poll voting form.
        return render_to_response('polls/detail.html', {
            'poll': p,
            'error_message': "You didn't select a choice.",
        })
    else:
        selected_choice.votes += 1
        selected_choice.save()
        # Always return an HttpResponseRedirect after successfully dealing
        # with POST data. This prevents data from being posted twice if a
        # user hits the Back button.
        return HttpResponseRedirect('/polls/%s/results/' % p.id)

Two requests hit this section of code at the same time. When the race condition occurs, one request gets the number of votes as X, then the second (again, as X), then the first sets the number of votes to X+1, then the second does the same, so one vote is lost. Presumably, one would protect the critical section using a transaction.

Good to know output escaping is the default in the latest version.

comment:4 Changed 7 years ago by ikelly

Race conditions like that are best solved at the database level by raising the transaction isolation level. Ticket #6064 would provide a relatively straightforward way to do this, but it hasn't yet been committed. I don't know if that issue should be brought up in the tutorial until it has.

comment:5 Changed 7 years ago by Simon Greenhill

  • Summary changed from Tutorial skips over two bugs: output escaping and a race condition to Tutorial skips over a race condition
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 0.96 to SVN

Changing the ticket to focus on the race condition, as the autoescape seems to be fixed.

comment:6 Changed 7 years ago by ubernostrum

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

Since the focus of the tutorial is to introduce the basics of Django's components and how they work, rather than to exhaustively document how to write web applications, this feels out of scope.

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