https + large-file sending, sometimes fails

[Igor Sysoev]

On Thu, Dec 20, 2007 at 01:40:09PM +0300, Igor Sysoev wrote:

> On Thu, Dec 20, 2007 at 01:30:50PM +0300, Igor Sysoev wrote:
> 
> > On Thu, Dec 20, 2007 at 01:16:19PM +0300, Igor Sysoev wrote:
> > 
> > > On Thu, Dec 20, 2007 at 10:55:16AM +0100, G?bor Farkas wrote:
> > > 
> > > > also, something i say by the testing: if you start to do a lot of 
> > > > concurrent requests, and start to kill the clients (which are fetching 
> > > > the file), then also other requests start to die more frequently then 
> > > > normally.
> > > 
> > > Thank you, I have just reproduce the case.
> > 
> > The attached patch should fix the bug.
> 
> The complete patch.

The new commited patch.

1st, it fixes the cause of bug - SSL_shutdown() handling.
SSL_shutdown() man states incorrectly that SSL_shutdown() will return -1
on error. Actually SSL_shutdown() never returns -1.

2nd, it clears and logs possible previously unhandled SSL errors so
they have not affect on a new operation.


-- 
Igor Sysoev
http://sysoev.ru/en/
-------------- next part --------------
Index: src/event/ngx_event_openssl.c
===================================================================
--- src/event/ngx_event_openssl.c	(revision 1073)
+++ src/event/ngx_event_openssl.c	(revision 1075)
@@ -22,6 +22,7 @@
 static void ngx_ssl_shutdown_handler(ngx_event_t *ev);
 static void ngx_ssl_connection_error(ngx_connection_t *c, int sslerr,
     ngx_err_t err, char *text);
+static void ngx_ssl_clear_error(ngx_log_t *log);
 
 static ngx_int_t ngx_ssl_session_cache_init(ngx_shm_zone_t *shm_zone,
     void *data);
@@ -404,6 +405,8 @@
     int        n, sslerr;
     ngx_err_t  err;
 
+    ngx_ssl_clear_error(c->log);
+
     n = SSL_do_handshake(c->ssl->connection);
 
     ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0, "SSL_do_handshake: %d", n);
@@ -605,6 +608,8 @@
 
     bytes = 0;
 
+    ngx_ssl_clear_error(c->log);
+
     /*
      * SSL_read() may return data in parts, so try to read
      * until SSL_read() would return no data
@@ -895,6 +900,8 @@
     int        n, sslerr;
     ngx_err_t  err;
 
+    ngx_ssl_clear_error(c->log);
+
     ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0, "SSL to write: %d", size);
 
     n = SSL_write(c->ssl->connection, data, size);
@@ -978,9 +985,8 @@
 ngx_int_t
 ngx_ssl_shutdown(ngx_connection_t *c)
 {
-    int         n, sslerr, mode;
-    ngx_err_t   err;
-    ngx_uint_t  again;
+    int        n, sslerr, mode;
+    ngx_err_t  err;
 
     if (c->timedout) {
         mode = SSL_RECEIVED_SHUTDOWN|SSL_SENT_SHUTDOWN;
@@ -999,40 +1005,34 @@
 
     SSL_set_shutdown(c->ssl->connection, mode);
 
-    again = 0;
-    sslerr = 0;
+    ngx_ssl_clear_error(c->log);
 
-    for ( ;; ) {
-        n = SSL_shutdown(c->ssl->connection);
+    n = SSL_shutdown(c->ssl->connection);
 
-        ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0, "SSL_shutdown: %d", n);
+    ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0, "SSL_shutdown: %d", n);
 
-        if (n == 1 || (n == 0 && c->timedout)) {
-            SSL_free(c->ssl->connection);
-            c->ssl = NULL;
+    sslerr = 0;
 
-            return NGX_OK;
-        }
+    /* SSL_shutdown() never return -1, on error it return 0 */
 
-        if (n == 0) {
-            again = 1;
-            break;
-        }
-
-        break;
-    }
-
-    if (!again) {
+    if (n != 1) {
         sslerr = SSL_get_error(c->ssl->connection, n);
 
         ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0,
                        "SSL_get_error: %d", sslerr);
     }
 
-    if (again
-        || sslerr == SSL_ERROR_WANT_READ
-        || sslerr == SSL_ERROR_WANT_WRITE)
+    if (n == 1
+        || sslerr == SSL_ERROR_ZERO_RETURN
+        || (sslerr == 0 && c->timedout))
     {
+        SSL_free(c->ssl->connection);
+        c->ssl = NULL;
+
+        return NGX_OK;
+    }
+
+    if (sslerr == SSL_ERROR_WANT_READ || sslerr == SSL_ERROR_WANT_WRITE) {
         c->read->handler = ngx_ssl_shutdown_handler;
         c->write->handler = ngx_ssl_shutdown_handler;
 
@@ -1044,7 +1044,7 @@
             return NGX_ERROR;
         }
 
-        if (again || sslerr == SSL_ERROR_WANT_READ) {
+        if (sslerr == SSL_ERROR_WANT_READ) {
             ngx_add_timer(c->read, 30000);
         }
 
@@ -1125,6 +1125,15 @@
 }
 
 
+static void
+ngx_ssl_clear_error(ngx_log_t *log)
+{
+    if (ERR_peek_error()) {
+        ngx_ssl_error(NGX_LOG_ALERT, log, 0, "ignoring stale global SSL error");
+    }
+}
+
+
 void ngx_cdecl
 ngx_ssl_error(ngx_uint_t level, ngx_log_t *log, ngx_err_t err, char *fmt, ...)
 {