Skip to content

Comments

[DTINAPPXO-3265][DTINAPPXO-3391] Resume Flow fix: Extract query-style params embedded in the hash fragment#2599

Merged
ravishekhar merged 2 commits intomainfrom
fix/hash-param-resume-flow-fix
Feb 25, 2026
Merged

[DTINAPPXO-3265][DTINAPPXO-3391] Resume Flow fix: Extract query-style params embedded in the hash fragment#2599
ravishekhar merged 2 commits intomainfrom
fix/hash-param-resume-flow-fix

Conversation

@naderchehab
Copy link
Contributor

@naderchehab naderchehab commented Feb 20, 2026

Description

Two related fixes for the app-switch resume flow when a merchant's return_url contains a hash fragment:

  1. Hash fragment param extraction: When a merchant's return_url contains a hash fragment (e.g. /checkout/#payment), PayPal params (token, PayerID) get appended inside the hash as query-style params (e.g. #payment?token=ABC&PayerID=XYZ) in the web fallback flow.
    This enters into the resume flow which only recognizes hashes starting with a known action (onApprove, onCancel, onError) and silently dropped params embedded in non-action hash fragments. This change adds a getParamsFromHashFragment() helper that extracts query-style params from the hash, and updates getAppSwitchParamsWebFallback() to use it as a fallback when no PayPal params are found in the query string.

  2. Multiple ? parsing fix: When returning from PayPal app with a merchant's return_url which itself contains a ? (e.g. #hash?query=param), the native app constructs the return URL as #onApprove?token=...&hash?query=param. Using String.split("?") produced 3 segments and destructuring only captured the first two, silently dropping everything after the second ?. Replaced with indexOf + slice to only split on the first ?, consistent with how getParamsFromHashFragment() already handles the same parsing.

Reproduction Steps

  1. Set a merchant return_url with a hash fragment, e.g. https://example.com/checkout/#payment or
    https://example.com#hash?query=param
  2. Complete a PayPal checkout via app switch with fallback to web (e.g. no app installed)
  3. Return URL becomes something like: https://example.com/checkout/#payment?token=3JB49854KA&PayerID=BF44HY8FD
  4. Before fix: getAppSwitchResumeParams() returns null, checkout does not resume
  5. After fix: Params are extracted from the hash fragment and the checkout resumes correctly

For the native app case with a merchant hash containing ?:

  1. Set a merchant return_url like https://example.com#hash?query=param
  2. Complete a PayPal checkout via native app switch
  3. Return URL becomes: https://example.com#onApprove?token=...&button_session_id=...&hash?query=param
  4. Before fix: split("?") drops the third segment — works by luck today but is fragile
  5. After fix: slice() preserves the full query string after the first ?

Dependent Changes

None

@naderchehab naderchehab changed the title Resume Flow fix: Extract query-style params embedded in the hash fragment [DTINAPPXO-3265] Resume Flow fix: Extract query-style params embedded in the hash fragment Feb 20, 2026
@naderchehab naderchehab force-pushed the fix/hash-param-resume-flow-fix branch 2 times, most recently from 314c53a to 690fcee Compare February 21, 2026 00:45
@codecov
Copy link

codecov bot commented Feb 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.26%. Comparing base (a1c952c) to head (690fcee).
⚠️ Report is 89 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2599      +/-   ##
==========================================
- Coverage   54.51%   54.26%   -0.26%     
==========================================
  Files         162      164       +2     
  Lines       16365    17712    +1347     
  Branches     1083     1218     +135     
==========================================
+ Hits         8922     9611     +689     
- Misses       7330     7975     +645     
- Partials      113      126      +13     
Flag Coverage Δ
jest 28.61% <0.00%> (+1.12%) ⬆️
karma 56.09% <35.71%> (+0.42%) ⬆️
vitest 49.83% <100.00%> (-0.19%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@naderchehab naderchehab marked this pull request as ready for review February 21, 2026 01:33
@naderchehab naderchehab requested a review from a team as a code owner February 21, 2026 01:33
@naderchehab naderchehab changed the title [DTINAPPXO-3265] Resume Flow fix: Extract query-style params embedded in the hash fragment [DTINAPPXO-3265][DTINAPPXO-3391] Resume Flow fix: Extract query-style params embedded in the hash fragment Feb 21, 2026
searchParams.approval_token_id ||
searchParams.approval_session_id
? searchParams
: { ...getParamsFromHashFragment(), ...searchParams };
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential false positive for merchant hash params

This new fallback to getParamsFromHashFragment() introduces a risk: if a merchant's SPA uses token in their own hash fragment (e.g. https://merchant.com/shop#products?token=my-cart-token), this will now be detected as a resume flow and trigger onCancel.

On main this was safe — the web fallback only read window.location.search, so merchant hash params were ignored. With this change, any page with token= or a vault param in the hash will match.
Can we explore an additional guard here for this scenaio?

Copy link
Contributor Author

@naderchehab naderchehab Feb 24, 2026

Choose a reason for hiding this comment

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

@nikrom17 This isn't a new risk introduced by this change - the same issue already exists with query params today. If a merchant uses a query param named token, it will incorrectly trigger the cancel flow. This change brings hash params to parity with query params. If we want to address the false positive problem, it should be tackled holistically for both query and hash params. (I think we should, but not part of this PR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a ticket to handle this issue DTINAPPXO-3434

@naderchehab naderchehab force-pushed the fix/hash-param-resume-flow-fix branch from 9e7eb9c to b05f89f Compare February 24, 2026 20:11
@naderchehab naderchehab requested a review from nikrom17 February 24, 2026 22:39
@ravishekhar ravishekhar merged commit f0fc4d0 into main Feb 25, 2026
5 checks passed
@ravishekhar ravishekhar deleted the fix/hash-param-resume-flow-fix branch February 25, 2026 17:17
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.

4 participants