From 4e62cd167b66f33e20344bff776253f19e2bc3e6 Mon Sep 17 00:00:00 2001
From: Mark Ellzey <socket@gmail.com>
Date: Fri, 30 Mar 2012 15:08:40 -0400
Subject: [PATCH] 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).
---
 bufferevent_openssl.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/bufferevent_openssl.c b/bufferevent_openssl.c
index 412c08e5..bdc363e5 100644
--- 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;