Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't change relative location header on manual redirect #1105

Merged
merged 2 commits into from Jan 15, 2022

Conversation

tekwiz
Copy link
Member

@tekwiz tekwiz commented Feb 28, 2021

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • New feature
  • Other, please explain:

What changes did you make? (provide an overview)

This removes the code that changes the location header to an absolute URL in the case of a manual redirect request. While doing this is helpful in most cases, it is too destructive in cases where the original, actual location header is needed. See the situation described in #1086.

This change also documents the variation from the spec

This change reduces the total lines of code, thus reducing the overall test percentage for test coverage. To keep that check passing, I added some appropriate c8 ignores to unrelated code (see 52f864b)

Which issue (if any) does this pull request address?

Fixes #1086

Is there anything you'd like reviewers to know?

No

@tekwiz tekwiz requested review from bitinn and TimothyGu Feb 28, 2021
@tekwiz tekwiz force-pushed the fix/relative-manual-redirect branch from 8232b42 to 09df486 Compare Feb 28, 2021
@tekwiz tekwiz requested review from Richienb and gr2m and removed request for bitinn and TimothyGu Oct 7, 2021
@tekwiz tekwiz force-pushed the fix/relative-manual-redirect branch from 52f864b to 53ead47 Compare Oct 7, 2021
LinusU
LinusU approved these changes Nov 4, 2021
@LinusU
Copy link
Member

@LinusU LinusU commented Nov 4, 2021

@tekwiz I think that the linting issue has been fixed on the latest master, could you rebase please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants