Re: [PATCH v2] Let mailsplit and mailinfo handle mails with CRLF line-endings

From: Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil>
Date: Aug 4, 2009, 21:29 UTC
Message-ID: <H9KDJzU0XgndJHC8J4OETn7kxYhGP3gl0YJPR0low0hfIHjZOUsoxw@cipher.nrlssc.navy.mil>
In-Reply-To: <20090804210327.GA23747@blimp.localdomain>

Alex Riesen wrote:

Show 30 quoted lines

> Noticed by H. Peter Anvin.
> 
> It is not that uncommon to have mails with DOS line-ending,
> notably Thunderbird and web mailers like Gmail (when saving
> what they call "original" message).
> 
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ---
> 
> Corrected bug with unconditonal last (or very long) line shortening if
> it contains a CR in next-to-last character. Noticed by Sverre Rabbelier.
> 
> It should also handle the case mentioned by Brandon Casey.
> 
>  builtin-mailsplit.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
> index ad5f6b5..48285a0 100644
> --- a/builtin-mailsplit.c
> +++ b/builtin-mailsplit.c
> @@ -58,6 +58,8 @@ int read_line_with_nul(char *buf, int size, FILE *in)
>  		if (c == '\n' || len + 1 >= size)
>  			break;
>  	}
> +	if (c == '\n' && len > 1 && buf[len - 2] == '\r')
> +		buf[--len - 1] = '\n';
>  	buf[len] = '\0';
>  
>  	return len;

What if \r lands at character 99 and \n is at character 100? If buf has exactly 100 characters available for writing. Wouldn't the '\r' be stored into buf, but not the '\n'? Then the 'for' loop would terminate since len + 1 would be >= size and the code above would test whether c == '\n', and it would not, so the '\r' would not be removed from buf as it should be.

At the point where buf has been filled, and the last character read is a '\r', we can not tell whether the next character is a '\n' or not, so we do not know if it is a solitary '\r' or whether it is the start of a '\r\n' sequence. It seems to me that we must push the '\r' back into the stream and allow the next call to read_line_with_nul() handle it, or peek to see if the next character is a '\n'.

Be aware that if we use the version I suggested which pushes the '\r' back into the input stream, I think we risk an infinite loop if size == 1. I don't think that is possible from the current callers though.

-brandon