apple / swift-nio-imap

A Swift project that provides an implementation of the IMAP4rev1 protocol, built upon SwiftNIO.
Apache License 2.0
98 stars 14 forks source link

IMAPClientHandler breaks when inbound buffer splits between CR and LF #772

Open Aloisius opened 1 month ago

Aloisius commented 1 month ago

Expected behavior

Reading well-formatted responses from a server should succeed regardless of how frames are split

Actual behavior

An exception is thrown IMAPDecoderError(parserError: NIOIMAPCore.ParserError(...) if a response is split between two buffers between a \r and \n.

This can happen when a server sends a very large response back which ends up being split between 8K buffers and the split just happens to land right after a carriage return. The next buffer will then start with a line-feed which the parser will blow up on.

Steps to reproduce

This is a bit hard to reproduce in the wild

  1. Issue a command to a server with a very large response, where the 8192th byte of a response buffer is a \r
  2. Parse the responses with IMAPClientHandler()

If possible, minimal yet complete reproducer code (or URL to code)

Add this test case to Tests/NIOIMAPTests/Coders/IMAPClientHandlerTests.swift:

class IMAPClientHandlerTests: XCTestCase {
    // ....

    func testSplitCRLF() {
        self.writeOutbound(.tagged(.init(tag: "a", command: .uidFetch(messages: .all, attributes: [.uid], modifiers: [])!)))
        self.assertOutboundString("a UID FETCH 1:* (UID)\r\n")

        self.writeInbound("* 38001 FETCH (UID 114597)\r")
        self.writeInbound("\n* 38045 FETCH (UID 114662)\r\n")
    }

This will give an error:

IMAPDecoderError(parserError: NIOIMAPCore.ParserError(hint: "none of the options match", file: "NIOIMAPCore/ParserLibrary.swift", line: 221), buffer: ByteBuffer { readerIndex: 0, writerIndex: 29, readableBytes: 29, capacity: 32, storageCapacity: 32, slice: _ByteBufferSlice { 0..<32 }, storage: 0x00006000032f1480 (32 bytes) } readable bytes (max 1k): [ 0a 2a 20 33 38 30 34 35 20 46 45 54 43 48 20 28 55 49 44 20 31 31 34 36 36 32 29 0d 0a ])"

SwiftNIO version/commit hash

7d0f2cf79902c73d7bb1564e625e0de39d669045

Swift & OS version (output of swift --version && uname -a)

swift-driver version: 1.115 Apple Swift version 6.0 (swiftlang-6.0.0.9.10 clang-1600.0.26.2) Target: arm64-apple-macosx15.0 Darwin redacted.local 24.0.0 Darwin Kernel Version 24.0.0: Mon Aug 12 20:49:48 PDT 2024; root:xnu-11215.1.10~2/RELEASE_ARM64_T8103 arm64

Aloisius commented 1 month ago

I think I see the problem here.

For the IMAPServerHandler, there is a FramingParser which has logic which will properly ignore a buffer that starts with a \n if the previous buffer ended in a \r. To that end, #670 was commited to make the newline parsing in ParserLibrary.parseNewline to be more lenient than the protocol allows by accepting a single \r as the termination of the response if it existed at the end of a buffer.

Unfortunately, IMAPClientHandler does not use the FramingParser, so the leniency ends up causing a parsing error when a buffer ends in a \r and the next starts with a \n.