Bug 1494764 - Removed MOZ_ASSERT but now still process line where it would occur.

MOZ_ASSERT changed to NS_WARNING. Also correctly handle case where last chunk ends with \r. Tested to make sure that regression identified in Bug 1494764 comment 10 remains fixed and that non-chunked and chunked messages are handled correctly including when \r\n is split between chunks.

Tag #1273
This commit is contained in:
Matt A. Tobin 2019-11-12 23:01:21 -05:00 committed by Roy Tam
commit 2e230a6c19

View file

@ -3116,10 +3116,19 @@ void nsImapServerResponseParser::ResetCapabilityFlag()
}
/*
literal ::= "{" number "}" CRLF *CHAR8
;; Number represents the number of CHAR8 octets
*/
// returns true if this is the last chunk and we should close the stream
literal ::= "{" number "}" CRLF *CHAR8
Number represents the number of CHAR8 octets
*/
// Processes a message body, header or message part fetch response. Typically
// the full message, header or part are proccessed in one call (effectively, one
// chunk), and parameter `chunk` is false and `origin` (offset into the
// response) is 0. But under some conditions and larger messages, multiple calls
// will occur to process the message in multiple chunks and parameter `chunk`
// will be true and parameter `origin` will increase by the chunk size from
// initially 0 with each call. This function returns true if this is the last or
// only chunk. This signals the caller that the stream should be closed since
// the message response has been processed.
bool nsImapServerResponseParser::msg_fetch_literal(bool chunk, int32_t origin)
{
numberOfCharsInThisChunk = atoi(fNextToken + 1);
@ -3128,13 +3137,11 @@ bool nsImapServerResponseParser::msg_fetch_literal(bool chunk, int32_t origin)
bool lastChunk = (!chunk ||
(numberOfCharsInThisChunk != fServerConnection.GetCurFetchSize()));
#ifdef DEBUG
if (lastChunk)
MOZ_LOG(IMAP, mozilla::LogLevel::Debug,
("PARSER: fetch_literal chunk = %d, requested %d, receiving %d",
chunk, fServerConnection.GetCurFetchSize(),
numberOfCharsInThisChunk));
#endif
("PARSER: msg_fetch_literal() chunking=%s, requested=%d, receiving=%d",
chunk ? "true":"false", fServerConnection.GetCurFetchSize(),
numberOfCharsInThisChunk));
charsReadSoFar = 0;
@ -3244,20 +3251,37 @@ bool nsImapServerResponseParser::msg_fetch_literal(bool chunk, int32_t origin)
else
{
// Not the last line of a chunk.
if (!fNextChunkStartsWithNewline)
{
// Process unmodified fCurrentLine string.
bool processTheLine = true;
if (fNextChunkStartsWithNewline && origin > 0) {
// A split of the \r\n between chunks was detected. Ignore orphan \n
// on line by itself which can occur on the first line of a 2nd or
// later chunk. Line length should be 1 and the only character should
// be \n. Note: If previous message ended with just \r, don't expect
// the first chunk of a message (origin == 0) to begin with \n.
// (Typically, there is only one chunk required for a message or
// header response unless its size exceeds the chunking threshold.)
if (strlen(fCurrentLine) > 1 || fCurrentLine[0] != '\n') {
// In case expected orphan \n is not really there, go ahead and
// process the line. This should theoretically not occur but rarely,
// and for yet to be determined reasons, it does. Logging may help.
NS_WARNING(
"'\\n' is not the only character in this line as expected!");
MOZ_LOG(IMAP, mozilla::LogLevel::Debug,
("PARSER: expecting just '\\n' but line is = |%s|",
fCurrentLine));
} else {
// Discard the line containing only \n.
processTheLine = false;
MOZ_LOG(IMAP, mozilla::LogLevel::Debug,
("PARSER: discarding lone '\\n'"));
}
}
if (processTheLine) {
fServerConnection.HandleMessageDownLoadLine(fCurrentLine,
!lastChunk && (charsReadSoFar == numberOfCharsInThisChunk),
fCurrentLine);
}
else
{
// Ignore the orphan '\n' on a line by itself.
MOZ_ASSERT(strlen(fCurrentLine) == 1 && fCurrentLine[0] == '\n',
"Expect '\\n' as only character in this line");
fNextChunkStartsWithNewline = false;
}
fNextChunkStartsWithNewline = false;
}
}
}
@ -3276,9 +3300,8 @@ bool nsImapServerResponseParser::msg_fetch_literal(bool chunk, int32_t origin)
skip_to_CRLF();
AdvanceToNextToken();
}
}
else
{
} else {
// Don't typically (maybe never?) see this.
fNextChunkStartsWithNewline = false;
}
return lastChunk;