Opened 9 years ago

Closed 9 years ago

#6609 defect closed fixed (fixed)

twisted.cred.credentials.DigestCredentialFactory fails to decode responses for uri which include ","

Reported by: Jean-Paul Calderone Owned by: Jean-Paul Calderone
Priority: normal Milestone:
Component: core Keywords:
Cc: jknight Branch: branches/digest-with-comma-6609
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun


The parser is a little bit too naive. It explodes like this if "," occurs in the URI (or any other field):

  File "twisted/web/_auth/", line 114, in _authorizedResource
    credentials = factory.decode(respString, request)
  File "twisted/web/_auth/", line 54, in decode
  File "twisted/cred/", line 345, in decode
    for (k, v) in [p.split('=', 1) for p in parts]:
exceptions.ValueError: need more than 1 value to unpack

Change History (7)

comment:1 Changed 9 years ago by DefaultCC Plugin

Cc: jknight added

comment:2 Changed 9 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/digest-with-comma-6609

(In [39010]) Branching to 'digest-with-comma-6609'

comment:3 Changed 9 years ago by Jean-Paul Calderone

Component: webcore

Ha ha, all this code is in core I guess.

comment:4 Changed 9 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted

comment:5 Changed 9 years ago by Glyph

This was a duplicate of #6445, but as noted there, I closed that one because this one has a branch in review.

comment:6 Changed 9 years ago by Glyph

Keywords: review removed
Owner: set to Jean-Paul Calderone

Thanks for fixing this. It's bothered me in the past and I am sad I didn't write a patch sooner.

  1. A couple of lines in the change are too long.
  2. As it happens, the only other comment I've got is that the inline regex could be pulled out to a call to re.compile; that would fix the line-length issue as a consequence, be a little more readable, (and also speeeeeeeed).
  3. I'm going to start mentioning this every time it happens, when reviewing tickets from core Twisted people, at least; the Windows build for this branch hit the ENOSPC bug. It'd be nice to finally fix that so we get reliable builds. (Obviously, I hope, not part of this ticket.)

Please land at your discretion.

comment:7 Changed 9 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [39015]) Merge digest-with-comma-6609

Author: exarkun Reviewer: glyph Fixes: #6609

Fix the digest authentication response parser to handle values which include ",".

Note: See TracTickets for help on using tickets.