Thread (24 messages) flat view 24 messages, 8 authors, 2016-06-15

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

From: Brandon Casey <hidden>
Date: 2016-06-15 22:47:09

Alex Riesen wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help