Fixed potential double-readcb execution with openssl bufferevents.

the function do_read() will call SSL_read(), and if successful, will
call _bufferevent_run_readcb() before returning to consider_reading().

consider_reading() will then check SSL_pending() to make sure all
pending data has also been read. If it does not, do_read() is called
again.

The issue with this is the possibility that the function that is
executed by _bufferevent_run_readcb() called
bufferevent_disable(ssl_bev, EV_READ) before the second do_read(); In
this case, the users read callback is executed a second time. This is
potentially bad for applications that expect the bufferevent to stay
disabled until further notice. (this is why running openssl bufferevents
without DEFER_CALLBACKS has always been troublesome).

diff --git a/bufferevent_openssl.c b/bufferevent_openssl.c
index 412c08e5c0b68007066c65b7416cadab5a5bbd1c..bdc363e5de43cd4ac2cc6e6bdeb8108cd8ce9374 100644 (file)
--- a/bufferevent_openssl.c
+++ b/bufferevent_openssl.c
@@ -615,9 +615,6 @@ do_read(struct bufferevent_openssl *bev_ssl, int n_to_read)
                evbuffer_commit_space(input, space, n_used);
                if (bev_ssl->underlying)
                        BEV_RESET_GENERIC_READ_TIMEOUT(bev);
-
-               if (evbuffer_get_length(input) >= bev->wm_read.low)
-                       _bufferevent_run_readcb(bev);
        }
 
        return blocked ? 0 : 1;
@@ -757,6 +754,7 @@ consider_reading(struct bufferevent_openssl *bev_ssl)
 {
        int r;
        int n_to_read;
+       int read_data = 0;
 
        while (bev_ssl->write_blocked_on_read) {
                r = do_write(bev_ssl, WRITE_FRAME);
@@ -772,6 +770,8 @@ consider_reading(struct bufferevent_openssl *bev_ssl)
                if (do_read(bev_ssl, n_to_read) <= 0)
                        break;
 
+               read_data = 1;
+
                /* Read all pending data.  This won't hit the network
                 * again, and will (most importantly) put us in a state
                 * where we don't need to read anything else until the
@@ -800,6 +800,15 @@ consider_reading(struct bufferevent_openssl *bev_ssl)
                        n_to_read = bytes_to_read(bev_ssl);
        }
 
+       if (read_data == 1) {
+               struct bufferevent *bev = &bev_ssl->bev.bev;
+               struct evbuffer *input = bev->input;
+
+               if (evbuffer_get_length(input) >= bev->wm_read.low) {
+                       _bufferevent_run_readcb(bev);
+               }
+       }
+
        if (!bev_ssl->underlying) {
                /* Should be redundant, but let's avoid busy-looping */
                if (bev_ssl->bev.read_suspended ||
@@ -821,6 +830,14 @@ consider_writing(struct bufferevent_openssl *bev_ssl)
                r = do_read(bev_ssl, 1024); /* XXXX 1024 is a hack */
                if (r <= 0)
                        break;
+               else {
+                       struct bufferevent *bev = &bev_ssl->bev.bev;
+                       struct evbuffer *input = bev->input;
+
+                       if (evbuffer_get_length(input) >= bev->wm_read.low) {
+                               _bufferevent_run_readcb(bev);
+                       }
+               }
        }
        if (bev_ssl->read_blocked_on_write)
                return;