diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/LaunchConversationActivityRecipientsTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/LaunchConversationActivityRecipientsTest.kt new file mode 100644 index 000000000..35d8f3c17 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/LaunchConversationActivityRecipientsTest.kt @@ -0,0 +1,114 @@ +package com.android.messaging.ui.conversation + +import android.content.Intent +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertNull +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Covers the EXTRA_EMAIL parsing and recipient filtering in LaunchConversationActivity. The + * extras arrive from other apps through an exported activity, so null and empty entries have to + * be tolerated rather than assumed away. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [36]) +class LaunchConversationActivityRecipientsTest { + + private fun intentWithEmailArray(vararg emails: String?): Intent = + Intent(Intent.ACTION_SENDTO).putExtra(Intent.EXTRA_EMAIL, arrayOf(*emails)) + + private fun intentWithEmailString(email: String?): Intent = + Intent(Intent.ACTION_SENDTO).putExtra(Intent.EXTRA_EMAIL, email) + + @Test + fun emailArrayIsRead() { + assertArrayEquals( + arrayOf("a@example.com", "b@example.com"), + LaunchConversationActivity.getEmailRecipients( + intentWithEmailArray("a@example.com", "b@example.com"), + ), + ) + } + + @Test + fun singleEmailStringIsStillRead() { + assertArrayEquals( + arrayOf("a@example.com"), + LaunchConversationActivity.getEmailRecipients( + intentWithEmailString("a@example.com"), + ), + ) + } + + @Test + fun nullAndEmptyArrayEntriesAreDropped() { + assertArrayEquals( + arrayOf("a@example.com"), + LaunchConversationActivity.getEmailRecipients( + intentWithEmailArray(null, "", "a@example.com"), + ), + ) + } + + @Test + fun arrayOfOnlyNullsCountsAsNoRecipient() { + assertNull( + LaunchConversationActivity.getEmailRecipients(intentWithEmailArray(null, null)), + ) + } + + @Test + fun emptyArrayCountsAsNoRecipient() { + assertNull(LaunchConversationActivity.getEmailRecipients(intentWithEmailArray())) + } + + @Test + fun missingExtraCountsAsNoRecipient() { + assertNull(LaunchConversationActivity.getEmailRecipients(Intent(Intent.ACTION_SENDTO))) + } + + @Test + fun emptyEmailStringCountsAsNoRecipient() { + assertNull(LaunchConversationActivity.getEmailRecipients(intentWithEmailString(""))) + } + + @Test + fun trimInvalidRecipientsToleratesNullEntries() { + // Regression: a null entry used to NPE on recipient.length(). + assertArrayEquals( + arrayOf("a@example.com"), + LaunchConversationActivity.trimInvalidRecipients( + arrayOf(null, "a@example.com"), + ), + ) + } + + @Test + fun trimInvalidRecipientsDropsEmptyEntries() { + assertArrayEquals( + arrayOf("a@example.com"), + LaunchConversationActivity.trimInvalidRecipients( + arrayOf("", "a@example.com"), + ), + ) + } + + @Test + fun trimInvalidRecipientsDropsOverlongEntries() { + val tooLong = "x".repeat(1000) + assertArrayEquals( + arrayOf("a@example.com"), + LaunchConversationActivity.trimInvalidRecipients( + arrayOf(tooLong, "a@example.com"), + ), + ) + } + + @Test + fun trimInvalidRecipientsReturnsNullWhenNothingValid() { + assertNull(LaunchConversationActivity.trimInvalidRecipients(arrayOf(null, ""))) + } +} diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversationpicker/host/share/ShareIntentActivityRedirectTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversationpicker/host/share/ShareIntentActivityRedirectTest.kt new file mode 100644 index 000000000..e215c7372 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/conversationpicker/host/share/ShareIntentActivityRedirectTest.kt @@ -0,0 +1,78 @@ +package com.android.messaging.ui.conversationpicker.host.share + +import android.content.Intent +import android.net.Uri +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Covers when a share intent may be redirected to LaunchConversationActivity. That activity only + * understands a destination plus a text body, so an intent carrying an EXTRA_STREAM attachment + * must stay in the conversation picker or the attachment is silently dropped. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [36]) +class ShareIntentActivityRedirectTest { + + private val address = "address" + private val stream: Uri = Uri.parse("content://media/external/images/media/1") + + private fun sendIntent(): Intent = Intent(Intent.ACTION_SEND).setType("text/plain") + + @Test + fun redirectsWhenEmailArrayIsTheDestination() { + val intent = sendIntent().putExtra(Intent.EXTRA_EMAIL, arrayOf("a@example.com")) + assertTrue(ShareIntentActivity.shouldRedirectToSendTo(intent)) + } + + @Test + fun redirectsWhenEmailStringIsTheDestination() { + val intent = sendIntent().putExtra(Intent.EXTRA_EMAIL, "a@example.com") + assertTrue(ShareIntentActivity.shouldRedirectToSendTo(intent)) + } + + @Test + fun redirectsWhenAddressIsTheDestination() { + val intent = sendIntent().putExtra(address, "+15555550100") + assertTrue(ShareIntentActivity.shouldRedirectToSendTo(intent)) + } + + @Test + fun doesNotRedirectWithoutADestination() { + assertFalse(ShareIntentActivity.shouldRedirectToSendTo(sendIntent())) + } + + @Test + fun doesNotRedirectForOtherActions() { + val intent = Intent(Intent.ACTION_SEND_MULTIPLE) + .putExtra(Intent.EXTRA_EMAIL, arrayOf("a@example.com")) + assertFalse(ShareIntentActivity.shouldRedirectToSendTo(intent)) + } + + @Test + fun doesNotRedirectWhenAnAttachmentWouldBeLost() { + // Regression: EXTRA_STREAM + EXTRA_EMAIL used to redirect and drop the attachment. + val intent = sendIntent() + .putExtra(Intent.EXTRA_EMAIL, arrayOf("a@example.com")) + .putExtra(Intent.EXTRA_STREAM, stream) + assertFalse(ShareIntentActivity.shouldRedirectToSendTo(intent)) + } + + @Test + fun doesNotRedirectWhenAttachmentAccompaniesAnAddress() { + val intent = sendIntent() + .putExtra(address, "+15555550100") + .putExtra(Intent.EXTRA_STREAM, stream) + assertFalse(ShareIntentActivity.shouldRedirectToSendTo(intent)) + } + + @Test + fun emailArrayOfOnlyNullsIsNotADestination() { + val intent = sendIntent().putExtra(Intent.EXTRA_EMAIL, arrayOf(null, "")) + assertFalse(ShareIntentActivity.shouldRedirectToSendTo(intent)) + } +} diff --git a/src/com/android/messaging/ui/conversation/LaunchConversationActivity.java b/src/com/android/messaging/ui/conversation/LaunchConversationActivity.java index 5c9e10357..88651cea7 100644 --- a/src/com/android/messaging/ui/conversation/LaunchConversationActivity.java +++ b/src/com/android/messaging/ui/conversation/LaunchConversationActivity.java @@ -71,12 +71,13 @@ protected void onCreate(final Bundle savedInstanceState) { recipients = commaSeparatedRecipients.split(","); } final boolean haveAddress = !TextUtils.isEmpty(intent.getStringExtra(ADDRESS)); - final boolean haveEmail = !TextUtils.isEmpty(intent.getStringExtra(Intent.EXTRA_EMAIL)); + final String[] emails = getEmailRecipients(intent); + final boolean haveEmail = emails != null; if (recipients == null && (haveAddress || haveEmail)) { if (haveAddress) { recipients = new String[] { intent.getStringExtra(ADDRESS) }; } else { - recipients = new String[] { intent.getStringExtra(Intent.EXTRA_EMAIL) }; + recipients = emails; } } if (recipients != null) { @@ -109,10 +110,32 @@ protected void onCreate(final Bundle savedInstanceState) { finish(); } - private String[] trimInvalidRecipients(String[] recipients) { + /** + * {@link Intent#EXTRA_EMAIL} is documented as a String[], but senders also put a single + * String there, which is what this activity used to read. Accept both so neither kind of + * sender regresses. Returns null when no usable address is present. + */ + static String[] getEmailRecipients(final Intent intent) { + final String[] emails = intent.getStringArrayExtra(Intent.EXTRA_EMAIL); + if (emails != null) { + final List 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 }; + } + + static String[] trimInvalidRecipients(String[] recipients) { List trimmedRecipients = new ArrayList<>(); for (String recipient : recipients) { - if (recipient.length() < MAX_RECIPIENT_LENGTH) { + // 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); } } diff --git a/src/com/android/messaging/ui/conversationpicker/host/share/ShareIntentActivity.kt b/src/com/android/messaging/ui/conversationpicker/host/share/ShareIntentActivity.kt index bba460809..b21861986 100644 --- a/src/com/android/messaging/ui/conversationpicker/host/share/ShareIntentActivity.kt +++ b/src/com/android/messaging/ui/conversationpicker/host/share/ShareIntentActivity.kt @@ -127,10 +127,7 @@ class ShareIntentActivity : BugleComponentActivity() { } private fun redirectToSendToIfNeeded(): Boolean { - val hasNoDestination = intent.getStringExtra(EXTRA_ADDRESS).isNullOrEmpty() && - intent.getStringExtra(Intent.EXTRA_EMAIL).isNullOrEmpty() - - if (Intent.ACTION_SEND != intent.action || hasNoDestination) { + if (!shouldRedirectToSendTo(intent)) { return false } @@ -158,6 +155,29 @@ class ShareIntentActivity : BugleComponentActivity() { ) companion object { + /** + * LaunchConversationActivity only understands a destination plus a text body, so + * redirecting an intent that carries an EXTRA_STREAM attachment would silently drop the + * attachment. Keep such intents in the conversation picker instead. + */ + internal fun shouldRedirectToSendTo(intent: Intent): Boolean = + Intent.ACTION_SEND == intent.action && + !intent.hasExtra(Intent.EXTRA_STREAM) && + ( + !intent.getStringExtra(EXTRA_ADDRESS).isNullOrEmpty() || + hasEmailDestination(intent) + ) + + /** + * EXTRA_EMAIL is documented as a String[] but senders also put a single String there. + * Accept both, and ignore null or empty entries since they come from another app. + */ + internal fun hasEmailDestination(intent: Intent): Boolean { + val emails = intent.getStringArrayExtra(Intent.EXTRA_EMAIL) + return emails?.any { !it.isNullOrEmpty() } + ?: !intent.getStringExtra(Intent.EXTRA_EMAIL).isNullOrEmpty() + } + internal fun createForwardIntent( context: Context, uri: Uri,