Skip to content

Comments

Fix TypeError crash in SEP-7 replace parameter parsing#227

Open
CassioMG wants to merge 8 commits intorelease/1.10.0from
bugfix/replace-crash
Open

Fix TypeError crash in SEP-7 replace parameter parsing#227
CassioMG wants to merge 8 commits intorelease/1.10.0from
bugfix/replace-crash

Conversation

@CassioMG
Copy link
Contributor

@CassioMG CassioMG commented Feb 20, 2026

Summary

  • Fix unhandled TypeError in sep7ReplacementsFromString when the replace parameter string is missing the ; hints delimiter
  • Add spec-compliant validation that rejects replace values with unbalanced reference identifiers
  • Add test coverage for invalid replacement strings and edge cases

Problem

sep7ReplacementsFromString calls .split() on hintsString without checking if it exists. When a replace parameter value lacks the ; delimiter (e.g. sourceAccount:X instead of sourceAccount:X;X:some hint), the destructuring on line 165 leaves hintsString as undefined, and the immediate .split() call on line 166 throws:

TypeError: Cannot read properties of undefined (reading 'split')

Fix

Per the SEP-7 spec:

The replace value is considered invalid unless the reference_identifiers are balanced on both sides of the separating semicolon.

The fix:

  1. Guards against undefined on hintsString to prevent the TypeError
  2. Validates that reference identifiers are balanced between the txrep side and the hints side
  3. Throws Sep7InvalidUriError with a descriptive message when identifiers are unbalanced, instead of silently accepting invalid input

Tests added

  • Missing hints section (no ; delimiter) — single and multiple fields — throws Sep7InvalidUriError
  • Unbalanced identifiers (spec example: sourceAccount:X;Y:The account) — throws Sep7InvalidUriError
  • undefined and empty string input — returns []
  • End-to-end: Sep7Tx.getReplacements() with a URI whose replace param has no hints — throws Sep7InvalidUriError
  • Existing tests for valid replacement strings continue to pass

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request fixes a TypeError crash in the SEP-7 replacement parameter parsing when the hints delimiter (;) is missing from the replace parameter string. The crash occurred because the code attempted to call .split() on an undefined value when no hints section was present in the replacement string format.

Changes:

  • Added a guard clause to check for hintsString existence before calling .split() to prevent the TypeError crash
  • Added comprehensive test coverage for replacements without hints (multiple fields, single field, undefined/empty input)
  • Added end-to-end test verifying Sep7Tx.getReplacements() handles URIs with missing hints section

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
@stellar/typescript-wallet-sdk/src/walletSdk/Uri/sep7Parser.ts Added conditional check for hintsString before processing hints to prevent TypeError when the semicolon delimiter is missing
@stellar/typescript-wallet-sdk/test/sep7.test.ts Added 4 new test cases covering replacements without hints, including edge cases and end-to-end validation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Per the SEP-7 spec, reference identifiers must be balanced on both
sides of the semicolon delimiter. Previously, a replace value without
a semicolon caused a TypeError crash. Now, sep7ReplacementsFromString
validates that all identifiers match between the txrep and hints
sections, throwing Sep7InvalidUriError on invalid input.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Extract hardcoded 4096 length limit to URI_REPLACE_MAX_LENGTH constant
- Validate hint entries have both identifier and hint value
- Validate txrep entries have non-empty path values
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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