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

fix: update redirect logic to also update path #4055

Merged
merged 2 commits into from
Apr 1, 2024
Merged

Conversation

CaerusKaru
Copy link
Contributor

@CaerusKaru CaerusKaru commented Mar 18, 2022

When a 307 redirect comes in, it contains the new, full URL for the location change.
The current logic respects this for the Host header that gets sent, but it does not
then update the path, leading to possibly invalid redirects. The new logic pulls the
path out of the endpoint and updates the outgoing request.

Checklist
  • npm run test passes

Ref: Ticket ID V544628989

When a 307 redirect comes in, it contains the new, full URL for the location change.
The current logic respects this for the `Host` header that gets sent, but it does not
then update the path, leading to possibly invalid redirects. The new logic pulls the
path out of the endpoint and updates the outgoing request.
@CaerusKaru CaerusKaru requested a review from a team as a code owner March 18, 2022 21:08
Copy link
Contributor

@AllanZhengYP AllanZhengYP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. It makes sense to me. Can you add a changelog so we can release it?

@CaerusKaru
Copy link
Contributor Author

@AllanZhengYP Added the CHANGELOG entry following the contribution guidelines, please let me know if you need anything else. The commit name is weird because I did it through GitHub Dev online, so hopefully you can squash it before release.

@github-actions
Copy link

Greetings! We’re closing this issue because it has been open a long time and hasn’t been updated in a while and may not be getting the attention it deserves. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to comment or open a new issue.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Mar 30, 2023
@CaerusKaru
Copy link
Contributor Author

Please do not close. Internal issue that should be addressed.

@github-actions github-actions bot removed the closing-soon This issue will automatically close in 4 days unless further comments are made. label Mar 31, 2023
@kuhe kuhe merged commit 8a82af5 into aws:master Apr 1, 2024
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.

3 participants