Skip to content

refactor(oauth2-redirect): parse query with URLSearchParams#10945

Open
MysteryBlokHed wants to merge 1 commit into
swagger-api:masterfrom
MysteryBlokHed:improve-oauth-redirect-parsing
Open

refactor(oauth2-redirect): parse query with URLSearchParams#10945
MysteryBlokHed wants to merge 1 commit into
swagger-api:masterfrom
MysteryBlokHed:improve-oauth-redirect-parsing

Conversation

@MysteryBlokHed

Copy link
Copy Markdown

Description

Replaces the query parameter parser that used splitting and concatenation with URLSearchParams. This makes the parsing more correct and avoids issues if JSON-significant characters appear in the query params.

I didn't touch the files in dist/ because I'm not fully sure how they're generated.

Technically this is not strictly a refactor since edge cases are now handled correctly, but the behaviour is the same on expected input.

Motivation and Context

The old parser built a fake JSON object literal from the query string and broke on any value containing JSON-significant characters (like ", \, {, }). The standard URLSearchParams API is reliable and preserves the existing output shape (a plain object passed through as the token).

How Has This Been Tested?

Verified parsing equivalence in Node (Object.fromEntries(new URLSearchParams(...)) for code/token/error query and hash forms, empty input, and a=b=c)

Screenshots (if appropriate):

N/A

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

Replaces the query parameter parser that used splitting and
concatenation with `URLSearchParams`. This makes the parsing more
correct and avoids issues if JSON-significant characters appear in the
query params.

I didn't touch the files in `dist/` because I'm not fully sure how
they're generated.
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.

1 participant