Mailing list Reply-To munging and notmuch reply

Mailing list Reply-To munging and notmuch reply

[Jed Brown]

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

First, I'm aware that such munging is A Bad Thing

  http://www.unicom.com/pw/reply-to-harmful.html

but a lot of lists do it anyway (mostly to work around widely used
mailers with lame defaults).  After munging, we get headers looking like
this

  From: Some User <some.user@example.com>
  To: Sample users list <sample-users@sample.org>
  Reply-To: Sample users list <sample-users@sample.org>
  
Notmuch reply produces

  To: Sample users list <sample-users@sample.org>,
      Sample users list <sample-users@sample.org>

Handling this is a bit messy, I think we want the current behavior
unless To matches Reply-To, in which case we use From and Reply-To.  If
this is indeed the least bad behavior, I will make a patch for it.

Jed

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Mailing list Reply-To munging and notmuch reply

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]

On Sat, 28 Nov 2009 17:05:06 +0100, Jed Brown <jed@59A2.org> wrote:
> First, I'm aware that such munging is A Bad Thing
> 
>   http://www.unicom.com/pw/reply-to-harmful.html

Definitely.

>   From: Some User <some.user@example.com>
>   To: Sample users list <sample-users@sample.org>
>   Reply-To: Sample users list <sample-users@sample.org>
>   
> Notmuch reply produces
> 
>   To: Sample users list <sample-users@sample.org>,
>       Sample users list <sample-users@sample.org>
> 
> Handling this is a bit messy, I think we want the current behavior
> unless To matches Reply-To, in which case we use From and Reply-To.  If
> this is indeed the least bad behavior, I will make a patch for it.

Oh, I really like that. The condition there avoids breaking legitimate
uses of Reply-To, (such as the cairo lists I run, where cairo-commit@
has no user-generated From:---just a single automated address, but has
Reply-To: set to the cairo@ list instead so that replies to committed
patches go to the right place).

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Mailing list Reply-To munging and notmuch reply

[Jed Brown]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

On Sat, 28 Nov 2009 09:55:43 -0800, Carl Worth <cworth@cworth.org> wrote:
> On Sat, 28 Nov 2009 17:05:06 +0100, Jed Brown <jed@59A2.org> wrote:
> > Handling this is a bit messy, I think we want the current behavior
> > unless To matches Reply-To, in which case we use From and Reply-To.  If
> > this is indeed the least bad behavior, I will make a patch for it.
> 
> Oh, I really like that. The condition there avoids breaking legitimate
> uses of Reply-To, (such as the cairo lists I run, where cairo-commit@
> has no user-generated From:---just a single automated address, but has
> Reply-To: set to the cairo@ list instead so that replies to committed
> patches go to the right place).

I'm not sure I follow (at least not when comparing to the sanitized
headers shown in the online archives).  Could you send me one of these
headers?

When mailing lists munge, do they ever just add to that field (RFC-2822
says Reply-To may contain multiple addresses)?

Jed

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Mailing list Reply-To munging and notmuch reply

[Carl Worth]

[-- Attachment #1.1: Type: text/plain, Size: 1311 bytes --]

On Sat, 28 Nov 2009 20:26:52 +0100, Jed Brown <jed@59A2.org> wrote:
> I'm not sure I follow (at least not when comparing to the sanitized
> headers shown in the online archives).  Could you send me one of these
> headers?

No problem. See attached.

The mail on cairo-commit has a From: address that is auto-generated by
the username of the committer, (so is likely not even valid for a
reply). The To: address (cairo-commit@cairographics.org) also does not
accept any mail. So a reply to either of those addresses will not be
useful.

Instead, the mail sets a Reply-To: to the general
cairo@cairographics.org list.

So that's not your typical Reply-to munging on a mailing list, (in this
case, there's no incoming mail where the munging could break a user's
manually set Reply-To header).

And it sounds like your idea would not break the replying to this list,
(replies would still be directed to cairo@ which is what I want). If I
understand correctly, it would still also reply to cairo-commit@ which
will end up bouncing, until we add something for "reply to sender only".
And that's the same behavior I've had with any MUA and this list.

> When mailing lists munge, do they ever just add to that field (RFC-2822
> says Reply-To may contain multiple addresses)?

I have no idea about that myself.

-Carl


[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: cairo-commit-sample.mail --]
[-- Type: text/plain, Size: 5449 bytes --]

Return-path: <cairo-commit-bounces@lists.cairographics.org>
Envelope-to: cworth@localhost
Delivery-date: Fri, 27 Nov 2009 04:50:53 -0800
Received: from localhost ([127.0.0.1] helo=yoom.home.cworth.org)
	by yoom.home.cworth.org with esmtp (Exim 4.69)
	(envelope-from <cairo-commit-bounces@lists.cairographics.org>)
	id 1NE0Hk-0008Lf-W7
	for cworth@localhost; Fri, 27 Nov 2009 04:50:53 -0800
X-Original-To: cworth@cworth.org
Delivered-To: cworth@cworth.org
Received: from olra.theworths.org [82.165.184.25]
	by yoom.home.cworth.org with IMAP (fetchmail-6.3.9-rc2)
	for <cworth@localhost> (single-drop); Fri, 27 Nov 2009 04:50:52 -0800 (PST)
Received: from localhost (localhost [127.0.0.1])
	by olra.theworths.org (Postfix) with ESMTP id A35EC431FBC
	for <cworth@cworth.org>; Fri, 27 Nov 2009 04:50:19 -0800 (PST)
X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
X-Spam-Flag: NO
X-Spam-Score: -1.93
X-Spam-Level: 
X-Spam-Status: No, score=-1.93 tagged_above=-999 required=2 tests=[AWL=-0.931,
	BAYES_50=0.001, RCVD_IN_DNSWL_LOW=-1] autolearn=ham
Received: from olra.theworths.org ([127.0.0.1])
	by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id 58abAJlDmp9X for <cworth@cworth.org>;
	Fri, 27 Nov 2009 04:50:19 -0800 (PST)
Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177])
	by olra.theworths.org (Postfix) with ESMTP id ED626431FAE
	for <cworth@cworth.org>; Fri, 27 Nov 2009 04:50:18 -0800 (PST)
Received: from gabe.freedesktop.org (localhost [127.0.0.1])
	by gabe.freedesktop.org (Postfix) with ESMTP id CC2099F6BA;
	Fri, 27 Nov 2009 04:50:18 -0800 (PST)
X-Original-To: cairo-commit@cairographics.org
Delivered-To: cairo-commit@cairographics.org
Received: from kemper.freedesktop.org (kemper.freedesktop.org
	[131.252.210.178])
	by gabe.freedesktop.org (Postfix) with ESMTP id 36C809F643
	for <cairo-commit@cairographics.org>;
	Fri, 27 Nov 2009 04:50:17 -0800 (PST)
Received: by kemper.freedesktop.org (Postfix, from userid 2987)
	id C147710051; Fri, 27 Nov 2009 04:50:16 -0800 (PST)
To: cairo-commit@cairographics.org
X-Git-Repository: git://git.cairographics.org/git/cairo
Message-Id: <20091127125016.C147710051@kemper.freedesktop.org>
Date: Fri, 27 Nov 2009 04:50:16 -0800 (PST)
From: ajohnson@kemper.freedesktop.org (Adrian Johnson)
Subject: [cairo-commit] src/cairo-pdf-surface.c
X-BeenThere: cairo-commit@lists.cairographics.org
X-Mailman-Version: 2.1.9
Precedence: list
Reply-To: cairo@cairographics.org
List-Id: CVS commit messages for cairo <cairo-commit.lists.cairographics.org>
List-Unsubscribe: <http://lists.cairographics.org/mailman/listinfo/cairo-commit>, 
	<mailto:cairo-commit-request@lists.cairographics.org?subject=unsubscribe>
List-Archive: <http://lists.cairographics.org/archives/cairo-commit>
List-Post: <mailto:cairo-commit@lists.cairographics.org>
List-Help: <mailto:cairo-commit-request@lists.cairographics.org?subject=help>
List-Subscribe: <http://lists.cairographics.org/mailman/listinfo/cairo-commit>, 
	<mailto:cairo-commit-request@lists.cairographics.org?subject=subscribe>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: cairo-commit-bounces@lists.cairographics.org
Errors-To: cairo-commit-bounces@lists.cairographics.org

 src/cairo-pdf-surface.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

New commits:
commit 02d7e123d640d62db46ef03d7faecbc7598444e1
Author: Adrian Johnson <ajohnson@redneon.com>
Date:   Fri Nov 27 23:11:56 2009 +1030

    PDF: Remove extraneous \n from end of jpeg/jp2 data
    
    PDF requires a '\n' between the end of stream data and the "endstream"
    that is not included in the stream length. Ensure this is always added
    in _close_stream where it is not included in the stream length.
    
    Previously the jpeg/jp2 embedding functions were adding the '\n'. This
    resulted in the '\n' becoming part of the stream data.

diff --git a/src/cairo-pdf-surface.c b/src/cairo-pdf-surface.c
index 62178dc..e8335f0 100644
--- a/src/cairo-pdf-surface.c
+++ b/src/cairo-pdf-surface.c
@@ -1332,13 +1332,12 @@ _cairo_pdf_surface_close_stream (cairo_pdf_surface_t *surface)
 	surface->output = surface->pdf_stream.old_output;
 	_cairo_pdf_operators_set_stream (&surface->pdf_operators, surface->output);
 	surface->pdf_stream.old_output = NULL;
-	_cairo_output_stream_printf (surface->output,
-				     "\n");
     }
 
     length = _cairo_output_stream_get_position (surface->output) -
 	surface->pdf_stream.start_offset;
     _cairo_output_stream_printf (surface->output,
+				 "\n"
 				 "endstream\n"
 				 "endobj\n");
 
@@ -2010,7 +2009,6 @@ _cairo_pdf_surface_emit_jpx_image (cairo_pdf_surface_t   *surface,
 	return status;
 
     _cairo_output_stream_write (surface->output, mime_data, mime_data_length);
-    _cairo_output_stream_printf (surface->output, "\n");
     status = _cairo_pdf_surface_close_stream (surface);
 
     return status;
@@ -2058,7 +2056,6 @@ _cairo_pdf_surface_emit_jpeg_image (cairo_pdf_surface_t   *surface,
 	return status;
 
     _cairo_output_stream_write (surface->output, mime_data, mime_data_length);
-    _cairo_output_stream_printf (surface->output, "\n");
     status = _cairo_pdf_surface_close_stream (surface);
 
     return status;
_______________________________________________
cairo-commit mailing list
cairo-commit@lists.cairographics.org
http://lists.cairographics.org/mailman/listinfo/cairo-commit

[PATCH 1/2] notmuch-reply.c: factor adding recipients into common function

[Jed Brown]

add_recipients_from_message, in order to isolate more sophisticated
mailing list logic.

Signed-off-by: Jed Brown <jed@59A2.org>
---
 notmuch-reply.c |   88 ++++++++++++++++++++++++++++---------------------------
 1 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 9ca1236..b91a830 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -23,17 +23,6 @@
 #include "notmuch-client.h"
 #include "gmime-filter-reply.h"
 
-static const struct {
-    const char *header;
-    const char *fallback;
-    GMimeRecipientType recipient_type;
-} reply_to_map[] = {
-    { "reply-to", "from", GMIME_RECIPIENT_TYPE_TO  },
-    { "to",         NULL, GMIME_RECIPIENT_TYPE_TO  },
-    { "cc",         NULL, GMIME_RECIPIENT_TYPE_CC  },
-    { "bcc",        NULL, GMIME_RECIPIENT_TYPE_BCC }
-};
-
 static void
 reply_part_content (GMimeObject *part)
 {
@@ -193,16 +182,56 @@ add_recipients_for_string (GMimeMessage *message,
     return add_recipients_for_address_list (message, config, type, list);
 }
 
+/* Augments the recipients of reply from the headers of message.
+ *
+ * If any of the user's addresses were found in these headers, the first
+ * of these returned, otherwise NULL is returned.
+ */
+static const char *
+add_recipients_from_message (GMimeMessage *reply,
+			     notmuch_config_t *config,
+			     notmuch_message_t *message)
+{
+    static const struct {
+	const char *header;
+	const char *fallback;
+	GMimeRecipientType recipient_type;
+    } reply_to_map[] = {
+	{ "reply-to", "from", GMIME_RECIPIENT_TYPE_TO  },
+	{ "to",         NULL, GMIME_RECIPIENT_TYPE_TO  },
+	{ "cc",         NULL, GMIME_RECIPIENT_TYPE_CC  },
+	{ "bcc",        NULL, GMIME_RECIPIENT_TYPE_BCC }
+    };
+    const char *from_addr = NULL;
+    unsigned int i;
+
+    for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {
+	const char *addr, *recipients;
+
+	recipients = notmuch_message_get_header (message,
+						 reply_to_map[i].header);
+	if ((recipients == NULL || recipients[0] == '\0') && reply_to_map[i].fallback)
+	    recipients = notmuch_message_get_header (message,
+						     reply_to_map[i].fallback);
+
+	addr = add_recipients_for_string (reply, config,
+					  reply_to_map[i].recipient_type,
+					  recipients);
+	if (from_addr == NULL)
+	    from_addr = addr;
+    }
+    return from_addr;
+}
+
 static int
 notmuch_reply_format_default(void *ctx, notmuch_config_t *config, notmuch_query_t *query)
 {
     GMimeMessage *reply;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
-    const char *subject, *recipients, *from_addr = NULL;
+    const char *subject, *from_addr = NULL;
     const char *in_reply_to, *orig_references, *references;
     char *reply_headers;
-    unsigned int i;
 
     for (messages = notmuch_query_search_messages (query);
 	 notmuch_messages_has_more (messages);
@@ -223,21 +252,7 @@ notmuch_reply_format_default(void *ctx, notmuch_config_t *config, notmuch_query_
 	    subject = talloc_asprintf (ctx, "Re: %s", subject);
 	g_mime_message_set_subject (reply, subject);
 
-	for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {
-	    const char *addr;
-
-	    recipients = notmuch_message_get_header (message,
-						     reply_to_map[i].header);
-	    if ((recipients == NULL || recipients[0] == '\0') && reply_to_map[i].fallback)
-		recipients = notmuch_message_get_header (message,
-							 reply_to_map[i].fallback);
-
-	    addr = add_recipients_for_string (reply, config,
-					      reply_to_map[i].recipient_type,
-					      recipients);
-	    if (from_addr == NULL)
-		from_addr = addr;
-	}
+	from_addr = add_recipients_from_message (reply, config, message);
 
 	if (from_addr == NULL)
 	    from_addr = notmuch_config_get_user_primary_email (config);
@@ -290,9 +305,8 @@ notmuch_reply_format_headers_only(void *ctx, notmuch_config_t *config, notmuch_q
     GMimeMessage *reply;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
-    const char *recipients, *in_reply_to, *orig_references, *references;
+    const char *in_reply_to, *orig_references, *references;
     char *reply_headers;
-    unsigned int i;
 
     for (messages = notmuch_query_search_messages (query);
 	 notmuch_messages_has_more (messages);
@@ -326,19 +340,7 @@ notmuch_reply_format_headers_only(void *ctx, notmuch_config_t *config, notmuch_q
 	g_mime_object_set_header (GMIME_OBJECT (reply),
 				  "References", references);
 
-	for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {
-	    const char *addr;
-
-	    recipients = notmuch_message_get_header (message,
-						     reply_to_map[i].header);
-	    if ((recipients == NULL || recipients[0] == '\0') && reply_to_map[i].fallback)
-		recipients = notmuch_message_get_header (message,
-							 reply_to_map[i].fallback);
-
-	    addr = add_recipients_for_string (reply, config,
-					      reply_to_map[i].recipient_type,
-					      recipients);
-	}
+	(void)add_recipients_from_message (reply, config, message);
 
 	g_mime_object_set_header (GMIME_OBJECT (reply), "Bcc",
 			   notmuch_config_get_user_primary_email (config));
-- 
1.6.5.3

[PATCH 2/2] notmuch-reply.c: Handle munged `Reply-To' headers.

[Jed Brown]

Some mailing lists engage in the evil practice of changing the Reply-To
header so that replies from all mailers go to the list by default, at
the expense of not responding to the person who actually sent the
message.  When this is detected, we reply to `From' and remove the
duplicate response to the mailing list.  Consider a reply to the
following message.

  From: Some User <some.user@example.com>
  To: Sample users list <sample-users@sample.org>
  Reply-To: Sample users list <sample-users@sample.org>

Prior to this patch, `notmuch reply' produces

  To: Sample users list <sample-users@sample.org>,
      Sample users list <sample-users@sample.org>

and after the patch,

  To: Some User <some.user@example.com>,
      Sample users list <sample-users@sample.org>

Signed-off-by: Jed Brown <jed@59A2.org>
---
 notmuch-reply.c |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index b91a830..9b9e9ab 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -182,6 +182,39 @@ add_recipients_for_string (GMimeMessage *message,
     return add_recipients_for_address_list (message, config, type, list);
 }
 
+/* Some mailing lists munge the Reply-To header despite it being A Bad
+ * Thing, see http://www.unicom.com/pw/reply-to-harmful.html
+ *
+ * This function detects such munging so that reasonable headers can be
+ * generated anyway.  Returns 1 if munged, else 0.
+ *
+ * The current logic is fairly naive, Reply-To is diagnosed as munged if
+ * it contains exactly one address, and this address is also present in
+ * the To or Cc fields.
+ */
+static int
+mailing_list_munged_reply_to (notmuch_message_t *message)
+{
+    const char *header, *addr;
+    InternetAddressList *list;
+    InternetAddress *address;
+    InternetAddressMailbox *mailbox;
+
+    header = notmuch_message_get_header (message, "reply-to");
+    list = internet_address_list_parse_string (header);
+    if (internet_address_list_length (list) != 1)
+	return 0;
+    address = internet_address_list_get_address (list, 0);
+    if (INTERNET_ADDRESS_IS_GROUP (address))
+	return 0;
+    mailbox = INTERNET_ADDRESS_MAILBOX (address);
+    addr = internet_address_mailbox_get_addr (mailbox);
+    /* Note that strcasestr() is a GNU extension, strstr() might be sufficient */
+    if (strcasestr (notmuch_message_get_header (message, "to"), addr) == 0 ||
+	strcasestr (notmuch_message_get_header (message, "cc"), addr) == 0)
+	return 1;
+    return 0; }
+
 /* Augments the recipients of reply from the headers of message.
  *
  * If any of the user's addresses were found in these headers, the first
@@ -192,7 +225,7 @@ add_recipients_from_message (GMimeMessage *reply,
 			     notmuch_config_t *config,
 			     notmuch_message_t *message)
 {
-    static const struct {
+    struct {
 	const char *header;
 	const char *fallback;
 	GMimeRecipientType recipient_type;
@@ -205,6 +238,18 @@ add_recipients_from_message (GMimeMessage *reply,
     const char *from_addr = NULL;
     unsigned int i;
 
+    /* When we have detected Reply-To munging, we ignore the Reply-To
+     * field (because it appears in the To or Cc headers) and use the
+     * From header so that person will get pinged and will actually
+     * receive the response if not subscribed to the list.  Note that
+     * under no circumstances does this fail to reply to the address in
+     * the Reply-To header.
+     */
+    if (mailing_list_munged_reply_to (message)) {
+	reply_to_map[0].header = "from";
+	reply_to_map[0].fallback = NULL;
+    }
+
     for (i = 0; i < ARRAY_SIZE (reply_to_map); i++) {
 	const char *addr, *recipients;
 
-- 
1.6.5.3

[PATCH] notmuch-reply.c: early exit for munged Reply-To when header is not present

[Jed Brown]

This avoids erroneously calling internet_address_list_length() with a
NULL list (which was causing a gmime exception).

Signed-off-by: Jed Brown <jed@59A2.org>
---
 notmuch-reply.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 9b9e9ab..6c9118e 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -202,7 +202,7 @@ mailing_list_munged_reply_to (notmuch_message_t *message)
 
     header = notmuch_message_get_header (message, "reply-to");
     list = internet_address_list_parse_string (header);
-    if (internet_address_list_length (list) != 1)
+    if (list == NULL || internet_address_list_length (list) != 1)
 	return 0;
     address = internet_address_list_get_address (list, 0);
     if (INTERNET_ADDRESS_IS_GROUP (address))
-- 
1.6.5.3

Re: [PATCH 2/2] notmuch-reply.c: Handle munged `Reply-To' headers.

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1662 bytes --]

On Sun, 29 Nov 2009 00:19:36 +0100, Jed Brown <jed@59A2.org> wrote:
> Some mailing lists engage in the evil practice of changing the Reply-To
> header so that replies from all mailers go to the list by default, at
> the expense of not responding to the person who actually sent the
> message.  When this is detected, we reply to `From' and remove the
> duplicate response to the mailing list.  Consider a reply to the
> following message.

Thanks for the patch, Jed!

I've gone ahead and pushed this out, (along with some cleanups/fixes).

I augmented the notmuch test suite to actually test this case, (and the
related case of honoring Reply-To for a non-munged message). And I'm
glad I did because that turned up a bug in the patch, (using == instead
of != for the return value of strcasestr resulted in *all* messages with
a Reply-To header being considered as munged).

Here's one cleanup I made which you might find interesting as a style
issue (where I prefer naming a function based on what it *does* rather
than on what it's being *used* for):

commit d111c720bad53f98edba958aa605e857036a2bc8
Author: Carl Worth <cworth@cworth.org>
Date:   Thu Feb 4 12:39:11 2010 -0800

    notmuch reply: Rename the mailing_list_munged_reply_to function
    
    This function detects whether the address in the Reply-To header
    already appears in either To or Cc. So give it a name that reflects
    what it does (reply_to_header_is_redundant) rather than the old name
    which described one possible use of the function, (as a simple
    heuristic for detecting whether a mailing list had applied reply-to
    munging).

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/2] notmuch-reply.c: Handle munged `Reply-To' headers.

[Jed Brown]

On Thu, 04 Feb 2010 12:54:20 -0800, Carl Worth <cworth@cworth.org> wrote:
> And I'm glad I did because that turned up a bug in the patch, (using
> == instead of != for the return value of strcasestr resulted in *all*
> messages with a Reply-To header being considered as munged).

Yikes, I've been using this thing for two months and hadn't noticed.

> Here's one cleanup I made which you might find interesting as a style
> issue (where I prefer naming a function based on what it *does* rather
> than on what it's being *used* for):

Yup, I do the same, but must have been too lazy to think of a decent name.

Jed