be/2e62d51a6f09d22fe87b0054aea335562370ed

   1 Return-Path: <m.walters@qmul.ac.uk>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 9539C431FAF\r
   6         for <notmuch@notmuchmail.org>; Wed,  8 Aug 2012 00:31:17 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -1.098\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
  12         tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
  13         NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id 66m1KglKLmZl for <notmuch@notmuchmail.org>;\r
  17         Wed,  8 Aug 2012 00:31:13 -0700 (PDT)\r
  18 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id AF15F431FAE\r
  22         for <notmuch@notmuchmail.org>; Wed,  8 Aug 2012 00:31:13 -0700 (PDT)\r
  23 Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
  24         by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
  25         (envelope-from <m.walters@qmul.ac.uk>)\r
  26         id 1Sz0jS-0000ni-5Y; Wed, 08 Aug 2012 08:31:08 +0100\r
  27 Received: from 94-192-233-223.zone6.bethere.co.uk ([94.192.233.223]\r
  28         helo=localhost)\r
  29         by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.69)\r
  30         (envelope-from <m.walters@qmul.ac.uk>)\r
  31         id 1Sz0jR-00009p-Pt; Wed, 08 Aug 2012 08:31:06 +0100\r
  32 From: Mark Walters <markwalters1009@gmail.com>\r
  33 To: Austin Clements <amdragon@MIT.EDU>, Ben Gamari <bgamari.foss@gmail.com>,\r
  34         notmuch@notmuchmail.org\r
  35 Subject: Re: [PATCH] sprinters: bugfix when NULL passed for a string.\r
  36 In-Reply-To: <87ipcut9r4.fsf@awakening.csail.mit.edu>\r
  37 References: <871ujjuu2z.fsf@gmail.com> <878vdrp4d9.fsf@qmul.ac.uk>\r
  38         <874noe1o0r.fsf@qmul.ac.uk>\r
  39         <87ipcut9r4.fsf@awakening.csail.mit.edu>\r
  40 User-Agent: Notmuch/0.13.2+96~g634443c (http://notmuchmail.org) Emacs/23.4.1\r
  41         (x86_64-pc-linux-gnu)\r
  42 Date: Wed, 08 Aug 2012 08:31:00 +0100\r
  43 Message-ID: <878vdp3knf.fsf@qmul.ac.uk>\r
  44 MIME-Version: 1.0\r
  45 Content-Type: text/plain; charset=us-ascii\r
  46 X-Sender-Host-Address: 94.192.233.223\r
  47 X-QM-SPAM-Info: Sender has good ham record.  :)\r
  48 X-QM-Body-MD5: 84b053bed1825c7735338e4e4331007e (of first 20000 bytes)\r
  49 X-SpamAssassin-Score: -1.8\r
  50 X-SpamAssassin-SpamBar: -\r
  51 X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
  52         determine if it is\r
  53         spam. We require at least 5.0 points to mark a message as spam.\r
  54         This message scored -1.8 points.\r
  55         Summary of the scoring: \r
  56         * -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/,\r
  57         *      medium trust\r
  58         *      [138.37.6.40 listed in list.dnswl.org]\r
  59         * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
  60         provider *      (markwalters1009[at]gmail.com)\r
  61         * -0.0 T_RP_MATCHES_RCVD Envelope sender domain matches handover relay\r
  62         *      domain\r
  63         *  0.5 AWL AWL: From: address is in the auto white-list\r
  64 X-QM-Scan-Virus: ClamAV says the message is clean\r
  65 X-BeenThere: notmuch@notmuchmail.org\r
  66 X-Mailman-Version: 2.1.13\r
  67 Precedence: list\r
  68 List-Id: "Use and development of the notmuch mail system."\r
  69         <notmuch.notmuchmail.org>\r
  70 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  71         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  72 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  73 List-Post: <mailto:notmuch@notmuchmail.org>\r
  74 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  75 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  76         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  77 X-List-Received-Date: Wed, 08 Aug 2012 07:31:17 -0000\r
  78 \r
  79 \r
  80 On Wed, 08 Aug 2012, Austin Clements <amdragon@MIT.EDU> wrote:\r
  81 > LGTM.\r
  82 >\r
  83 > This won't commute with [0], since that introduces broken tests that are\r
  84 > fixed by this patch.\r
  85 \r
  86 Yes: I guess the tidiest might be for me to send a version of my patch\r
  87 which marks these tests fixed so it can be applied after this.\r
  88 \r
  89 > I think we should remove the fields in the JSON header object for\r
  90 > missing headers (except perhaps From and Date, if those really are\r
  91 > mandatory headers), but I think we should do that after the freeze,\r
  92 > since it might affect frontends.  \r
  93 \r
  94 I agree with you and Jamie on this. I think it is a `feature' rather\r
  95 than a bugfix (the current patch just restores the output to what it was\r
  96 before the sprinter changes it) so agree it should be after the\r
  97 freeze. We could deprecate passing NULL to sprinter string functions\r
  98 (possibly keep the check but fprintf a warning?)\r
  99 \r
 100 For the From/Date headers see http://www.ietf.org/rfc/rfc2822.txt\r
 101 section 3.6 for details:\r
 102 \r
 103    The only required header fields are the origination date field and\r
 104    the originator address field(s).  All other header fields are\r
 105    syntactically optional.\r
 106 \r
 107 and origination date field means a Date: header, and originator\r
 108 address field means a From: header. However, I don't think an empty\r
 109 string is valid for either of these so we can't sensibly output\r
 110 something standards compliant. Thus I would go for following the\r
 111 original message and omit any fields not present in it.\r
 112 \r
 113 > It probably makes sense to add an Emacs test or two to the tests in [0].\r
 114 \r
 115 This seems like a good idea.\r
 116 \r
 117 Best wishes\r
 118 \r
 119 Mark\r
 120 \r
 121 \r
 122 > [0] id:"1344389313-7886-1-git-send-email-amdragon@mit.edu"\r
 123 >\r
 124 > On Tue, 07 Aug 2012, Mark Walters <markwalters1009@gmail.com> wrote:\r
 125 >> The string function in a sprinter may be called with a NULL string\r
 126 >> pointer (eg if a header is absent). This causes a segfault. We fix\r
 127 >> this by checking for a null pointer in the string functions and update\r
 128 >> the sprinter documentation.\r
 129 >>\r
 130 >> At the moment some output when format=text is done directly rather than\r
 131 >> via an sprinter: in that case a null pointer is passed to printf or\r
 132 >> similar and a "(null)" appears in the output. That behaviour is not\r
 133 >> changed in this patch.\r
 134 >> ---\r
 135 >>\r
 136 >> This could really do with some tests (it is the second time this type of\r
 137 >> bug has occurred). To be considered as a message by notmuch new a file\r
 138 >> needs at least one of a From: Subject: or To: header. Thus we should\r
 139 >> have three messages each of which just contains that single header (and\r
 140 >> nothing else) and check that search and show work as expected. \r
 141 >>\r
 142 >>\r
 143 >>\r
 144 >>  sprinter-json.c |    2 ++\r
 145 >>  sprinter-text.c |    2 ++\r
 146 >>  sprinter.h      |    4 +++-\r
 147 >>  3 files changed, 7 insertions(+), 1 deletions(-)\r
 148 >>\r
 149 >> diff --git a/sprinter-json.c b/sprinter-json.c\r
 150 >> index c9b6835..0a07790 100644\r
 151 >> --- a/sprinter-json.c\r
 152 >> +++ b/sprinter-json.c\r
 153 >> @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, size_t len)\r
 154 >>  static void\r
 155 >>  json_string (struct sprinter *sp, const char *val)\r
 156 >>  {\r
 157 >> +    if (val == NULL)\r
 158 >> +    val = "";\r
 159 >>      json_string_len (sp, val, strlen (val));\r
 160 >>  }\r
 161 >>  \r
 162 >> diff --git a/sprinter-text.c b/sprinter-text.c\r
 163 >> index dfa54b5..10343be 100644\r
 164 >> --- a/sprinter-text.c\r
 165 >> +++ b/sprinter-text.c\r
 166 >> @@ -38,6 +38,8 @@ text_string_len (struct sprinter *sp, const char *val, size_t len)\r
 167 >>  static void\r
 168 >>  text_string (struct sprinter *sp, const char *val)\r
 169 >>  {\r
 170 >> +    if (val == NULL)\r
 171 >> +    val = "";\r
 172 >>      text_string_len (sp, val, strlen (val));\r
 173 >>  }\r
 174 >>  \r
 175 >> diff --git a/sprinter.h b/sprinter.h\r
 176 >> index 5f43175..912a526 100644\r
 177 >> --- a/sprinter.h\r
 178 >> +++ b/sprinter.h\r
 179 >> @@ -27,7 +27,9 @@ typedef struct sprinter {\r
 180 >>       * a list or map, followed or preceded by separators).  For string\r
 181 >>       * and string_len, the char * must be UTF-8 encoded.  string_len\r
 182 >>       * allows non-terminated strings and strings with embedded NULs\r
 183 >> -     * (though the handling of the latter is format-dependent).\r
 184 >> +     * (though the handling of the latter is format-dependent). For\r
 185 >> +     * string (but not string_len) the string pointer passed may be\r
 186 >> +     * NULL.\r
 187 >>       */\r
 188 >>      void (*string) (struct sprinter *, const char *);\r
 189 >>      void (*string_len) (struct sprinter *, const char *, size_t);\r
 190 >> -- \r
 191 >> 1.7.9.1\r
 192 >>\r
 193 >>\r
 194 >> H\r
 195 >> _______________________________________________\r
 196 >> notmuch mailing list\r
 197 >> notmuch@notmuchmail.org\r
 198 >> http://notmuchmail.org/mailman/listinfo/notmuch\r