apple / swift-nio-imap

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

IMAPClientHandler can't process very long line responses from the server #763

Open Aloisius opened 3 weeks ago

Aloisius commented 3 weeks ago

Expected behavior

Very long responses from an IMAP server in IMAPClientHandler should decode

Actual behavior

Very long responses that exceed IMAPDefaults.lineLengthLimit bytes throws an exception ByteToMessageDecoderError.PayloadTooLargeError.

The cause is IMAPClientHandler.init setting a maximum buffer size of IMAPDefaults.lineLengthLimit for decoding responses from the server and there is no way to override it without modifying the swift-nio-imap source.

Setting the limit to 8192 bytes seems to be arbitrary. IMAPDefaults.lineLengthLimit references RFC7162 section 4 for the limit, however according to the RFC, the limit is for commands sent from the client to the server, not from the server to the client:

The updated recommendation is as follows: a client should limit the length of the command lines it generates to approximately 8192 octets (including all quoted strings but not including literals). If the client is unable to group things into ranges so that the command line is within that length, it should split the request into multiple commands. The client should use literals instead of long quoted strings in order to keep the command length down.

Section 3.2.1.5 of RFC2683, referenced by RFC7162 section 4 also does not place limits on line length sent by servers to clients either.

While some limit may be useful, it would be helpful if there was a way to override it without modifying the source.

Steps to reproduce

  1. Create a IMAPClientHandler()
  2. Pass a response > 8192 bytes, like for a UID SEARCH RETURN () ALL on a very large mailbox in 10000 byte chunks to its channel

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

import Testing
import Foundation
import NIOIMAPCore
import NIO
import NIOIMAP

@Test func testIMAPHandler() throws {
    let clientHandler: IMAPClientHandler = IMAPClientHandler()
    let channel = EmbeddedChannel(handler: clientHandler)
    channel.pipeline.fireChannelActive()

    // this should be bigger than 8K
    var uids = MessageIdentifierSet<UnknownMessageIdentifier>()
    for i in 1...10000 {
        uids.insert(UnknownMessageIdentifier(rawValue: UInt32(i * 2)))
    }

    let command: CommandStreamPart = .tagged(.init(tag: "a3", command: .uidSearch(key: .all, charset: nil, returnOptions: [.all])))
    try channel.writeAndFlush(command).wait()

    let esearchResponse = Response.untagged(
        .mailboxData(
            .extendedSearch(.init(correlator: .init(tag: "a3"), kind: .uid, returnData: [
                .all(.set(.init(set: uids)!))
            ]))))

    // partial writes needed to trigger
    var buffer = ByteBuffer(string: esearchResponse.debugDescription)
    while buffer.readableBytes != 0 {
        let slice = min(buffer.readableBytes, 10000)
        try channel.writeInbound(buffer.readSlice(length: slice))
    }

    var maybeRead: Response? = try channel.readInbound()
    let _ = try #require(maybeRead)
}

SwiftNIO version/commit hash

2b6d7281916fe6da5229fed6a5aff3f986b37869

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 21:27:44 PDT 2024; root:xnu-11215.1.10~5/RELEASE_ARM64_T8103 arm64

Lukasa commented 3 weeks ago

Having a defensive limit is a good idea, as it's not sensible to let a remote peer force you to commit an unbounded amount of resources. However, we shouldn't prevent this being configured, as there's no one-right-value. We'd love a change that makes this configurable.