Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Aloisius opened this issue Sep 19, 2024 · 1 comment
Open

Comments

@Aloisius
Copy link
Contributor

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

7d0f2cf

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
Copy link
Contributor Author

Aloisius commented Sep 19, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant