[PATCH] notify extension: truncate lines to 998 characters to conform to RFC 5322

Matt Mackall mpm at selenic.com
Thu Oct 27 11:09:46 CDT 2011


On Thu, 2011-10-27 at 14:51 +0100, William Gallafent wrote:
> According to RFC 5322 ( http://www.rfc-editor.org/rfc/rfc5322.txt )
> section 2.1.1, lines in internet messages must not exceed 998
> characters in length. The Notify extension does not enforce this when
> sending emails containing the result of a diff. The following patch
> fixes this by truncating any such lines found in the diff output,
> adding the conventional "..." to indicate that truncation has taken
> place on that line, and preceding the diff output with a warning that
> the truncation has taken place.

Looks like we have a case of battling, evolving standards from the early
70s.

Such a truncated diff is no longer a correct diff. Applying it may in
fact result in '...' ending up in the source erroneously. So at a
minimum, the warning needs to end up -in the email-, preferably right in
the middle of the diff where the truncation occurred to prevent it from
applying.

But we've also got the patchbomb extension, which will have the same
issue. And there, the whole point is for the remote end to apply the
patch. Intentionally crippling a patch there is a non-starter.
I'm not sure whether it's better to send a correct patch and hope the
client doesn't choke on it, or simply abort and force the user to use an
attachment.

Notably, RFC821 (1982) has this to say about line lengths:

          ****************************************************
          *                                                  *
          *  TO THE MAXIMUM EXTENT POSSIBLE, IMPLEMENTATION  *
          *  TECHNIQUES WHICH IMPOSE NO LIMITS ON THE LENGTH *
          *  OF THESE OBJECTS SHOULD BE USED.                *
          *                                                  *
          ****************************************************



> I use the approach given at
> http://www.guyrutenberg.com/2007/10/12/conditional-expressions-in-python-24/ to retain something similar to conditional expression syntax within the list comprehension, allowing it to work with Python 2.4.
> 
> Apologies for unwrapped paragraphs above, Thunderbird's composition
> window not letting me wrap the text but not wrap the diff! This diff
> just comes from my live installation, I don't have a development tree
> installed here. I hope that its simplicity means it is still
> acceptable.
> 
> --- /usr/share/pyshared/hgext/notify.py~	2011-08-01 23:08:59.000000000 +0000
> +++ /usr/share/pyshared/hgext/notify.py	2011-10-27 13:06:12.000000000 +0000
> @@ -257,6 +257,11 @@
>          chunks = patch.diff(self.repo, prev, ref, opts=patch.diffopts(self.ui))
>          difflines = ''.join(chunks).splitlines()
>  
> +        if any(len(l)>998 for l in difflines):

The any function is post-py2.4-ism. Simpler to just use a for loop over
the lines. Or util.any.

> +            msg = _('\nwarning: long diff lines truncated to 998 characters to conform to RFC5322.\n')
> +            self.ui.write(msg)

ui.warning would be more appropriate here, but who's going to read it?
It'll generally end up in an web server log file.

> +            difflines = [(l[:995]+"...",l)[len(l)<999] for l in difflines]
> +

You'll notice we use PEP8 spacing around operators like '>', '+', and
','.

>          if self.ui.configbool('notify', 'diffstat', True):
>              s = patch.diffstat(difflines)
>              # s may be nil, don't include the header if it is
> 
> 

 
-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list