Closed Bug 456250 Opened 11 years ago Closed 11 years ago

Properly detect application/pkcs7-mime subtypes in libmime

Categories

(MailNews Core :: MIME, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.1b1

People

(Reporter: mbugz, Assigned: mbugz)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached patch Proposed patch (obsolete) — Splinter Review
Currently, libmime will feed all application/pkcs7-mime parts to mimeEncryptedCMSClass. There are certain subtypes which aren't handled properly by this class, however, and the net result is that the MIME part is not being displayed at all. For "certs-only" PKCS#7 files, this is especially unfortunate - these attachments can't be saved (since they're invisible).

The attached patch adds more logic for determining whether an application/pkcs7-mime part should indeed be handed off to mimeEncryptedCMSClass, or whether it should simply be shown as an attachment (mimeExternalObjectClass/clickable link). It is based on this list from RFC 3851:

   MIME Type                                            File Extension

   application/pkcs7-mime (SignedData, EnvelopedData)      .p7m

   application/pkcs7-mime (degenerate SignedData           .p7c
     certificate management message)

   application/pkcs7-mime (CompressedData)                 .p7z
Attachment #339641 - Flags: superreview?(bienvenu)
Attachment #339641 - Flags: review?(bienvenu)
In current versions of Thunderbird/Seamonkey, the "cert.p7c" attachment is invisible, unfortunately (i.e., can't be saved).
Comment on attachment 339641 [details] [diff] [review]
Proposed patch

thx for doing this.

+        char *ct = (hdrs ? MimeHeaders_get(hdrs, HEADER_CONTENT_TYPE,
+                                           PR_FALSE, PR_FALSE)
+                           : 0);
+        char *st = (ct ? MimeHeaders_get_parameter(ct, "smime-type", NULL, NULL)
+                         : 0);
+

can you use nsnull instead of 0 here? 

+          PR_FREEIF(name);
+        }
+        PR_FREEIF(st);
+        PR_FREEIF(ct);
+

These can all be PR_Free - PR_Free checks for a null arg; the difference is that PR_FREEIF nulls the pointer when its done, and we don't care about that here since they're local vars.

If you'll attach a patch addressing those nits, I'll try it out and land it (probably post b1)
(In reply to comment #2)
> If you'll attach a patch addressing those nits, I'll try it out and land it
> (probably post b1)

Here it is - apparently I was using the wrong code as a model for my patch (just look at the previous #ifdef ENABLE_SMIME block in mimei.cpp... ;-) Thanks for landing this!
Attachment #339641 - Attachment is obsolete: true
Attachment #339673 - Flags: superreview?(bienvenu)
Attachment #339673 - Flags: review?(bienvenu)
Attachment #339641 - Flags: superreview?(bienvenu)
Attachment #339641 - Flags: review?(bienvenu)
Comment on attachment 339673 [details] [diff] [review]
Patch v2, addressing review comments

thx for the patch. I'll try to land this for b1
Attachment #339673 - Flags: superreview?(bienvenu)
Attachment #339673 - Flags: superreview+
Attachment #339673 - Flags: review?(bienvenu)
Attachment #339673 - Flags: review+
Assignee: nobody → mozbugzilla
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.9.1b1
fix checked in, changeset:   414:bf5ecaf12416, thx, Kaspar.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 472028
You need to log in before you can comment on or make changes to this bug.