show: Rewrite show_message_body to use the MIME tree interface.
authorAustin Clements <amdragon@MIT.EDU>
Sat, 24 Dec 2011 18:52:46 +0000 (13:52 -0500)
committerDavid Bremner <bremner@debian.org>
Mon, 26 Dec 2011 02:23:15 +0000 (22:23 -0400)
This removes all of the MIME traversal logic from show_message_body
and leaves only its interaction with the format callbacks.

Besides isolating concerns, since traversal happens behind a trivial
interface, there is now much less code duplication in
show_message_part.  Also, this uses mime_node_seek_dfs to start at the
requested part, eliminating all of the logic about parts being
selected or being in_zone (and reducing the "show state" to only a
part counter).  notmuch_show_params_t no longer needs to be passed
through the recursion because the only two fields that mattered
(related to crypto) are now handled by the MIME tree.

The few remaining complexities in show_message_part highlight
irregularities in the format callbacks with respect to top-level
messages and embedded message parts.

Since this is a rewrite, the diff is not very enlightening.  It's
easier to look at the old code and the new code side-by-side.

show-message.c

index 09fa607f3301533c53df6f30058351224ff473fe..8768889efddd11ed665e84f788bf0864bd6da3c3 100644 (file)
 
 typedef struct show_message_state {
     int part_count;
-    int in_zone;
 } show_message_state_t;
 
 static void
-show_message_part (GMimeObject *part,
+show_message_part (mime_node_t *node,
                   show_message_state_t *state,
                   const notmuch_show_format_t *format,
-                  notmuch_show_params_t *params,
                   int first)
 {
-    GMimeObject *decryptedpart = NULL;
-    int selected;
-    state->part_count += 1;
+    /* Formatters expect the envelope for embedded message parts */
+    GMimeObject *part = node->envelope_part ?
+       GMIME_OBJECT (node->envelope_part) : node->part;
+    int i;
 
-    if (! (GMIME_IS_PART (part) || GMIME_IS_MULTIPART (part) || GMIME_IS_MESSAGE_PART (part))) {
-       fprintf (stderr, "Warning: Not displaying unknown mime part: %s.\n",
-                g_type_name (G_OBJECT_TYPE (part)));
-       return;
-    }
+    if (!first)
+       fputs (format->part_sep, stdout);
 
-    selected = (params->part <= 0 || state->part_count == params->part);
+    /* Format this part */
+    if (format->part_start)
+       format->part_start (part, &(state->part_count));
 
-    if (selected || state->in_zone) {
-       if (!first && (params->part <= 0 || state->in_zone))
-           fputs (format->part_sep, stdout);
+    if (node->decrypt_attempted && format->part_encstatus)
+       format->part_encstatus (node->decrypt_success);
 
-       if (format->part_start)
-           format->part_start (part, &(state->part_count));
-    }
+    if (node->verify_attempted && format->part_sigstatus)
+       format->part_sigstatus (node->sig_validity);
 
-    /* handle PGP/MIME parts */
-    if (GMIME_IS_MULTIPART (part) && params->cryptoctx) {
-       GMimeMultipart *multipart = GMIME_MULTIPART (part);
-       GError* err = NULL;
-
-       if (GMIME_IS_MULTIPART_ENCRYPTED (part) && params->decrypt)
-       {
-           if ( g_mime_multipart_get_count (multipart) != 2 ) {
-               /* this violates RFC 3156 section 4, so we won't bother with it. */
-               fprintf (stderr,
-                        "Error: %d part(s) for a multipart/encrypted message (should be exactly 2)\n",
-                        g_mime_multipart_get_count (multipart));
-           } else {
-               GMimeMultipartEncrypted *encrypteddata = GMIME_MULTIPART_ENCRYPTED (part);
-               decryptedpart = g_mime_multipart_encrypted_decrypt (encrypteddata, params->cryptoctx, &err);
-               if (decryptedpart) {
-                   if ((selected || state->in_zone) && format->part_encstatus)
-                       format->part_encstatus (1);
-                   const GMimeSignatureValidity *sigvalidity = g_mime_multipart_encrypted_get_signature_validity (encrypteddata);
-                   if (!sigvalidity)
-                       fprintf (stderr, "Failed to verify signed part: %s\n", (err ? err->message : "no error explanation given"));
-                   if ((selected || state->in_zone) && format->part_sigstatus)
-                       format->part_sigstatus (sigvalidity);
-               } else {
-                   fprintf (stderr, "Failed to decrypt part: %s\n", (err ? err->message : "no error explanation given"));
-                   if ((selected || state->in_zone) && format->part_encstatus)
-                       format->part_encstatus (0);
-               }
-           }
-       }
-       else if (GMIME_IS_MULTIPART_SIGNED (part))
-       {
-           if ( g_mime_multipart_get_count (multipart) != 2 ) {
-               /* this violates RFC 3156 section 5, so we won't bother with it. */
-               fprintf (stderr,
-                        "Error: %d part(s) for a multipart/signed message (should be exactly 2)\n",
-                        g_mime_multipart_get_count (multipart));
-           } else {
-               /* For some reason the GMimeSignatureValidity returned
-                * here is not a const (inconsistent with that
-                * returned by
-                * g_mime_multipart_encrypted_get_signature_validity,
-                * and therefore needs to be properly disposed of.
-                * Hopefully the API will become more consistent. */
-               GMimeSignatureValidity *sigvalidity = g_mime_multipart_signed_verify (GMIME_MULTIPART_SIGNED (part), params->cryptoctx, &err);
-               if (!sigvalidity) {
-                   fprintf (stderr, "Failed to verify signed part: %s\n", (err ? err->message : "no error explanation given"));
-               }
-               if ((selected || state->in_zone) && format->part_sigstatus)
-                   format->part_sigstatus (sigvalidity);
-               if (sigvalidity)
-                   g_mime_signature_validity_free (sigvalidity);
-           }
-       }
-
-       if (err)
-           g_error_free (err);
-    }
-    /* end handle PGP/MIME parts */
-
-    if (selected || state->in_zone)
-       format->part_content (part);
-
-    if (GMIME_IS_MULTIPART (part)) {
-       GMimeMultipart *multipart = GMIME_MULTIPART (part);
-       int i;
-
-       if (selected)
-           state->in_zone = 1;
-
-       if (decryptedpart) {
-           /* We emit the useless application/pgp-encrypted version
-            * part here only to keep the emitted output as consistent
-            * as possible between decrypted output and the
-            * unprocessed multipart/mime. For some strange reason,
-            * the actual encrypted data is the second part of the
-            * multipart. */
-           show_message_part (g_mime_multipart_get_part (multipart, 0), state, format, params, TRUE);
-           show_message_part (decryptedpart, state, format, params, FALSE);
-       } else {
-           for (i = 0; i < g_mime_multipart_get_count (multipart); i++) {
-               show_message_part (g_mime_multipart_get_part (multipart, i),
-                                  state, format, params, i == 0);
-           }
-       }
-
-       if (selected)
-           state->in_zone = 0;
-
-    } else if (GMIME_IS_MESSAGE_PART (part)) {
-       GMimeMessage *mime_message = g_mime_message_part_get_message (GMIME_MESSAGE_PART (part));
-
-       if (selected)
-           state->in_zone = 1;
-
-       if (selected || (!selected && state->in_zone)) {
-           fputs (format->header_start, stdout);
-           if (format->header_message_part)
-               format->header_message_part (mime_message);
-           fputs (format->header_end, stdout);
-
-           fputs (format->body_start, stdout);
-       }
-
-       show_message_part (g_mime_message_get_mime_part (mime_message),
-                          state, format, params, TRUE);
-
-       if (selected || (!selected && state->in_zone))
-           fputs (format->body_end, stdout);
-
-       if (selected)
-           state->in_zone = 0;
-    }
+    format->part_content (part);
 
-    if (selected || state->in_zone) {
-       if (format->part_end)
-           format->part_end (part);
+    if (node->envelope_part) {
+       fputs (format->header_start, stdout);
+       if (format->header_message_part)
+           format->header_message_part (GMIME_MESSAGE (node->part));
+       fputs (format->header_end, stdout);
+
+       fputs (format->body_start, stdout);
     }
+
+    /* Recurse over the children */
+    state->part_count += 1;
+    for (i = 0; i < node->nchildren; i++)
+       show_message_part (mime_node_child (node, i), state, format, i == 0);
+
+    /* Finish this part */
+    if (node->envelope_part)
+       fputs (format->body_end, stdout);
+
+    if (format->part_end)
+       format->part_end (part);
 }
 
 notmuch_status_t
@@ -179,49 +79,24 @@ show_message_body (notmuch_message_t *message,
                   const notmuch_show_format_t *format,
                   notmuch_show_params_t *params)
 {
-    GMimeStream *stream = NULL;
-    GMimeParser *parser = NULL;
-    GMimeMessage *mime_message = NULL;
-    notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
-    const char *filename = notmuch_message_get_filename (message);
-    FILE *file = NULL;
+    notmuch_status_t ret;
     show_message_state_t state;
+    mime_node_t *root, *part;
 
-    state.part_count = 0;
-    state.in_zone = 0;
-
-    file = fopen (filename, "r");
-    if (! file) {
-       fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));
-       ret = NOTMUCH_STATUS_FILE_ERROR;
-       goto DONE;
-    }
-
-    stream = g_mime_stream_file_new (file);
-    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream), FALSE);
-
-    parser = g_mime_parser_new_with_stream (stream);
-
-    mime_message = g_mime_parser_construct_message (parser);
-
-    show_message_part (g_mime_message_get_mime_part (mime_message),
-                      &state,
-                      format,
-                      params,
-                      TRUE);
-
-  DONE:
-    if (mime_message)
-       g_object_unref (mime_message);
+    ret = mime_node_open (NULL, message, params->cryptoctx, params->decrypt,
+                         &root);
+    if (ret)
+       return ret;
 
-    if (parser)
-       g_object_unref (parser);
+    /* The caller of show_message_body has already handled the
+     * outermost envelope, so skip it. */
+    state.part_count = MAX (params->part, 1);
 
-    if (stream)
-       g_object_unref (stream);
+    part = mime_node_seek_dfs (root, state.part_count);
+    if (part)
+       show_message_part (part, &state, format, TRUE);
 
-    if (file)
-       fclose (file);
+    talloc_free (root);
 
-    return ret;
+    return NOTMUCH_STATUS_SUCCESS;
 }