#5839 Fix parsing noname attachment with "message/global" content type (#5844)

* #5839 Skip “message/global” noname attachments

* wip

* add ui test

* wip

* wip

* wip

* wip

* rename isHidden to shoudBeHidden

Author: sosnovsky

extension/chrome/elements/attachment_preview.ts

@@ -98,10 +98,8 @@ View.run(
       const extension = nameSplit[nameSplit.length - 1].toLowerCase();
       if (['jpg', 'jpeg', 'png', 'gif'].includes(extension)) {
         return 'img';
-      } else if (extension === 'txt') {
-        return 'txt';
-      } else if (extension === 'pdf') {
-        return 'pdf';
+      } else if (['txt', 'pdf'].includes(extension)) {
+        return extension as AttachmentType;
       }
       return undefined;
     };

extension/js/common/api/email-provider/gmail/gmail-parser.ts

@@ -154,43 +154,44 @@ export class GmailParser {
     internalResults: Attachment[] = [],
     { pgpEncryptedIndex }: { pgpEncryptedIndex?: number } = {}
   ) => {
-    if (msgOrPayloadOrPart.hasOwnProperty('payload')) {
-      internalMsgId = (msgOrPayloadOrPart as GmailRes.GmailMsg).id;
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      GmailParser.findAttachments((msgOrPayloadOrPart as GmailRes.GmailMsg).payload!, internalMsgId, internalResults);
+    if ('payload' in msgOrPayloadOrPart && msgOrPayloadOrPart.payload) {
+      internalMsgId = msgOrPayloadOrPart.id;
+      GmailParser.findAttachments(msgOrPayloadOrPart.payload, internalMsgId, internalResults);
     }
-    if (msgOrPayloadOrPart.hasOwnProperty('parts')) {
-      const payload = msgOrPayloadOrPart as GmailRes.GmailMsg$payload;
-      const contentType = payload.headers?.find(x => x.name.toLowerCase() === 'content-type');
-      const parts = payload.parts!; // eslint-disable-line @typescript-eslint/no-non-null-assertion
-      // are we dealing with a PGP/MIME encrypted message?
-      const pgpEncrypted = Boolean(
-        parts.length === 2 &&
-          contentType?.value?.startsWith('multipart/encrypted') &&
-          (contentType.value.includes('protocol="application/pgp-encrypted"') || parts[0].mimeType === 'application/pgp-encrypted')
-      );
-      for (const [i, part] of parts.entries()) {
-        GmailParser.findAttachments(part, internalMsgId, internalResults, {
-          pgpEncryptedIndex: pgpEncrypted ? i : undefined,
-        });
+    if ('parts' in msgOrPayloadOrPart) {
+      const contentType = msgOrPayloadOrPart.headers?.find(header => header.name.toLowerCase() === 'content-type');
+      const parts = msgOrPayloadOrPart.parts ?? [];
+      const hasMultipartAlternativePart = parts.find(part => part.mimeType === 'multipart/alternative');
+      // ignore plain inline attachments
+      if (!contentType?.value.startsWith('multipart/related') || !hasMultipartAlternativePart) {
+        // are we dealing with a PGP/MIME encrypted message?
+        const pgpEncrypted = Boolean(
+          parts.length === 2 &&
+            contentType?.value.startsWith('multipart/encrypted') &&
+            (contentType.value.includes('protocol="application/pgp-encrypted"') || parts[0].mimeType === 'application/pgp-encrypted')
+        );
+        for (const [i, part] of parts.entries()) {
+          GmailParser.findAttachments(part, internalMsgId, internalResults, {
+            pgpEncryptedIndex: pgpEncrypted ? i : undefined,
+          });
+        }
       }
     }
-    /* eslint-disable @typescript-eslint/no-non-null-assertion */
-    if (msgOrPayloadOrPart.hasOwnProperty('body') && (msgOrPayloadOrPart as GmailRes.GmailMsg$payload$part).body!.hasOwnProperty('attachmentId')) {
-      const payload = msgOrPayloadOrPart as GmailRes.GmailMsg$payload;
+    if ('body' in msgOrPayloadOrPart && msgOrPayloadOrPart.body?.hasOwnProperty('attachmentId')) {
+      const payload = msgOrPayloadOrPart as GmailRes.GmailMsg$payload$part;
       const treatAs = Attachment.treatAsForPgpEncryptedAttachments(payload.mimeType, pgpEncryptedIndex);
+      const inline = (GmailParser.findHeader(payload, 'content-disposition') || '').toLowerCase().startsWith('inline');
       internalResults.push(
         new Attachment({
           msgId: internalMsgId,
-          id: (msgOrPayloadOrPart as GmailRes.GmailMsg$payload$part).body!.attachmentId,
-          length: (msgOrPayloadOrPart as GmailRes.GmailMsg$payload$part).body!.size,
-          name: (msgOrPayloadOrPart as GmailRes.GmailMsg$payload$part).filename,
-          type: (msgOrPayloadOrPart as GmailRes.GmailMsg$payload$part).mimeType,
+          id: payload.body?.attachmentId ?? '',
+          length: payload.body?.size ?? 0,
+          name: payload.filename,
+          type: payload.mimeType,
           treatAs,
-          inline: (GmailParser.findHeader(msgOrPayloadOrPart, 'content-disposition') || '').toLowerCase().startsWith('inline'),
+          inline,
         })
       );
-      /* eslint-enable @typescript-eslint/no-non-null-assertion */
     }
     return internalResults;
   };

extension/js/common/core/attachment.ts

@@ -148,15 +148,6 @@ export class Attachment {
     );
   };
 
-  public isPrivateKey = (): boolean => {
-    return (
-      Boolean(this.name.match(/(cryptup|flowcrypt)-backup-([a-z0-9]+(?:\-[A-F0-9]{40})?)\.(key|asc)$/g)) ||
-      (/\.(asc|key)$/.test(this.name) &&
-        this.hasData() &&
-        Buf.with(this.getData().subarray(0, 100)).toUtfStr().includes('-----BEGIN PGP PRIVATE KEY BLOCK-----'))
-    );
-  };
-
   public hasData = () => {
     return this.bytes instanceof Uint8Array;
   };
@@ -178,7 +169,7 @@ export class Attachment {
     throw new Error('Attachment has no data set');
   };
 
-  public isAttachmentAnImage = () => {
+  public isImage = () => {
     return this.type.startsWith('image/') || this.type.startsWith('img/');
   };
 
@@ -201,9 +192,9 @@ export class Attachment {
         }
       }
       return 'signature';
-    } else if (this.inline && this.isAttachmentAnImage()) {
+    } else if (this.inline && this.isImage()) {
       return 'inlineImage';
-    } else if (!this.name && !this.isAttachmentAnImage() && this.type !== 'application/octet-stream' && this.type !== 'multipart/mixed') {
+    } else if (!this.name && !this.isImage() && !['application/octet-stream', 'multipart/mixed', 'message/global'].includes(this.type)) {
       // this.name may be '' or undefined - catch either
       return this.length < 100 ? 'hidden' : 'encryptedMsg';
     } else if (this.name === 'msg.asc' && this.length < 100 && this.type === 'application/pgp-encrypted') {
@@ -222,11 +213,10 @@ export class Attachment {
     } else if (this.isPrivateKey()) {
       return 'privateKey';
     } else {
-      // && !Attachment.encryptedMsgNames.includes(this.name) -- already checked above
       const isAmbiguousAscFile = this.name.endsWith('.asc'); // ambiguous .asc name
       const isAmbiguousNonameFile = !this.name || this.name === 'noname'; // may not even be OpenPGP related
-      if (!this.inline && this.length < 100000 && (isAmbiguousAscFile || isAmbiguousNonameFile) && !this.isAttachmentAnImage()) {
-        if (isAmbiguousNonameFile && this.type === 'application/octet-stream') {
+      if (!this.inline && this.length < 100000 && (isAmbiguousAscFile || isAmbiguousNonameFile) && !this.isImage()) {
+        if (isAmbiguousNonameFile && ['application/octet-stream', 'message/global'].includes(this.type)) {
           return 'plainFile';
         }
         return this.hasData() ? 'maybePgp' : 'needChunk';
@@ -239,6 +229,12 @@ export class Attachment {
     return this.type === 'application/pgp-encrypted' && this.name.length === 0 && this.getData().toUtfStr() === 'Version: 1';
   };
 
+  public shouldBeHidden = () => {
+    return (
+      this.type === 'application/pgp-keys' || this.isPublicKey() || this.inline || Attachment.encryptedMsgNames.some(filename => this.name.includes(filename))
+    );
+  };
+
   public isExecutableFile = () => {
     return [
       'ade',
@@ -296,4 +292,13 @@ export class Attachment {
       'xll',
     ].some(exeFileExtension => this.name.endsWith('.' + exeFileExtension));
   };
+
+  private isPrivateKey = (): boolean => {
+    return (
+      Boolean(this.name.match(/(cryptup|flowcrypt)-backup-([a-z0-9]+(?:\-[A-F0-9]{40})?)\.(key|asc)$/g)) ||
+      (/\.(asc|key)$/.test(this.name) &&
+        this.hasData() &&
+        Buf.with(this.getData().subarray(0, 100)).toUtfStr().includes('-----BEGIN PGP PRIVATE KEY BLOCK-----'))
+    );
+  };
 }

extension/js/content_scripts/webmail/gmail/gmail-element-replacer.ts

@@ -59,7 +59,7 @@ export class GmailElementReplacer extends WebmailElementReplacer {
     settingsBtnContainer: 'div.aeH > div > .fY',
     standardComposeRecipient: 'div.az9 span[email][data-hovercard-id]',
     numberOfAttachments: '.aVW',
-    numberOfAttachmentsDigit: '.aVW span:first-child',
+    numberOfAttachmentsLabel: '.aVW span:first-child',
     attachmentsButtons: '.aZi',
     draftsList: '.ae4',
   };
@@ -475,16 +475,6 @@ export class GmailElementReplacer extends WebmailElementReplacer {
     }
   };
 
-  private isAttachmentHideable = (attachment: Attachment, renderStatus: string) => {
-    return (
-      renderStatus === 'hidden' ||
-      attachment.type === 'application/pgp-keys' ||
-      attachment.isPublicKey() ||
-      attachment.inline ||
-      Attachment.encryptedMsgNames.some(filename => attachment.name.includes(filename))
-    );
-  };
-
   private processAttachments = async (
     msgId: string,
     attachments: Attachment[],
@@ -519,7 +509,7 @@ export class GmailElementReplacer extends WebmailElementReplacer {
         messageInfo,
         skipGoogleDrive
       );
-      if (this.isAttachmentHideable(a, renderStatus)) {
+      if (renderStatus === 'hidden' || a.shouldBeHidden()) {
         nRenderedAttachments--;
       }
     }
@@ -528,22 +518,14 @@ export class GmailElementReplacer extends WebmailElementReplacer {
       $(this.sel.attachmentsButtons).hide();
     }
     if (nRenderedAttachments === 0) {
+      attachmentsContainerInner.parents(this.sel.attachmentsContainerOuter).first().hide();
       $(this.sel.attachmentsContainerOuter).children('.hp').hide();
       if ($('.pgp_block').length === 0) {
         attachmentsContainerInner.parents(this.sel.attachmentsContainerOuter).first().hide();
       }
     } else {
-      const elementsToClone = ['span.a2H', 'div.a2b'];
-      const scannedByGmailLabel = $(elementsToClone[0]).first().clone();
-      const scannedByGmailPopover = $(elementsToClone[1]).first().clone(true);
-      // for uniformity reasons especially when Google used "One" for single attachment and numeric representation for multiple.
-      const gmailAttachmentLabelReplacement = $(
-        `<span>${nRenderedAttachments}</span>&nbsp;<span>${nRenderedAttachments > 1 ? 'Attachments' : 'Attachment'}</span>`
-      );
-      attachmentsContainerInner.parent().find(this.sel.numberOfAttachments).empty();
-      gmailAttachmentLabelReplacement.appendTo(attachmentsContainerInner.parent().find(this.sel.numberOfAttachments));
-      scannedByGmailLabel.appendTo(attachmentsContainerInner.parent().find(this.sel.numberOfAttachments));
-      scannedByGmailPopover.appendTo(attachmentsContainerInner.parent().find(this.sel.numberOfAttachments));
+      const attachmentsLabel = nRenderedAttachments > 1 ? `${nRenderedAttachments} Attachments` : 'One Attachment';
+      attachmentsContainerInner.parent().find(this.sel.numberOfAttachmentsLabel).text(attachmentsLabel);
       attachmentsContainerInner.parent().find(this.sel.numberOfAttachments).show();
     }
     if (!skipGoogleDrive) {