[MannXo]

Summary

When the SMTP endpoint delivers a message to a single target that can stream, and nothing in the pipeline reads the body, the body is now copied straight from the inbound connection into that target instead of being buffered first. The targets that can stream are target.smtp and target.lmtp, target.queue, target.remote when the message needs a single connection (one recipient domain), and target.ignore_error when the target it wraps can. Every other configuration buffers exactly as before.

This goes through a new optional interface, module.StreamingDelivery, so module.Delivery and the buffer.Buffer contract stay unchanged.

Changes

• framework/module: new optional StreamingDelivery interface with CanStreamBody() bool and BodyStream(ctx, header, io.Reader) error.
• target/smtp: implements it. If the inbound body fails midway, the downstream connection is dropped instead of closed with QUIT (see below).
• msgpipeline: implements it. CanStreamBody is true only with no checks (global, source or per-recipient), no modifiers, DMARC off, and exactly one target that can stream. Received and Authentication-Results are still added. The header part of Body moved into a helper that BodyStream shares.
• endpoint/smtp: Data reads the header, then streams if the delivery allows it. If the stream fails, the delivery is left in place so the following Reset aborts it. LMTPData is unchanged.
• testutils.Target gets a Streaming option, plus DoTestDeliveryStream.

Partial messages

The part that needs care is a client that fails halfway through DATA, since by then the downstream server has part of the message.

• go-smtp's DATA reader returns io.ErrUnexpectedEOF (and the BDAT pipe returns ErrDataReset) when the client goes away before the final dot, so a truncated body is never mistaken for a complete one.
• smtpconn.C.Close is not safe at that point. It sends QUIT through net/textproto, whose PrintfLine first closes any open DotWriter, so the downstream server would receive the terminating dot and accept the truncated message. BodyStream therefore calls DirectClose as soon as a read fails, and Abort/Commit become no-ops afterwards. TestDownstreamDelivery_BodyStream_ReadErr fails with the message delivered if that drop is removed.

Testing

Run in a golang:1.26 Linux container with libpam-dev, using the commands from test.yml:

• go test ./... -coverprofile=coverage.out -covermode=atomic passes.
• cd tests && ./run.sh has 1 failure, TestSMTPSASLExternalTLS, which fails the same way in CI on dev (tests/testdata/tls/server.crt is only produced by init.sh there and is not committed). TestLMTPClient_Is_Actually_LMTP and TestLMTPClient_Issue308 forward smtp to target.lmtp with no checks, so they now take the streaming path, and pass.
• ./build.sh and ./build.sh --destdir destdir/ install pass.
• golangci-lint v2.15.0 (CI pins v2.11, which I did not have) reports one issue, SA4023 in endpoint/dovecot_sasld, a file this PR does not touch.
• Each commit builds and passes the tests of the packages it touches.

New tests:

• TestDownstreamDelivery_BodyStream, TestDownstreamDelivery_BodyStream_ReadErr (no message reaches the downstream server, no leaked connection).
• TestMsgPipeline_BodyStream, TestMsgPipeline_CanStreamBody (one streaming target allows it, a non-streaming target, two targets, a global check, a per-recipient check, a source modifier or DMARC each disable it).
• TestSMTPDelivery_StreamBody, TestSMTPDelivery_StreamBody_AbortData (client disconnects after header and part of the body, nothing is committed).

..._ReadErr, ..._AbortData, TestSMTPDelivery_StreamBody and the two-target, check, modifier and DMARC cases of TestMsgPipeline_CanStreamBody were each checked to fail with the corresponding code removed.

Design notes

The issue leaves the design open, so here is what I picked and what I considered.

• Interface shape. An optional interface on the Delivery object, like PartialDelivery. CanStreamBody is separate from BodyStream because the answer depends on routing done in AddRcpt, and the endpoint has to know before it starts reading the body. Putting it on DeliveryTarget would not work for the same reason.
• Read-once Buffer instead of a new method. A buffer.Buffer whose Open works once would avoid the interface, but it breaks the documented contract (reopenable, Len() known up front), and every Body implementation and body check would need auditing for a second Open or a Len() call. Adding a method to Delivery itself would break every out-of-tree target.
• LMTP. Outbound LMTP (target.lmtp) is covered. Inbound LMTP (LMTPData) still buffers, because streaming there would collapse the per-recipient statuses from a downstream LMTP server into one.
• Eligibility is conservative. Any check or modifier disables streaming, including ones that only look at the envelope, such as replace_rcpt. Letting a check or modifier declare that it never reads the body would widen this later.

Two questions where your preference would change the code:

1. Is the two-method interface fine, or would you rather have a single BodyStream that reports "not possible" before reading anything?
2. Should inbound LMTP stream as well, here or in a follow-up?

Fixes #882

[foxcpp]

Need to into what other modules can benefit from this optimization.

• target.queue - probably will work in all cases, it reads body once to save into queue storage.
• target.remote - probably will work ONLY if only one connection is needed (that is, one recipient domain).
• target.ignore_error - should work if wrapped delivery supports it.
• storage.imapsql - probably can make it work if only one recipient is specified but that needs substantiation changes in go-imap-sql so let's leave that one out too for now.

Kinda unfortunate to see any checks and modifiers disabling optimization but let's leave that for a second step. That needs more design work.

[foxcpp]

checkRoutingLoops does not need buffered body and can run after readHeader, avoiding the need to duplicate its call in streamBody.

Same goes for cleanSession that can be separated into its own defer independent of buf.Remove.

Also accepted log.

[MannXo]

Pushed five commits on top of 14fbb8e:

• ed6fcc5 endpoint/smtp: checkRoutingLoops runs once right after readHeader, buf.Remove and cleanSession are separate defers, and Commit and the "accepted" log are shared, so streamBody is gone. cleanSession is deferred only once Body/BodyStream succeeded. On failure the delivery is kept and the Reset after DATA aborts it. This also changes the buffered path. A failed Body used to clean the session without ever calling Abort, now it is aborted.
• cdb1c6c target/queue: streams in all cases. storeNewMessage takes an io.Reader, and it already removes the partial files if reading fails.
• c5e5553 target: moves the reader that records source read errors out of target/smtp, so the next two can use it.
• 36ed55e target/ignore_error: streams if the wrapped delivery can, or if there is nothing wrapped because StartDelivery failed. Wrapped errors are still ignored, but a read error from the source is returned. After a wrapped error the rest of the body is drained, so a later read error (ErrDataTooLarge, client gone) is not swallowed.
• 33b67a9 target/remote: streams only with one connection, i.e. one recipient domain. On a read error the connection is dropped without QUIT, as in target/smtp, and is not returned to the pool. closeConn now skips a connection that was already dropped.

storage.imapsql, checks and modifiers are left as they were.

Each package has new tests, including a read error mid-body. In Docker (golang:1.26 with libpam-dev) go test ./... passes, and tests/run.sh fails only TestSMTPSASLExternalTLS, which fails the same way on dev itself (run 39380989123, 3aff5d1). golangci-lint reports nothing in the touched packages.