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 can't process very long line responses from the server #763

Open
Aloisius opened this issue Aug 23, 2024 · 1 comment
Open

Comments

@Aloisius
Copy link
Contributor

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

2b6d728

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
Copy link
Collaborator

Lukasa commented Aug 23, 2024

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.

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

2 participants