Skip to content

fix: accept Intent.EXTRA_EMAIL as String or String[] - #256

Open
xhon-pelushi wants to merge 1 commit into
GrapheneOS:mainfrom
xhon-pelushi:fix/164-extra-email-string-array
Open

fix: accept Intent.EXTRA_EMAIL as String or String[]#256
xhon-pelushi wants to merge 1 commit into
GrapheneOS:mainfrom
xhon-pelushi:fix/164-extra-email-string-array

Conversation

@xhon-pelushi

Copy link
Copy Markdown

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

Intent.EXTRA_EMAIL is documented as a String[] of e-mail addresses, but
LaunchConversationActivity and ShareIntentActivity read it with
getStringExtra(), which silently returns null for the array extras put
by well-behaved senders (including this app's own VCardDetailEffectHandler,
which already puts EXTRA_EMAIL as a String[]). Use getStringArrayExtra()
so intents carrying the extra in its documented type are handled, and
forward every address rather than only the first one.

Fixes GrapheneOS#164

@RankoR RankoR left a comment

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.

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.

}
final boolean haveAddress = !TextUtils.isEmpty(intent.getStringExtra(ADDRESS));
final boolean haveEmail = !TextUtils.isEmpty(intent.getStringExtra(Intent.EXTRA_EMAIL));
final String[] emails = intent.getStringArrayExtra(Intent.EXTRA_EMAIL);

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.

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

intent.getStringExtra(Intent.EXTRA_EMAIL).isNullOrEmpty()
intent.getStringArrayExtra(Intent.EXTRA_EMAIL).isNullOrEmpty()

if (Intent.ACTION_SEND != intent.action || hasNoDestination) {

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.

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.

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.

Intent.EXTRA_EMAIL extra is treated as a String

2 participants