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: login redirect was not handling hash #553

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

andrewwylde
Copy link
Contributor

When passing a route to the next callback with an object that looks like {path: string}, the hash is not included. When the hash is included in the redirect, then, it would continually compare the path to the redirectPath, and they would not match (one would be missing the hash). Therefore it would cause an infinite redirect loop causing a crash/OOM.

By checking for a hash and passing it along to the next callback, it allows correct comparison, and redirects properly.

@andrewwylde andrewwylde requested a review from a team as a code owner September 4, 2024 19:20
Mierenga
Mierenga previously approved these changes Sep 4, 2024
Copy link
Contributor

@Mierenga Mierenga left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 52 to 53
const hash = `${redirectToPath.split('#')[1]}`
const redirectObject = { path: redirectToPath, hash: hash ? `#${hash}` : undefined }
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this could be optimized if done inside the if block

@andrewwylde andrewwylde merged commit 58378c1 into main Sep 5, 2024
9 checks passed
@andrewwylde andrewwylde deleted the fix/login-redirect-hash branch September 5, 2024 14:51
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.

2 participants