Date: Mon, 12 Aug 2013 16:01:26 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: notmuch@notmuchmail.org
Subject: Re: [PATCH 6/6] reply: Use RFC 2822/MIME wholly for text format
	template
Message-ID: <20130812200126.GC13257@mit.edu>
References: <1376332839-22825-1-git-send-email-amdragon@mit.edu>
	<1376332839-22825-7-git-send-email-amdragon@mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1376332839-22825-7-git-send-email-amdragon@mit.edu>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: list

Quoth myself on Aug 12 at  2:40 pm:
> Previously, reply's default text format used an odd mix of RFC 2045
> MIME encoding for the reply template's body and some made-up RFC
> 2822-like UTF-8 format for the headers.  The intent was to present the
> headers to the user in a nice, un-encoded format, but this assumed
> that whatever ultimately sent the email would RFC 2047-encode the
> headers, while at the same time the body was already RFC 2045 encoded,
> so it assumed that whatever sent the email would *not* re-encode the
> body.
> 
> This can be fixed by either producing a fully decoded UTF-8 reply
> template, or a fully encoded MIME-compliant RFC 2822 message.  This
> patch does the latter because it is
> 
> a) Well-defined by RFC 2822 and MIME (while any UTF-8 format would be
>    ad hoc).
> 
> b) Ready to be piped to sendmail.  The point of the text format is to
>    be minimal, so a user should be able to pop up the tmeplate in
>    whatever editor they want, edit it, and push it to sendmail.
> 
> c) Consistent with how frontend capabilities.  If a frontend has the

This was supposed to be "Consistent with frontend capabilities." of
course.

>    smarts to RFC 2047 encode the headers before sending the mail, it
>    probably has the smarts to RFC 2047 decode them before presenting
>    the template to a user for editing.
> 
> Also, as far as I know, nothing automated consumes the reply text
> format, so changing this should not cause serious problems.  (And if
> anything does still consume this format, it probably gets these
> encoding issues wrong anyway.)
> ---
>  Makefile.local           |    1 -
>  gmime-filter-headers.c   |  263 ----------------------------------------------
>  gmime-filter-headers.h   |   69 ------------
>  man/man1/notmuch-reply.1 |    2 +-
>  notmuch-reply.c          |   15 +--
>  test/reply               |    9 +-
>  test/reply-to-sender     |    4 +-
>  7 files changed, 12 insertions(+), 351 deletions(-)
>  delete mode 100644 gmime-filter-headers.c
>  delete mode 100644 gmime-filter-headers.h
> 
> diff --git a/Makefile.local b/Makefile.local
> index 84043fe..b7cd266 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -255,7 +255,6 @@ notmuch_client_srcs =		\
>  	command-line-arguments.c\
>  	debugger.c		\
>  	gmime-filter-reply.c	\
> -	gmime-filter-headers.c	\
>  	hooks.c			\
>  	notmuch.c		\
>  	notmuch-config.c	\
> diff --git a/gmime-filter-headers.c b/gmime-filter-headers.c
> deleted file mode 100644
> index 7db3779..0000000
> --- a/gmime-filter-headers.c
> +++ /dev/null
> @@ -1,263 +0,0 @@
> -/*
> - * Copyright © 2009 Keith Packard <keithp@keithp.com>
> - * Copyright © 2010 Michal Sojka <sojkam1@fel.cvut.cz>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation, either version 3 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, write to the Free Software Foundation, Inc.,
> - * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> - */
> -
> -#include "gmime-filter-headers.h"
> -#include <string.h>
> -#include <gmime/gmime-utils.h>
> -#include <glib/gprintf.h>
> -#include <stdlib.h>
> -#include <xutil.h>
> -
> -/**
> - * SECTION: gmime-filter-headers
> - * @title: GMimeFilterHeaders
> - * @short_description: Add/remove headers markers
> - *
> - * A #GMimeFilter for decoding rfc2047 encoded headers to UTF-8
> - **/
> -
> -
> -static void g_mime_filter_headers_class_init (GMimeFilterHeadersClass *klass);
> -static void g_mime_filter_headers_init (GMimeFilterHeaders *filter, GMimeFilterHeadersClass *klass);
> -static void g_mime_filter_headers_finalize (GObject *object);
> -
> -static GMimeFilter *filter_copy (GMimeFilter *filter);
> -static void filter_filter (GMimeFilter *filter, char *in, size_t len, size_t prespace,
> -			   char **out, size_t *outlen, size_t *outprespace);
> -static void filter_complete (GMimeFilter *filter, char *in, size_t len, size_t prespace,
> -			     char **out, size_t *outlen, size_t *outprespace);
> -static void filter_reset (GMimeFilter *filter);
> -
> -
> -static GMimeFilterClass *parent_class = NULL;
> -
> -GType
> -g_mime_filter_headers_get_type (void)
> -{
> -	static GType type = 0;
> -
> -	if (!type) {
> -		static const GTypeInfo info = {
> -			sizeof (GMimeFilterHeadersClass),
> -			NULL, /* base_class_init */
> -			NULL, /* base_class_finalize */
> -			(GClassInitFunc) g_mime_filter_headers_class_init,
> -			NULL, /* class_finalize */
> -			NULL, /* class_data */
> -			sizeof (GMimeFilterHeaders),
> -			0,    /* n_preallocs */
> -			(GInstanceInitFunc) g_mime_filter_headers_init,
> -			NULL	/* value_table */
> -		};
> -
> -		type = g_type_register_static (GMIME_TYPE_FILTER, "GMimeFilterHeaders", &info, (GTypeFlags) 0);
> -	}
> -
> -	return type;
> -}
> -
> -
> -static void
> -g_mime_filter_headers_class_init (GMimeFilterHeadersClass *klass)
> -{
> -	GObjectClass *object_class = G_OBJECT_CLASS (klass);
> -	GMimeFilterClass *filter_class = GMIME_FILTER_CLASS (klass);
> -
> -	parent_class = (GMimeFilterClass *) g_type_class_ref (GMIME_TYPE_FILTER);
> -
> -	object_class->finalize = g_mime_filter_headers_finalize;
> -
> -	filter_class->copy = filter_copy;
> -	filter_class->filter = filter_filter;
> -	filter_class->complete = filter_complete;
> -	filter_class->reset = filter_reset;
> -}
> -
> -static void
> -g_mime_filter_headers_init (GMimeFilterHeaders *filter, GMimeFilterHeadersClass *klass)
> -{
> -	(void) klass;
> -	filter->saw_nl = TRUE;
> -	filter->line = NULL;
> -	filter->line_size = 0;
> -	filter->lineptr = NULL;
> -}
> -
> -static void
> -g_mime_filter_headers_finalize (GObject *object)
> -{
> -	free (GMIME_FILTER_HEADERS (object)->line);
> -	G_OBJECT_CLASS (parent_class)->finalize (object);
> -}
> -
> -
> -static GMimeFilter *
> -filter_copy (GMimeFilter *filter)
> -{
> -	(void) filter;
> -	return g_mime_filter_headers_new ();
> -}
> -
> -static void
> -output_decoded_header (GMimeFilterHeaders *headers, char **outptr)
> -{
> -	char *colon, *name, *s, *decoded_value;
> -	size_t offset;
> -	gint ret;
> -
> -	colon = strchr (headers->line, ':');
> -	if (colon == NULL)
> -		return;
> -
> -	name = headers->line;
> -	*colon = '\0';
> -	s = colon + 1;
> -	while (*s == ' ' || *s == '\t')
> -		s++;
> -	decoded_value = g_mime_utils_header_decode_text(s);
> -	if (decoded_value == NULL)
> -		return;
> -	offset = *outptr - GMIME_FILTER (headers)->outbuf;
> -	g_mime_filter_set_size (GMIME_FILTER (headers), strlen(name) + 2 +
> -			       strlen(decoded_value) + 2, TRUE);
> -	*outptr = GMIME_FILTER (headers)->outbuf + offset;
> -	ret = g_sprintf (*outptr, "%s: %s\n", name, decoded_value);
> -	if (ret > 0)
> -		*outptr += ret;
> -	free (decoded_value);
> -}
> -
> -static void
> -output_final_newline (GMimeFilterHeaders *headers, char **outptr)
> -{
> -	size_t offset;
> -
> -	offset = *outptr - GMIME_FILTER (headers)->outbuf;
> -	g_mime_filter_set_size (GMIME_FILTER (headers), 1, TRUE);
> -	*outptr = GMIME_FILTER (headers)->outbuf + offset;
> -	*(*outptr)++ = '\n';
> -}
> -
> -static void
> -filter_filter (GMimeFilter *filter, char *inbuf, size_t inlen, size_t prespace,
> -	       char **outbuf, size_t *outlen, size_t *outprespace)
> -{
> -	GMimeFilterHeaders *headers = (GMimeFilterHeaders *) filter;
> -	register const char *inptr = inbuf;
> -	const char *inend = inbuf + inlen;
> -	char *lineptr, *lineend, *outptr;
> -
> -	(void) prespace;
> -	if (headers->line == NULL) {
> -		headers->line_size = 200;
> -		headers->lineptr = headers->line = malloc (headers->line_size);
> -	}
> -	lineptr = headers->lineptr;
> -	lineend = headers->line + headers->line_size - 1;
> -	if (lineptr == NULL)
> -		return;
> -	outptr = filter->outbuf;
> -	while (inptr < inend) {
> -		if (*inptr == '\n') {
> -			if (headers->saw_nl)
> -				output_final_newline(headers, &outptr);
> -			headers->saw_nl = TRUE;
> -			inptr++;
> -			continue;
> -		}
> -
> -		if (lineptr == lineend) {
> -			headers->line_size *= 2;
> -			headers->line = xrealloc (headers->line, headers->line_size);
> -			lineptr = headers->line + (headers->line_size / 2) - 1;
> -			lineend = headers->line + headers->line_size - 1;
> -		}
> -
> -		if (headers->saw_nl && *inptr != ' ' && *inptr != '\t') {
> -			*lineptr = '\0';
> -			output_decoded_header (headers, &outptr);
> -			lineptr = headers->line;
> -		}
> -		if (headers->saw_nl && (*inptr == ' ' || *inptr == '\t')) {
> -			*lineptr = ' ';
> -			lineptr++;
> -			while (inptr < inend && (*inptr == ' ' || *inptr == '\t'))
> -				inptr++;
> -			headers->saw_nl = FALSE;
> -			continue;
> -		}
> -		headers->saw_nl = FALSE;
> -
> -		if (*inptr != '\r')
> -			*lineptr++ = *inptr;
> -		inptr++;
> -	}
> -	if (headers->saw_nl) {
> -		*lineptr = '\0';
> -		output_decoded_header (headers, &outptr);
> -		lineptr = headers->line;
> -	}
> -	headers->lineptr = lineptr;
> -	*outlen = outptr - filter->outbuf;
> -	*outprespace = filter->outpre;
> -	*outbuf = filter->outbuf;
> -}
> -
> -static void
> -filter_complete (GMimeFilter *filter, char *inbuf, size_t inlen, size_t prespace,
> -		 char **outbuf, size_t *outlen, size_t *outprespace)
> -{
> -	if (inbuf && inlen)
> -		filter_filter (filter, inbuf, inlen, prespace, outbuf, outlen, outprespace);
> -}
> -
> -static void
> -filter_reset (GMimeFilter *filter)
> -{
> -	GMimeFilterHeaders *headers = (GMimeFilterHeaders *) filter;
> -
> -	headers->saw_nl = TRUE;
> -	free(headers->line);
> -	headers->line = NULL;
> -	headers->line_size = 0;
> -}
> -
> -
> -/**
> - * g_mime_filter_headers_new:
> - * @encode: %TRUE if the filter should encode or %FALSE otherwise
> - * @dots: encode/decode dots (as for SMTP)
> - *
> - * Creates a new #GMimeFilterHeaders filter.
> - *
> - * If @encode is %TRUE, then all lines will be prefixed by "> ",
> - * otherwise any lines starting with "> " will have that removed
> - *
> - * Returns: a new #GMimeFilterHeaders filter.
> - **/
> -GMimeFilter *
> -g_mime_filter_headers_new (void)
> -{
> -	GMimeFilterHeaders *new_headers;
> -
> -	new_headers = (GMimeFilterHeaders *) g_object_newv (GMIME_TYPE_FILTER_HEADERS, 0, NULL);
> -
> -	return (GMimeFilter *) new_headers;
> -}
> -
> diff --git a/gmime-filter-headers.h b/gmime-filter-headers.h
> deleted file mode 100644
> index 1d1a3eb..0000000
> --- a/gmime-filter-headers.h
> +++ /dev/null
> @@ -1,69 +0,0 @@
> -/*
> - * Copyright © 2009 Keith Packard <keithp@keithp.com>
> - * Copyright © 2010 Michal Sojka <sojkam1@fel.cvut.cz>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation, either version 3 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, write to the Free Software Foundation, Inc.,
> - * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> - */
> -
> -#ifndef _GMIME_FILTER_HEADERS_H_
> -#define _GMIME_FILTER_HEADERS_H_
> -
> -#include <gmime/gmime-filter.h>
> -
> -G_BEGIN_DECLS
> -
> -#define GMIME_TYPE_FILTER_HEADERS            (g_mime_filter_headers_get_type ())
> -#define GMIME_FILTER_HEADERS(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), GMIME_TYPE_FILTER_HEADERS, GMimeFilterHeaders))
> -#define GMIME_FILTER_HEADERS_CLASS(klass)    (G_TYPE_CHECK_CLASS_CAST ((klass), GMIME_TYPE_FILTER_HEADERS, GMimeFilterHeadersClass))
> -#define GMIME_IS_FILTER_HEADERS(obj)         (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GMIME_TYPE_FILTER_HEADERS))
> -#define GMIME_IS_FILTER_HEADERS_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GMIME_TYPE_FILTER_HEADERS))
> -#define GMIME_FILTER_HEADERS_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), GMIME_TYPE_FILTER_HEADERS, GMimeFilterHeadersClass))
> -
> -typedef struct _GMimeFilterHeaders GMimeFilterHeaders;
> -typedef struct _GMimeFilterHeadersClass GMimeFilterHeadersClass;
> -
> -/**
> - * GMimeFilterHeaders:
> - * @parent_object: parent #GMimeFilter
> - * @saw_nl: previous char was a \n
> - * @line: temporary buffer for line unfolding
> - * @line_size: size of currently allocated memory for @line
> - * @lineptr: pointer to the first unused character in @line
> - *
> - * A filter to decode rfc2047 encoded headers
> - **/
> -struct _GMimeFilterHeaders {
> -	GMimeFilter parent_object;
> -
> -	gboolean saw_nl;
> -	char *line;
> -	size_t line_size;
> -	char *lineptr;
> -};
> -
> -struct _GMimeFilterHeadersClass {
> -	GMimeFilterClass parent_class;
> -
> -};
> -
> -
> -GType g_mime_filter_headers_get_type (void);
> -
> -GMimeFilter *g_mime_filter_headers_new (void);
> -
> -G_END_DECLS
> -
> -
> -#endif /* _GMIME_FILTER_HEADERS_H_ */
> diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
> index ac76b07..e553145 100644
> --- a/man/man1/notmuch-reply.1
> +++ b/man/man1/notmuch-reply.1
> @@ -41,7 +41,7 @@ include
>  .RS
>  .TP 4
>  .BR default
> -Includes subject and quoted message body.
> +Includes subject and quoted message body as an RFC 2822 message.
>  .TP
>  .BR json
>  Produces JSON output containing headers for a reply message and the
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 0f3b9cd..bfd0f51 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -21,28 +21,17 @@
>   */
>  
>  #include "notmuch-client.h"
> -#include "gmime-filter-headers.h"
>  #include "sprinter.h"
>  
>  static void
>  show_reply_headers (GMimeMessage *message)
>  {
> -    GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
> +    GMimeStream *stream_stdout = NULL;
>  
>      stream_stdout = g_mime_stream_file_new (stdout);
>      if (stream_stdout) {
>  	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
> -	stream_filter = g_mime_stream_filter_new(stream_stdout);
> -	if (stream_filter) {
> -		// g_mime_object_write_to_stream will produce
> -		// RFC2047-encoded headers, but we want to present the
> -		// user with decoded headers and let whatever
> -		// ultimately sends the mail do the RFC2047 encoding.
> -		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
> -					 g_mime_filter_headers_new());
> -		g_mime_object_write_to_stream(GMIME_OBJECT(message), stream_filter);
> -		g_object_unref(stream_filter);
> -	}
> +	g_mime_object_write_to_stream (GMIME_OBJECT(message), stream_stdout);
>  	g_object_unref(stream_stdout);
>      }
>  }
> diff --git a/test/reply b/test/reply
> index a85ebe5..d4389cf 100755
> --- a/test/reply
> +++ b/test/reply
> @@ -132,7 +132,9 @@ add_message '[subject]="This subject is exactly 200 bytes in length. Other than
>  	    '[body]="200-byte header"'
>  output=$(notmuch reply id:${gen_msg_id})
>  test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Subject: Re: This subject is exactly 200 bytes in length. Other than its length there is not much of note here. Note that the length of 200 bytes includes the Subject: and Re: prefixes with two spaces
> +Subject: Re: This subject is exactly 200 bytes in length. Other than its
> + length there is not much of note here. Note that the length of 200 bytes
> + includes the Subject: and Re: prefixes with two spaces
>  In-Reply-To: <${gen_msg_id}>
>  References: <${gen_msg_id}>
>  
> @@ -200,10 +202,11 @@ add_message '[subject]="=?iso-8859-1?q?=e0=df=e7?="' \
>  	    '[body]="Encoding"'
>  
>  output=$(notmuch reply id:${gen_msg_id})
> +# Note that GMime changes from Q- to B-encoding
>  test_expect_equal "$output" "\
>  From: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Subject: Re: àßç
> -To: ☃ <snowman@example.com>
> +Subject: Re: =?iso-8859-1?b?4N/n?=
> +To: =?UTF-8?b?4piD?= <snowman@example.com>
>  In-Reply-To: <${gen_msg_id}>
>  References: <${gen_msg_id}>
>  
> diff --git a/test/reply-to-sender b/test/reply-to-sender
> index c7d15bb..30e5e38 100755
> --- a/test/reply-to-sender
> +++ b/test/reply-to-sender
> @@ -200,7 +200,9 @@ add_message '[subject]="This subject is exactly 200 bytes in length. Other than
>              '[body]="200-byte header"'
>  output=$(notmuch reply  --reply-to=sender id:${gen_msg_id})
>  test_expect_equal "$output" "From: Notmuch Test Suite <test_suite@notmuchmail.org>
> -Subject: Re: This subject is exactly 200 bytes in length. Other than its length there is not much of note here. Note that the length of 200 bytes includes the Subject: and Re: prefixes with two spaces
> +Subject: Re: This subject is exactly 200 bytes in length. Other than its
> + length there is not much of note here. Note that the length of 200 bytes
> + includes the Subject: and Re: prefixes with two spaces
>  In-Reply-To: <${gen_msg_id}>
>  References: <${gen_msg_id}>
>  

-- 
Austin Clements                                      MIT/'06/PhD/CSAIL
amdragon@mit.edu                           http://web.mit.edu/amdragon
       Somewhere in the dream we call reality you will find me,
              searching for the reality we call dreams.