Skip to content

fix: [SDK-5331] reject blank identity strings - #66

Open
abdulraqeeb33 wants to merge 4 commits into
mainfrom
ar/sdk-5331
Open

abdulraqeeb33 wants to merge 4 commits into
mainfrom
ar/sdk-5331

Conversation

@abdulraqeeb33

@abdulraqeeb33 abdulraqeeb33 commented Sep 22, 2026 •

Copy link
Copy Markdown

Description

One Line Summary

Reject null and empty identity strings through one shared helper.

Details

Motivation

Fixes SDK-5331.

Null and empty strings were forwarded to the native plugin. Part of SDK-5327.

Scope

rejectNullOrEmpty and rejectNullOrEmptyKeys in src/helpers.ts reject null and empty for:

  • initialize(appId)
  • login(externalId)
  • addAlias / addAliases (label and id)
  • removeAlias / removeAliases
  • addEmail / removeEmail
  • addSms / removeSms
  • addTag / addTags (key only). An empty tag value is allowed. A null tag value is rejected.
  • removeTag / removeTags
  • addTrigger / addTriggers (key only). An empty trigger value is allowed. A null trigger value is rejected.
  • removeTrigger / removeTriggers
  • trackEvent(name)

Whitespace is still allowed. Public method signatures are unchanged.

setLanguage("") is not rejected. It is the reset to the device language, and there is no other reset path. null is still rejected.

Testing

Unit testing

vp test: 240 tests passed, coverage above the repo thresholds. Cases cover empty and null login, empty app id, empty tag keys, empty trigger keys, an allowed empty tag value, and setLanguage("") still calling native.

Manual testing

Not run. The guards return before the native call, and the unit tests assert the plugin is not called.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

Blank login, app id, language, alias, email, sms, tag key, trigger key, and custom event names were sent to the native plugin. One helper now drops those calls.

Co-authored-by: Cursor <cursoragent@cursor.com>
@abdulraqeeb33
abdulraqeeb33 requested a review from a team September 22, 2026 20:36
AR Abdul Azeez and others added 2 commits September 24, 2026 12:28
Empty language is the only reset path. Null is still rejected.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread src/helpers.ts Outdated
@@ -1,3 +1,26 @@
export function rejectNullOrEmpty(value: unknown, api: string): boolean {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Id rename to isMissing

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Renamed to isMissing on this PR and the other SDK PRs.

Comment thread src/helpers.ts Outdated
return true;
}

export function rejectNullOrEmptyKeys(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe rename to hasMissingEntries

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Renamed to hasMissingEntries. The list helper is isMissingAny.

The checks read as predicates: isMissing, isMissingAny, and hasMissingEntries.
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.

2 participants