Use built-in APIs to parse URLs#143
Conversation
e72ba04 to
072ac27
Compare
Closes keycloak#56 Signed-off-by: Jon Koops <[email protected]>
072ac27 to
fc42a40
Compare
There was a problem hiding this comment.
Pull Request Overview
Refactors callback URL handling to use the browser’s built-in URL and URLSearchParams APIs, fixes a double-encoding bug, and omits the hash fragment from redirect URIs.
- Updates tests to expect decoded error descriptions
- Rewrites
#parseCallbackUrland#parseCallbackParamsto useURL/URLSearchParams - Introduces
stripHashhelper and removes manualencodeURIComponentcalls
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/tests/implicit-flow.spec.ts | Adjusted expected error_description strings to use spaces instead of + |
| lib/keycloak.js | Refactored callback parsing, replaced manual string slicing with URL APIs, added stripHash, and updated redirect URI handling |
Comments suppressed due to low confidence (2)
lib/keycloak.js:2166
- Add unit tests for the new stripHash function to verify it correctly removes the hash fragment from various URL inputs.
function stripHash(url) {
lib/keycloak.js:1059
- Consider adding tests for parseCallbackUrl to ensure query parameters are properly preserved or stripped according to supportedParams and that redirectUri is computed as expected.
if (this.responseMode === 'query' && url.searchParams.size > 0) {
|
@twobiers since you are also working on this area in another PR, could I ask you to put in a review for this one? |
|
I am merging this change in, thanks for the reviews! |
|
Hello @jonkoops , |
|
Probably in the next couple of weeks or so |
|
Thank you. We look forward to this release. |
|
Hello @jonkoops, I'd like to follow up with you regarding the availability of a new release of Keycloak JS. The last release was in February. Currently, we're stuck deploying our applications, and this PR seems to be able to correct our usage. Thank you for your feedback. Sincerely, |
|
Working on landing the final bits now, after which I will ask @pskopek to do a release. |
Re-writes the callback URL parsing to use standard APIs such as
URLSearchParams. This also fixes a bug where some data would be double encoded, as well as improving spec compliance by omitting the hash from the redirect URI.Closes #56