-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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(ui): Allow Trailing Slashes #19093 #19389
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Henry Kim <[email protected]>
🔴 Preview Environment stopped on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
✅ Preview Environment created on Bunnyshell but will not be auto-deployedSee: Environment Details Available commands (reply to this comment):
|
/bns:deploy |
'/user-info': {component: userInfo.component}, | ||
'/help': {component: help.component}, | ||
'/pkce/verify': {component: PKCEVerification, noLayout: true} | ||
// for removing trailing slash, use regex (/*)? end of the path. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These regexes look too permissive. I've tried in Python and '/login(/*)?' would match anything starting with /login
and things like /login//
would be a part of the match. Not sure that's intended. Also, why isn't there an update to the logic checking the path? The existing code looks like it doesn't expect the regex but rather a regular string.
If you are on the
http://localhost:4000/applications/
and visiting an application page by searching applications, the empty page is shown.and other redirection including end of trailing slash are too.
to solve above problem, trailing slashes should be allowed.
My first solution was to fix in argoproj/argo-ui#565. but argo-ui has been being in deprecation as far as I know. thus I will fix it by here.
Checklist:
fixes #19093