-
Notifications
You must be signed in to change notification settings - Fork 689
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 replacing substring of params in search query #1226
base: master
Are you sure you want to change the base?
Fix replacing substring of params in search query #1226
Conversation
.replace(/^\?&/, '?') | ||
.replace(/&$/, '') | ||
.replace(/^\?$/, '') | ||
.replace(/&+/g, '&') | ||
.replace(/\?&/, '?') | ||
.replace(/\?$/, '') + |
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.
The new way of matching will result in less situations that can break the format of the URL that's why these replace are not needed anymore.
@GeorgeHarbers I think your recent updates make it so your private AWS and GitHub Workflow code became part of the PR to this upstream library, I presume that was not intended? I don't see any immediate secrets being leaked when scanning the changes in the PR, but I recommend double checking regardless. |
Correct, my bad. I will restore it. |
a54ea92
to
078415b
Compare
The tryLoginCodeFlow in the oAuth-service.ts clears the hash after login. The code removes the following parameters from the search query: code, scope, state and session_state. The regex that was used to remove these params was breaking different params that end on the same string.
e.g. if the param "zipcode=xxx" was used in the search query the string "code=xxx" was removed and the URL was not valid anymore.
"http://localhost:4300?zipcode=10115&consumption=2500"
Was resulting in
"http://localhost:4300?zip&consumption=2500"
In this PR I changed the regex to only remove the parameter if it's a total match.