This change should fix bug #1898: "assertion failed: tunnel.cc:372"

The assertion fails in SslStateData::Connection::dataSent method which is
called by SslStateData::writeClientDone and SslStateData::writeServerDone
methods.

Both write*Done methods did not check for the comm_err_t flag.

When the SSL side closes the connection after a successful but partial write,
write*Done is called with the len argument equal to the number of written
bytes, which is positive but less than the size of the given data for write,
and the dataSent() assertion "amount == (size_t)len" fails.

Based on Christos Tsantilas' work.

diff --git a/src/tunnel.cc b/src/tunnel.cc
index 1ea9246351ce26d42e9ab87dd956735c9435c142..5974ffdffeffa7e4ac28bb4322b92af6755cdfb5 100644 (file)
--- a/src/tunnel.cc
+++ b/src/tunnel.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: tunnel.cc,v 1.165 2006/09/19 07:56:57 adrian Exp $
+ * $Id: tunnel.cc,v 1.166 2007/04/12 14:21:50 rousskov Exp $
  *
  * DEBUG: section 26    Secure Sockets Layer Proxy
  * AUTHOR: Duane Wessels
@@ -324,20 +324,24 @@ void
 SslStateData::writeServerDone(char *buf, size_t len, comm_err_t flag, int xerrno)
 {
     debug(26, 3) ("sslWriteServer: FD %d, %d bytes written\n", server.fd(), (int)len);
-    /* Valid data */
 
-    if (len > 0) {
-        kb_incr(&statCounter.server.all.kbytes_out, len);
-        kb_incr(&statCounter.server.other.kbytes_out, len);
-        client.dataSent(len);
+    /* Error? */
+    if (len < 0 || flag != COMM_OK) {
+        server.error(xerrno); // may call comm_close
+        return;
     }
 
-    /* EOF */
+    /* EOF? */
     if (len == 0) {
         comm_close(server.fd());
         return;
     }
 
+    /* Valid data */
+    kb_incr(&statCounter.server.all.kbytes_out, len);
+    kb_incr(&statCounter.server.other.kbytes_out, len);
+    client.dataSent(len);
+
     /* If the other end has closed, so should we */
     if (client.fd() == -1) {
         comm_close(server.fd());
@@ -345,11 +349,8 @@ SslStateData::writeServerDone(char *buf, size_t len, comm_err_t flag, int xerrno
     }
 
     cbdataInternalLock(this);  /* ??? should be locked by the caller... */
-    /* Error? */
 
-    if (len < 0)
-        server.error(xerrno);
-    else if (cbdataReferenceValid(this))
+    if (cbdataReferenceValid(this))
         copyRead(client, ReadClient);
 
     cbdataInternalUnlock(this);        /* ??? */
@@ -382,17 +383,22 @@ SslStateData::writeClientDone(char *buf, size_t len, comm_err_t flag, int xerrno
 {
     debug(26, 3) ("sslWriteClient: FD %d, %d bytes written\n", client.fd(), (int)len);
 
-    if (len > 0) {
-        kb_incr(&statCounter.client_http.kbytes_out, len);
-        server.dataSent(len);
+    /* Error? */
+    if (len < 0 || flag != COMM_OK) {
+        client.error(xerrno); // may call comm_close
+        return;
     }
 
-    /* EOF */
+    /* EOF? */
     if (len == 0) {
         comm_close(client.fd());
         return;
     }
 
+    /* Valid data */
+    kb_incr(&statCounter.client_http.kbytes_out, len);
+    server.dataSent(len);
+
     /* If the other end has closed, so should we */
     if (server.fd() == -1) {
         comm_close(client.fd());
@@ -400,11 +406,8 @@ SslStateData::writeClientDone(char *buf, size_t len, comm_err_t flag, int xerrno
     }
 
     cbdataInternalLock(this);  /* ??? should be locked by the caller... */
-    /* Error? */
 
-    if (len < 0)
-        client.error(xerrno);
-    else if (cbdataReferenceValid(this))
+    if (cbdataReferenceValid(this))
         copyRead(server, ReadServer);
 
     cbdataInternalUnlock(this);        /* ??? */