CHUNKING: fix transmit with long headers

When the buffer used for SMTP commands and message headers filled to flush
point, protocol sequencing was wrong.

4 files changed, 27 insertions, 13 deletions

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))
     {