Skip to content

Conversation

@tboy1337
Copy link

Summary

This PR fixes a bug in HTTPDigestAuth where semicolons in URL paths were being truncated from the digest authentication uri field, causing authentication failures for APIs that use semicolons as path delimiters.

Problem

When making authenticated requests to URLs containing semicolons in the path (e.g., MusicBrainz API: /api/collection/id1/releases/uuid1;uuid2;uuid3), the HTTP Digest Authentication implementation was not including the URL parameters (semicolon-separated values) in the uri field of the Authorization header. According to RFC 2616, the URI should include the full request-uri, including any parameters.

This caused authentication failures because the server-side digest calculation included the full path with semicolons, while the client-side calculation used a truncated path without them.

Solution

Modified src/requests/auth.py in the HTTPDigestAuth.build_digest_header() method to properly extract and include URL parameters (semicolons) when constructing the request-uri:

path = p_parsed.path or "/"
if p_parsed.params:
    path += f";{p_parsed.params}"
if p_parsed.query:
    path += f"?{p_parsed.query}"

The fix uses urlparse().params to correctly handle semicolon-separated parameters and includes them in the path before appending any query string.

Testing

Added comprehensive test coverage in tests/test_requests.py with three scenarios:

  1. Path with semicolons and query parameters: Verifies URLs like /path/id1;id2;id3?param=value are handled correctly
  2. Path with semicolons only: Ensures URLs like /path/id1;id2;id3 work without query strings
  3. Simple path: Confirms existing functionality remains intact for standard paths

Impact

Files Changed

  • src/requests/auth.py - 2 lines added
  • tests/test_requests.py - 39 lines added

… test to verify correct handling of URLs with semicolons, addressing issue psf#6990.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant