250559 - don't recognize .CR not as message terminator

Status: RESOLVED FIXED

(MailNews Core :: Networking: POP, defect)

Description

[Christian Eyrich]

According to RFC 1939 only .CRLF has to be accepted as terminating.
But currently we also accept .LF and .CR as terminator. The former is because of
restriction in ReadNextLine().
But the later is a bug in HandleLine() that can easily be fixed.

Comment 1

[Christian Eyrich]

Attachment: proposed patch

This patch is basically a one-liner:
-    if ((line[0] == '.') &&
-	 ((line[1] == nsCRT::CR) || (line[1] == nsCRT::LF)))
+    // line contains only dot and linebreak -> message end
+    if (line[0] == '.' && line_length == 1 + MSG_LINEBREAK_LEN)

Currently everything with dot in the first and CR or LF in the second line is
treated as terminator, also ".\rThis is no terminator"\r\n. This patch changes
this. It tests if the dot is alone on a line with MSG_LINEBREAK (inserted by
|res = BufferInput(MSG_LINEBREAK, MSG_LINEBREAK_LEN);| in
nsPop3Protocol::RetrResponse()).

All the other changes are just cleanups. The removed code junk at #2919 and
following is never triggered, so removed.

Comment 2

[David :Bienvenu]

Comment on attachment 152684 [details] [diff] [review]
proposed patch

is there an advantage to doing this, other than more conformance with the RFC?
Could it break us with some poorly behaved servers?

Comment 3

[Christian Eyrich]

It can break us with those servers. Though the servers not sending CRLF I know
send LF, not CR.
Also accepting single CR and LF can make us terminate the message to early, so I
think the drawback isn't bigger than the gain. Besides from my position that
faulty servers shouldn't make us violate RFC's.

Comment 4

[David :Bienvenu]

fixed on trunk.