fix: accept Intent.EXTRA_EMAIL as String or String[]

[xhon-pelushi]

Handle EXTRA_EMAIL when callers pass a single String instead of String[].

Fixes #164

Test plan

• Review diff against issue
• Run project lint/tests if applicable

[RankoR]

Thanks for your contribution!

1. Added/changed code should be covered by tests
2. PR title states that both String and String[] now accepted, but in the code you're retrieving only array.

[RankoR]

Array items are not guaranteed to be non-null.

LaunchConversationActivity.java:113-119:

for (String recipient : recipients) {
   if (recipient.length() < MAX_RECIPIENT_LENGTH) { // Will crash here

So, it should also be fixed in LaunchConversationActivity.java:113-119:

for (String recipient : recipients) {
   if (!TextUtils.isEmpty(recipient) && recipient.length() < MAX_RECIPIENT_LENGTH) {

Same for ShareIntentActivity

[xhon-pelushi]

You're right, and it's worse than a latent risk — my change is what made it reachable. Before it, recipients was either split(",") output (never null elements) or a single getStringExtra guarded by !TextUtils.isEmpty. Reading the array put another app's contents straight into trimInvalidRecipients(), and since LaunchConversationActivity is android:exported="true" with BROWSABLE, any app (or an sms: link) can send EXTRA_EMAIL = new String[]{null}.

Reproduced it as a unit test against the old code, and it's exactly your prediction:

java.lang.NullPointerException: Cannot invoke "String.length()" because "recipient" is null

Applied your suggestion verbatim, which also fixes the pre-existing empty-string case ("a,,b".split(",") yields "", and "".length() < MAX was accepted as a recipient):

    static String[] trimInvalidRecipients(String[] recipients) {
        List<String> trimmedRecipients = new ArrayList<>();
        for (String recipient : recipients) {
            // The recipients come from another app's intent extras, so entries may be null or
            // empty; TextUtils.isEmpty() is null-safe where recipient.length() is not.
            if (!TextUtils.isEmpty(recipient) && recipient.length() < MAX_RECIPIENT_LENGTH) {
                trimmedRecipients.add(recipient);
            }
        }

I also stopped bad entries earlier so haveEmail doesn't become true for an array that holds nothing usable — an all-null array now reads as "no email" and falls through to the contact picker instead of producing a recipient list that trims to empty.

For ShareIntentActivity I handled it in the same spirit but without needing a null-unsafe call at all — see the reply on your other comment.

[RankoR]

Perhaps should become Intent.ACTION_SEND != intent.action || hasNoDestination || intent.hasExtra(Intent.EXTRA_STREAM), otherwise for EXTRA_STREAM + EXTRA_EMAIL intents we're losing EXTRA_STREAM.

[xhon-pelushi]

Good catch — and again my change is what exposed it. Before, hasNoDestination used getStringExtra(Intent.EXTRA_EMAIL), which returns null when the extra is a String[]. So for the very case this PR exists to support, the redirect didn't fire and the attachment survived by accident. Switching to getStringArrayExtra made the redirect fire, and LaunchConversationActivity only ever reads ADDRESS, EXTRA_EMAIL, SMS_BODY, the data-URI body and EXTRA_TEXT — never EXTRA_STREAM — so putExtras(intent) carries the stream along and then it's ignored.

Took your suggestion, but pulled the decision into a testable function rather than growing the condition, since it now has three parts and I wanted to cover it:

internal fun shouldRedirectToSendTo(intent: Intent): Boolean {
    if (Intent.ACTION_SEND != intent.action) {
        return false
    }
    if (intent.hasExtra(Intent.EXTRA_STREAM)) {
        return false
    }
    val hasDestination = !intent.getStringExtra(EXTRA_ADDRESS).isNullOrEmpty() ||
        hasEmailDestination(intent)
    return hasDestination
}

internal fun hasEmailDestination(intent: Intent): Boolean {
    val emails = intent.getStringArrayExtra(Intent.EXTRA_EMAIL)
    if (emails != null) {
        return emails.any { !it.isNullOrEmpty() }
    }
    return !intent.getStringExtra(Intent.EXTRA_EMAIL).isNullOrEmpty()
}

redirectToSendToIfNeeded() is now just if (!shouldRedirectToSendTo(intent)) return false. hasEmailDestination also covers the null-entry problem from your other comment here — an arrayOf(null, "") destination reads as absent rather than triggering a redirect to a recipient list that can't be built.

Verified with unit tests: EXTRA_STREAM + EXTRA_EMAIL and EXTRA_STREAM + address both stay in the picker; email-as-array, email-as-String and address alone all still redirect. Both EXTRA_STREAM tests fail against the previous code.

[xhon-pelushi]

Thanks @RankoR — all four points addressed, and both of your inline findings were regressions I introduced rather than pre-existing ones, which I've written up on the respective threads.

2. Title vs. code. You're right: the commit replaced the String read with a String[] read, so a sender putting a single String regressed even though the title claimed both. Since LaunchConversationActivity is exported and takes intents from arbitrary apps, dropping the form that used to work is the wrong trade, so I've made it accept both rather than narrowing the title:

static String[] getEmailRecipients(final Intent intent) {
    final String[] emails = intent.getStringArrayExtra(Intent.EXTRA_EMAIL);
    if (emails != null) {
        final List<String> nonEmpty = new ArrayList<>(emails.length);
        for (final String email : emails) {
            if (!TextUtils.isEmpty(email)) {
                nonEmpty.add(email);
            }
        }
        return nonEmpty.isEmpty() ? null : nonEmpty.toArray(new String[0]);
    }
    final String email = intent.getStringExtra(Intent.EXTRA_EMAIL);
    return TextUtils.isEmpty(email) ? null : new String[] { email };
}

The two forms can't collide: Bundle.getStringArray returns null for a String-valued key and getString returns null for a String[]-valued one, so checking the array first and falling back is unambiguous. Issue #164 and the Android docs both say String[] is the correct type — this keeps that as the primary path while not breaking the senders that get it wrong.

1. Test coverage. Added 19 unit tests in app/src/test/kotlin (Robolectric, matching the existing tests there):

• LaunchConversationActivityRecipientsTest (11) — array form, single-String form, null/empty entries dropped, all-null array, empty array, missing extra, and trimInvalidRecipients against null entries, empty entries, over-long entries, and nothing-valid.
• ShareIntentActivityRedirectTest (8) — redirect for each destination form, no redirect without a destination, no redirect for other actions, and no redirect when EXTRA_STREAM is present (with either destination kind).

All 19 pass on the change. Against the previous code, 10 of the 19 fail, so they pin the reported behaviour rather than just describing it:

LaunchConversationActivityRecipientsTest: 6 of 11 fail
  trimInvalidRecipientsToleratesNullEntries
        java.lang.NullPointerException: Cannot invoke "String.length()" because "recipient" is null
  trimInvalidRecipientsReturnsNullWhenNothingValid
        java.lang.NullPointerException: Cannot invoke "String.length()" because "recipient" is null
  singleEmailStringIsStillRead        actual array was null
  nullAndEmptyArrayEntriesAreDropped  expected.length=1 actual.length=3
  arrayOfOnlyNullsCountsAsNoRecipient expected null, but was:<[Ljava.lang.String;@...>
  trimInvalidRecipientsDropsEmptyEntries
ShareIntentActivityRedirectTest: 4 of 8 fail
  doesNotRedirectWhenAnAttachmentWouldBeLost
  doesNotRedirectWhenAttachmentAccompaniesAnAddress
  redirectsWhenEmailStringIsTheDestination
  emailArrayOfOnlyNullsIsNotADestination

Run with ./gradlew :app:testDebugUnitTest --tests '…LaunchConversationActivityRecipientsTest' --tests '…ShareIntentActivityRedirectTest' on compileSdk 36. One note in case it saves you time: the build needs git submodule update --init first, or :lib:platform_frameworks_opt_chips and :lib:platform_frameworks_opt_vcard resolve to "No variants exist".

The internal/package-private visibility on getEmailRecipients, trimInvalidRecipients, shouldRedirectToSendTo and hasEmailDestination is only so the tests can reach them; happy to change the approach if you'd rather these stayed private and the coverage came from driving the activities instead.

[xhon-pelushi]

Pushed as b4b4b14 — all four points are now on the branch: both EXTRA_EMAIL forms accepted, trimInvalidRecipients() null-safe via TextUtils.isEmpty, EXTRA_STREAM intents no longer redirected, and 19 unit tests in app/src/test/kotlin.

Ready for another look. As noted above, the internal/package-private visibility on the four extracted helpers exists only so the tests can reach them — happy to change that if you'd rather they stayed private.