Fix unquote IndexError on empty string input - #3771
Conversation
`unquote('')` raises `IndexError` because it accesses `value[0]`
without checking the string length first. This can occur when parsing
digest auth WWW-Authenticate headers containing parameters with empty
unquoted values (e.g. `realm=` instead of `realm=""`).
|
|
||
| def unquote(value: str) -> str: | ||
| return value[1:-1] if value[0] == value[-1] == '"' else value | ||
| if len(value) >= 2 and value[0] == value[-1] == '"': |
There was a problem hiding this comment.
| if len(value) >= 2 and value[0] == value[-1] == '"': | |
| if value and value[0] == value[-1] == '"': |
to avoid a global lookup, a function call and a comparison...?
unquote() in _utils.py accesses value[0] and value[-1] without checking string length first. This raises IndexError when called with an empty string, which can occur when parsing Digest auth WWW-Authenticate headers containing parameters with empty unquoted values (e.g. realm= instead of realm=""). Added a len(value) >= 2 guard and comprehensive test coverage for empty strings, single characters, quoted values, and unquoted values. Note: encode#3771 addresses the same issue. This PR adds the regression tests that are missing there. Happy to close in favor of that PR if tests are added. Powered by codepo8 — tribal knowledge extraction identified this as a guard-level issue: "Check string length before accessing index 0 in unquote() utilities because empty string inputs cause an IndexError."
CAOShurong
left a comment
CAOShurong
left a comment
There was a problem hiding this comment.
Verified independently (no run, no claim). The PR was force-pushed after I first fetched it; I re-fetched and confirmed the current head before reviewing — head 55ead904eff7376ccbc7f23e93be19117c642441.
Root cause: unquote() was value[1:-1] if value[0] == value[-1] == '"' else value. The truth condition value[0] == value[-1] == '"' accesses value[0] first, so for an empty string unquote('') raised IndexError: string index out of range before the quote-strip slice could ever run. This fires when parsing a WWW-Authenticate digest header carrying an empty unquoted value (e.g. realm=).
Behavior check at the current head:
unquote('')returns''(no crash).unquote('"a"')returns'a'(quote-stripping preserved).unquote('raw')returns'raw'(unchanged).
On base ae1b9f6 the same unquote('') raises IndexError: string index out of range — RED→GREEN confirmed. The fix guards with len(value) >= 2 before the quote check, which is the minimal correct change.
One non-blocking suggestion: the PR has no regression test for the empty-string case. A tiny assertion (assert unquote('') == '') in tests/test_utils.py would lock the behavior in. The change itself is correct and low-risk, so approving as-is; happy to see a follow-up test added.
(AI assistance used to collect verification evidence; the fix is the author's.)
3 participants
unquote('')in_utils.pyraisesIndexErrorbecause it accessesvalue[0]without checking string length first.This can occur when parsing digest auth
WWW-Authenticateheaders containing parameters with empty unquoted values (e.g.realm=instead ofrealm="").