Skip to content

Comments

Implement RFC 7231 compliant relative URI and fragment handling in redirects#13040

Closed
zanieb wants to merge 1 commit intozb/debug-redirectfrom
zb/redirect-rfc
Closed

Implement RFC 7231 compliant relative URI and fragment handling in redirects#13040
zanieb wants to merge 1 commit intozb/debug-redirectfrom
zb/redirect-rfc

Conversation

@zanieb
Copy link
Member

@zanieb zanieb commented Apr 22, 2025 •
edited
Loading

#13037 prompted me to start speculating on possible failure paths, and I found some cases we're not implementing the specification here?

A couple things

  • I'm not sure if I'm joining to the base URL correctly. The RFC points to another

    The field value consists of a single URI-reference. When it has the
    form of a relative reference ([RFC3986], Section 4.2), the final
    value is computed by resolving it against the effective request URI
    ([RFC3986], Section 5).

  • We need test coverage here

@zanieb zanieb added the bug Something isn't working label Apr 22, 2025
This was referenced Apr 22, 2025
Merged
Closed
zanieb added a commit that referenced this pull request Apr 22, 2025
@zanieb
Revert "Properly handle authentication for 302 redirect URLs" (#13041)
5349532 
This reverts commit 17ed789 / #12920 

There's a regression reported in
#13037 and it looks like we're
missing some important parts per #13040
@zanieb
Copy link
Member Author

zanieb commented Apr 22, 2025

Extends #13038 requires restoring #13041

@jtfmumm jtfmumm mentioned this pull request Apr 22, 2025
Merged
@jtfmumm
Copy link
Contributor

jtfmumm commented Apr 24, 2025

Closing in favor of #13050

@jtfmumm jtfmumm closed this Apr 24, 2025
jtfmumm added a commit that referenced this pull request Apr 28, 2025
@jtfmumm @zanieb
Implement RFC 7231 compliant relative URI and fragment handling in re…
4ee4a88 
…directs (#13050)

This PR restores #13041 and integrates two PRs from @zanieb:
* #13038
* #13040

It also adds tests for relative URI and fragment handling.

Closes #13037.

---------

Co-authored-by: Zanie Blue <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zanieb @jtfmumm