Bugzilla

The first thing I noticed, looking with network sniffer wireshark, is that when a message is downloaded with the pop3 RETR command, each line of data is terminated with \n (LF). The POP3 RFC specifies that each response line terminates with \r\n (CRLF).  However, responses to other commands generated directly by the server (e.g., CAPA, LIST, UIDL etc) are correctly terminated with CRLF. Also, the server correctly sends the final RETR response with ```\r\n.\r\n``` (```CRLF.CRLF```). (I didn't test it but I expected the server's TOP response has the same issue.)

Currently TB expects CRLF as line breaks so it was seeing the whole message as one long line. I made a change to LineReader.sys.mjs so that just LF is treated as valid and that fixed the problem.

I then was wondering if the problem exists in older TB versions. So I have a computer still running TB 68 and it worked OK with the test account.

I then noticed that the reporter had mention that the account was previously running on 115. I wasn't sure if there was a problem with 115 so I tested it and it also worked OK with the test account.

I then moved the 115 pop3 parsing and protocol JS code (LineReader.sys.mjs and Pop3Client.sys.mjs) into daily and it also had the same problem.

Then I went back to unchanged daily and ran with 115 nsPop3Sink.cpp and it worked OK.  I then compared the nsPop3Sink.cpp code between daily and 115 and noticed one change pertaining to line endings that has been removed in daily (and is also not in 128). With this change https://phabricator.services.mozilla.com/D190226, the call to write CRLF (MSG_LINEBREAK) was removed in ```mailnews/local/src/nsPop3Sink.cpp``` at line 579 with no explanation that I could find. Putting this back in, as seen in the attached patch, fixes the bug without doing any other changes.

Maybe there's a good reason to remove the write of a final CRLF so I'll ask Ben. (My guess would be that since all POP3 lines are supposed to terminate with CRLF, there is no reason to write an extra one at the end of file.)

The first thing I noticed, looking with network sniffer wireshark, is that when a message is downloaded with the pop3 RETR command, each line of data is terminated with \n (LF). The POP3 RFC specifies that each response line terminates with \r\n (CRLF).  However, responses to other commands generated directly by the server (e.g., CAPA, LIST, UIDL etc) are correctly terminated with CRLF. Also, the server correctly sends the final RETR response with ```\r\n.\r\n``` (```CRLF.CRLF```). (I didn't test it but I expected the server's TOP response has the same issue.)

Currently TB expects CRLF as line breaks so it was seeing the whole message as one long line. I made a not trivial change to LineReader.sys.mjs so that just LF is treated as valid and that fixed the problem.

I then was wondering if the problem exists in older TB versions. So I have a computer still running TB 68 and it worked OK with the test account.

I then noticed that the reporter had mention that the account was previously running on 115. I wasn't sure if there was a problem with 115 so I tested it and it also worked OK with the test account.

I then moved the 115 pop3 parsing and protocol JS code (LineReader.sys.mjs and Pop3Client.sys.mjs) into daily and it also had the same problem.

Then I went back to unchanged daily and ran with 115 nsPop3Sink.cpp and it worked OK.  I then compared the nsPop3Sink.cpp code between daily and 115 and noticed one change pertaining to line endings that has been removed in daily (and is also not in 128). With this change https://phabricator.services.mozilla.com/D190226, the call to write CRLF (MSG_LINEBREAK) was removed in ```mailnews/local/src/nsPop3Sink.cpp``` at line 579 with no explanation that I could find. Putting this back in, as seen in the attached patch, fixes the bug without doing any other changes.

Maybe there's a good reason to remove the write of a final CRLF so I'll ask Ben. (My guess would be that since all POP3 lines are supposed to terminate with CRLF, there is no reason to write an extra one at the end of file.)

The first thing I noticed, looking with network sniffer wireshark, is that when a message is downloaded with the pop3 RETR command, each line of data is terminated with \n (LF). The POP3 RFC specifies that each response line terminates with \r\n (CRLF).  However, responses to other commands generated directly by the server (e.g., CAPA, LIST, UIDL etc) are correctly terminated with CRLF. Also, the server correctly sends the final RETR response with ```\r\n.\r\n``` (```CRLF.CRLF```). (I didn't test it but I expected the server's TOP response has the same issue.)

Currently TB expects CRLF as line breaks so it was seeing the whole message as one long line. I made a not trivial change to LineReader.sys.mjs so that just LF is treated as valid and that fixed the problem.

I then was wondering if the problem exists in older TB versions. So I have a computer still running TB 68 and it worked OK with the test account.

I then noticed that the reporter had mention that the account was previously running on 115. I wasn't sure if there was a problem with 115 so I tested it and it also worked OK with the test account.

I then moved the 115 pop3 parsing and protocol JS code (LineReader.sys.mjs and Pop3Client.sys.mjs) into daily and it also had the same problem.

Then I went back to unchanged daily and ran with 115 nsPop3Sink.cpp and it worked OK.  I then compared the nsPop3Sink.cpp code between daily and 115 and noticed one change pertaining to line endings that has been removed in daily (and is also not in 128). With this change https://phabricator.services.mozilla.com/D190226, the call to write CRLF (MSG_LINEBREAK) was removed in ```mailnews/local/src/nsPop3Sink.cpp``` at line 579 with no explanation that I could find. Putting this back in, as seen in the attached patch, fixes the bug without doing any other changes.

Maybe there's a good reason to remove the write of a final CRLF so I'll ask Ben. (My guess would be that since all POP3 lines are supposed to terminate with CRLF, there is no reason to write an extra one at the end of file.)

**Edit:** The write of MSG_LINEBREAK  that the proposed patch puts back in writes CRLF only for windows; for all others platforms it writes just LF. Also, there is an additional write of CRLF for all platforms ending each message record in the mbox file. See comment 63 for an example.