Skip to content

Conversation

@tboy1337
Copy link

Summary

This PR refactors the URL fragment handling logic in SessionRedirectMixin to be more Pythonic and adds comprehensive test coverage for fragment preservation during HTTP redirects, as specified in RFC 7231 Section 7.1.2.

Changes Made

Code Refactoring

  • src/requests/sessions.py: Simplified fragment check from parsed.fragment == "" to not parsed.fragment for more idiomatic Python code
    • This change maintains the same behavior while being more concise and following Python best practices
    • The logic still correctly handles fragment preservation during redirects per RFC 7231 7.1.2

Test Coverage Enhancement

Added 5 new test cases in tests/test_requests.py to ensure robust fragment handling:

  1. test_fragment_preserved_when_redirect_has_no_fragment: Verifies that the original URL fragment is preserved when redirecting to a URL without a fragment
  2. test_fragment_replaced_when_redirect_has_new_fragment: Confirms that fragments are properly maintained through redirect chains
  3. test_fragment_with_no_initial_fragment: Ensures proper behavior when neither the original nor redirect URL contains a fragment
  4. test_fragment_multiple_redirects_preservation: Tests fragment preservation through multiple consecutive redirects (3+ hops)
  5. test_fragment_empty_string_handling: Validates correct handling of URLs ending with # (empty fragment string)

Why These Changes Matter

  • Code Quality: The refactored condition is more Pythonic and easier to read
  • Test Coverage: The new tests provide comprehensive coverage for various fragment scenarios, preventing regressions
  • RFC Compliance: Ensures requests properly implements RFC 7231 Section 7.1.2 for fragment preservation during redirects
  • Edge Cases: Covers important edge cases like empty fragments and multi-hop redirects

Testing

All new tests pass and verify the correct behavior of fragment handling across various redirect scenarios.

…ive tests for fragment preservation during redirects. This includes tests for scenarios with original fragments, new fragments, and multiple redirects, ensuring consistent behavior across various cases.
@tboy1337
Copy link
Author

tboy1337 commented Oct 22, 2025

I should add that this issue was found because of I think (I'm not 100% sure because everything was also typed according to mypy rules in the commit) pylint in #7054, if it was pylint that found this issue and it is a legitimate issue then it makes a strong case for the project to start using it imo.

@tboy1337 tboy1337 changed the title Improve URL fragment handling in redirects and add comprehensive test coverage Improve URL fragment handling in redirects Oct 22, 2025
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