From 58fc5fb2eec65bc0b1c7f5e571e3c534cf008b88 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 7 Aug 2016 15:14:59 +0100 Subject: CHUNKING: fix transmit with long headers When the buffer used for SMTP commands and message headers filled to flush point, protocol sequencing was wrong. --- src/src/smtp_out.c | 6 ++++-- src/src/structs.h | 3 +-- src/src/transport.c | 23 ++++++++++++++++------- src/src/transports/smtp.c | 8 ++++++-- 4 files changed, 27 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/src/smtp_out.c b/src/src/smtp_out.c index 76181b5f1..5ab15cb5f 100644 --- a/src/src/smtp_out.c +++ b/src/src/smtp_out.c @@ -324,14 +324,16 @@ static BOOL flush_buffer(smtp_outblock *outblock) { int rc; +int n = outblock->ptr - outblock->buffer; +HDEBUG(D_transport|D_acl) debug_printf("cmd buf flush %d bytes\n", n); #ifdef SUPPORT_TLS if (tls_out.active == outblock->sock) - rc = tls_write(FALSE, outblock->buffer, outblock->ptr - outblock->buffer); + rc = tls_write(FALSE, outblock->buffer, n); else #endif + rc = send(outblock->sock, outblock->buffer, n, 0); -rc = send(outblock->sock, outblock->buffer, outblock->ptr - outblock->buffer, 0); if (rc <= 0) { HDEBUG(D_transport|D_acl) debug_printf("send failed: %s\n", strerror(errno)); diff --git a/src/src/structs.h b/src/src/structs.h index ffbc899a5..23c40ea3d 100644 --- a/src/src/structs.h +++ b/src/src/structs.h @@ -226,8 +226,7 @@ typedef struct transport_info { /* smtp transport datachunk callback */ #define tc_reap_prev BIT(0) /* Flags: reap previous SMTP cmd responses */ -#define tc_reap_one BIT(1) /* reap one SMTP response */ -#define tc_chunk_last BIT(2) /* annotate chunk SMTP cmd as LAST */ +#define tc_chunk_last BIT(1) /* annotate chunk SMTP cmd as LAST */ struct transport_context; typedef int (*tpt_chunk_cmd_cb)(int fd, struct transport_context * tctx, diff --git a/src/src/transport.c b/src/src/transport.c index a68c22f29..fdd97822a 100644 --- a/src/src/transport.c +++ b/src/src/transport.c @@ -418,15 +418,22 @@ for (ptr = start; ptr < end; ptr++) if ((len = chunk_ptr - deliver_out_buffer) > mlen) { + DEBUG(D_transport) debug_printf("flushing headers buffer\n"); + /* If CHUNKING, prefix with BDAT (size) NON-LAST. Also, reap responses from previous SMTP commands. */ if (tctx && tctx->options & topt_use_bdat && tctx->chunk_cb) - if (tctx->chunk_cb(fd, tctx, (unsigned)len, tc_reap_prev|tc_reap_one) != OK) + { + if ( tctx->chunk_cb(fd, tctx, (unsigned)len, 0) != OK + || !transport_write_block(fd, deliver_out_buffer, len) + || tctx->chunk_cb(fd, tctx, 0, tc_reap_prev) != OK + ) + return FALSE; + } + else + if (!transport_write_block(fd, deliver_out_buffer, len)) return FALSE; - - if (!transport_write_block(fd, deliver_out_buffer, len)) - return FALSE; chunk_ptr = deliver_out_buffer; } @@ -922,11 +929,11 @@ if (!(tctx->options & topt_no_headers)) return FALSE; } -/* When doing RFC3030 CHUNKING output, work out how much data will be in the -last BDAT, consisting of the current write_chunk() output buffer fill +/* When doing RFC3030 CHUNKING output, work out how much data would be in a +last-BDAT, consisting of the current write_chunk() output buffer fill (optimally, all of the headers - but it does not matter if we already had to flush that buffer with non-last BDAT prependix) plus the amount of body data -(as expanded for CRLF lines). Then create and write the BDAT, and ensure +(as expanded for CRLF lines). Then create and write BDAT(s), and ensure that further use of write_chunk() will not prepend BDATs. The first BDAT written will also first flush any outstanding MAIL and RCPT commands which were buffered thans to PIPELINING. @@ -960,6 +967,8 @@ if (tctx->options & topt_use_bdat) if (size > DELIVER_OUT_BUFFER_SIZE && hsize > 0) { + DEBUG(D_transport) + debug_printf("sending small initial BDAT; hssize=%d\n", hsize); if ( tctx->chunk_cb(fd, tctx, hsize, 0) != OK || !transport_write_block(fd, deliver_out_buffer, hsize) || tctx->chunk_cb(fd, tctx, 0, tc_reap_prev) != OK diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index 52b2b913f..416ea6297 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -1402,6 +1402,8 @@ if (tctx->pending_BDAT) if (flags & tc_reap_prev && prev_cmd_count > 0) { + DEBUG(D_transport) debug_printf("look for %d responses" + " for previous pipelined cmds\n", prev_cmd_count); switch(sync_responses(tctx->first_addr, tctx->tblock->rcpt_include_affixes, tctx->sync_addr, tctx->host, prev_cmd_count, @@ -1424,10 +1426,12 @@ if (flags & tc_reap_prev && prev_cmd_count > 0) pipelining_active = FALSE; } -/* Reap response for the cmd we just emitted, or an outstanding BDAT */ +/* Reap response for an outstanding BDAT */ -if (flags & tc_reap_one || tctx->pending_BDAT) +if (tctx->pending_BDAT) { + DEBUG(D_transport) debug_printf("look for one response for BDAT\n"); + if (!smtp_read_response(tctx->inblock, buffer, DELIVER_BUFFER_SIZE, '2', ob->command_timeout)) { -- cgit v1.2.3