summaryrefslogtreecommitdiff
path: root/mailnews
diff options
context:
space:
mode:
authorMatt A. Tobin <email@mattatobin.com>2019-11-12 23:01:21 -0500
committerMatt A. Tobin <email@mattatobin.com>2019-11-12 23:01:21 -0500
commitf3611104b357c4c40fb51438ce82c972dd0f4247 (patch)
tree35688a0b51daf515bb14c1d07a161d76ae1a3970 /mailnews
parent9f00e5184aca0a5dbc3cb05142322bc3713e681a (diff)
downloaduxp-f3611104b357c4c40fb51438ce82c972dd0f4247.tar.gz
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
Diffstat (limited to 'mailnews')
-rw-r--r--mailnews/imap/src/nsImapServerResponseParser.cpp67
1 files changed, 45 insertions, 22 deletions
diff --git a/mailnews/imap/src/nsImapServerResponseParser.cpp b/mailnews/imap/src/nsImapServerResponseParser.cpp
index 68e929fe62..138ee596f9 100644
--- a/mailnews/imap/src/nsImapServerResponseParser.cpp
+++ b/mailnews/imap/src/nsImapServerResponseParser.cpp
@@ -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;