From: Jani Nikula Date: Sat, 12 May 2012 17:23:16 +0000 (+0300) Subject: Re: [PATCH 2/2] cli: clean up user address matching code in guess_from_received_header() X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=fc755f6f64cc1b58fb691ab005768ebf32c82b55;p=notmuch-archives.git Re: [PATCH 2/2] cli: clean up user address matching code in guess_from_received_header() --- diff --git a/5d/f4d8f2977afc048a57c2151c362235c0d7c00c b/5d/f4d8f2977afc048a57c2151c362235c0d7c00c new file mode 100644 index 000000000..11fcd49d8 --- /dev/null +++ b/5d/f4d8f2977afc048a57c2151c362235c0d7c00c @@ -0,0 +1,215 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 87EF0431FB6 + for ; Sat, 12 May 2012 10:23:24 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +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 7Lisn6X46AHH for ; + Sat, 12 May 2012 10:23:23 -0700 (PDT) +Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com + [209.85.217.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 1BA66431FAE + for ; Sat, 12 May 2012 10:23:22 -0700 (PDT) +Received: by lbbgk8 with SMTP id gk8so2797909lbb.26 + for ; Sat, 12 May 2012 10:23:20 -0700 (PDT) +X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=google.com; s=20120113; + h=from:to:cc:subject:in-reply-to:references:user-agent:date + :message-id:mime-version:content-type:x-gm-message-state; + bh=Mm7vU5wOMu/hoWZTzXF8/5XrT2vHlc2iTkKtmFNi5Jw=; + b=FwD2JnXEPgP/Nv7TCAq2hWw86aug0RULkPw+C6ajsYdXwFo0+/gDZVuWjSB+zGUqEG + oe87wze+dT5z9w8/TvTLAnLFssoZo0swmxON0d/PpGR/2Z9SeN3uiHgzu6JGjmMMdJqV + 1r+wLR65k5VV2f5jjFdJ/q6NYW5euige/F5lJPl/WC+bOGvMLMP7DUzrHznla+fuLx5a + E1/uhD0l2eSkyPW2a6Zwi5y/f9msoOBLUhTq2Wk0ZCJa5xgU1hFiGNqOxiMkpns20ECb + 9WnL/R7iSIBQ9a5bPWiDr6+bU7Q2I7/9IMi2ehvAGLR3lvm23ty3SXWpiTM+zt+KYBfr + eoNg== +Received: by 10.112.98.137 with SMTP id ei9mr973937lbb.102.1336843400107; + Sat, 12 May 2012 10:23:20 -0700 (PDT) +Received: from localhost (dsl-hkibrasgw4-fe50dc00-68.dhcp.inet.fi. + [80.220.80.68]) + by mx.google.com with ESMTPS id i6sm16636115lbg.2.2012.05.12.10.23.17 + (version=SSLv3 cipher=OTHER); Sat, 12 May 2012 10:23:18 -0700 (PDT) +From: Jani Nikula +To: Austin Clements +Subject: Re: [PATCH 2/2] cli: clean up user address matching code in + guess_from_received_header() +In-Reply-To: <20120512154746.GK11804@mit.edu> +References: + + + <20120512154746.GK11804@mit.edu> +User-Agent: Notmuch/0.12+169~g45438b0 (http://notmuchmail.org) Emacs/23.3.1 + (i686-pc-linux-gnu) +Date: Sat, 12 May 2012 20:23:16 +0300 +Message-ID: <87k40h1fgb.fsf@nikula.org> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Gm-Message-State: + ALoCoQnNgvbFzQ1bMidIifUj3dpwUmPRQlG1ShYbb5Pvt66IiFoKEJlxX2xkzfUqzpfd7WFJwkP0 +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Sat, 12 May 2012 17:23:24 -0000 + +On Sat, 12 May 2012, Austin Clements wrote: +> Series LGTM. This is a nice cleanup. + +Thanks for the review. + +> My one comment is that it seems like we should be stricter about +> matching email address lists, since a naive substring match could +> yield strange results. That's not something that should be changed in +> this patch, though. + +Agreed. Especially the Received: header matching should be scrutinized, +as *all* the Received: headers in a message are merged into one by +notmuch_message_file_get_header(). But that should be another patch, +another time. + +> Does the context of this patch conflict with +> id:"21a946917c5c8dd63295b7c87b7c2d1ebcb6e71e.1336746160.git.jani@nikula.org" +> ? It seems like both have context that's changed by the other patch. + +Ooops, you're right. The two series do *merge* on top of each other in +either order, but as patches they apply only on master. I wanted to keep +the series separate. Once either gets pushed, I'll rebase the other. + +BR, +Jani. + +> +> Quoth Jani Nikula on May 11 at 5:33 pm: +>> Get rid of user address matching code duplication in +>> guess_from_received_header() by using the new address matching +>> helpers. +>> +>> No functional changes. +>> +>> Signed-off-by: Jani Nikula +>> --- +>> notmuch-reply.c | 64 +++++++++++++++++-------------------------------------- +>> 1 file changed, 19 insertions(+), 45 deletions(-) +>> +>> diff --git a/notmuch-reply.c b/notmuch-reply.c +>> index 0c82755..51cb6de 100644 +>> --- a/notmuch-reply.c +>> +++ b/notmuch-reply.c +>> @@ -377,20 +377,15 @@ add_recipients_from_message (GMimeMessage *reply, +>> static const char * +>> guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message) +>> { +>> - const char *received,*primary,*by; +>> - const char **other; +>> - char *tohdr; +>> + const char *addr, *received, *by; +>> char *mta,*ptr,*token; +>> char *domain=NULL; +>> char *tld=NULL; +>> const char *delim=". \t"; +>> - size_t i,j,other_len; +>> + size_t i; +>> +>> const char *to_headers[] = {"Envelope-to", "X-Original-To"}; +>> +>> - primary = notmuch_config_get_user_primary_email (config); +>> - other = notmuch_config_get_user_other_email (config, &other_len); +>> - +>> /* sadly, there is no standard way to find out to which email +>> * address a mail was delivered - what is in the headers depends +>> * on the MTAs used along the way. So we are trying a number of +>> @@ -405,23 +400,13 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message +>> * 'by' part of Received headers +>> * If none of these work, we give up and return NULL +>> */ +>> - for (i = 0; i < sizeof(to_headers)/sizeof(*to_headers); i++) { +>> - tohdr = xstrdup(notmuch_message_get_header (message, to_headers[i])); +>> - if (tohdr && *tohdr) { +>> - /* tohdr is potentialy a list of email addresses, so here we +>> - * check if one of the email addresses is a substring of tohdr +>> - */ +>> - if (strcasestr(tohdr, primary)) { +>> - free(tohdr); +>> - return primary; +>> - } +>> - for (j = 0; j < other_len; j++) +>> - if (strcasestr (tohdr, other[j])) { +>> - free(tohdr); +>> - return other[j]; +>> - } +>> - free(tohdr); +>> - } +>> + for (i = 0; i < ARRAY_SIZE (to_headers); i++) { +>> + const char *tohdr = notmuch_message_get_header (message, to_headers[i]); +>> + +>> + /* Note: tohdr potentially contains a list of email addresses. */ +>> + addr = user_address_in_string (tohdr, config); +>> + if (addr) +>> + return addr; +>> } +>> +>> /* We get the concatenated Received: headers and search from the +>> @@ -439,19 +424,12 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message +>> * header +>> */ +>> ptr = strstr (received, " for "); +>> - if (ptr) { +>> - /* the text following is potentialy a list of email addresses, +>> - * so again we check if one of the email addresses is a +>> - * substring of ptr +>> - */ +>> - if (strcasestr(ptr, primary)) { +>> - return primary; +>> - } +>> - for (i = 0; i < other_len; i++) +>> - if (strcasestr (ptr, other[i])) { +>> - return other[i]; +>> - } +>> - } +>> + +>> + /* Note: ptr potentially contains a list of email addresses. */ +>> + addr = user_address_in_string (ptr, config); +>> + if (addr) +>> + return addr; +>> + +>> /* Finally, we parse all the " by MTA ..." headers to guess the +>> * email address that this was originally delivered to. +>> * We extract just the MTA here by removing leading whitespace and +>> @@ -492,15 +470,11 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message +>> */ +>> *(tld-1) = '.'; +>> +>> - if (strcasestr(primary, domain)) { +>> - free(mta); +>> - return primary; +>> + addr = string_in_user_address (domain, config); +>> + if (addr) { +>> + free (mta); +>> + return addr; +>> } +>> - for (i = 0; i < other_len; i++) +>> - if (strcasestr (other[i],domain)) { +>> - free(mta); +>> - return other[i]; +>> - } +>> } +>> free (mta); +>> }