Check return value of evbuffer_remove() in bufferevent_read()

The conflict cast convertion between the return value of
bufferevent_read() and evbuffer_remove(), int(-1)->size_t(An undefined
maximum)

Add test case of bufferevent_read() should return 0 in case of
evbuffer_remove() returns -1

Fixes: #1132

3 files changed, 57 insertions, 2 deletions

diff --git a/bufferevent.c b/bufferevent.c
index 08c0486c..27f2a9ba 100644
--- a/bufferevent.c
+++ b/bufferevent.c
@@ -469,7 +469,12 @@ bufferevent_write_buffer(struct bufferevent *bufev, struct evbuffer *buf)
 size_t
 bufferevent_read(struct bufferevent *bufev, void *data, size_t size)
 {
-	return (evbuffer_remove(bufev->input, data, size));
+	int r = evbuffer_remove(bufev->input, data, size);
+
+	if (r == -1)
+		return 0;
+
+	return r;
 }
 
 int
diff --git a/include/event2/bufferevent.h b/include/event2/bufferevent.h
index 987b7eca..58baf831 100644
--- a/include/event2/bufferevent.h
+++ b/include/event2/bufferevent.h
@@ -430,7 +430,8 @@ int bufferevent_write_buffer(struct bufferevent *bufev, struct evbuffer *buf);
   @param bufev the bufferevent to be read from
   @param data pointer to a buffer that will store the data
   @param size the size of the data buffer, in bytes
-  @return the amount of data read, in bytes.
+  @return the amount of data read, in bytes. If 0 is returned, it is possible
+  that there is no data in the buffer or that the read failed.
  */
 EVENT2_EXPORT_SYMBOL
 size_t bufferevent_read(struct bufferevent *bufev, void *data, size_t size);
diff --git a/test/regress_bufferevent.c b/test/regress_bufferevent.c
index c276a0e5..ffecba56 100644
--- a/test/regress_bufferevent.c
+++ b/test/regress_bufferevent.c
@@ -1354,6 +1354,52 @@ end:
 		bufferevent_free(filter);
 }
 
+static void
+read_failed_readcb(struct bufferevent *bev, void *arg)
+{
+	int r;
+	struct evbuffer *in_buf = NULL;
+	char buf[] = "test data\0";
+	struct event_base *base = arg;
+
+	in_buf = bufferevent_get_input(bev);
+	tt_int_op(evbuffer_get_length(in_buf), ==, strlen(buf));
+
+	r = bufferevent_read(bev, buf, strlen(buf) - 1);
+	tt_int_op(r, ==, strlen(buf) - 1);
+	tt_int_op(evbuffer_get_length(in_buf), ==, 1);
+
+	evbuffer_freeze(in_buf, 1);
+	r = bufferevent_read(bev, buf, 1);
+	tt_int_op(r, ==, 0);
+end:;
+	event_base_loopexit(base, NULL);
+}
+
+static void
+test_bufferevent_read_failed(void *arg)
+{
+	struct basic_test_data *data = arg;
+	struct bufferevent *bev = NULL;
+	char buf[] = "test data\0";
+
+	bev = bufferevent_socket_new(
+		data->base, data->pair[1], BEV_OPT_CLOSE_ON_FREE);
+	bufferevent_setcb(bev, read_failed_readcb, NULL, NULL, data->base);
+	bufferevent_enable(bev, EV_READ);
+	tt_assert(bev != NULL);
+
+#ifdef _WIN32
+	send(data->pair[0], buf, strlen(buf), 0);
+#else
+	write(data->pair[0], buf, strlen(buf));
+#endif
+	event_base_dispatch(data->base);
+
+end:;
+	bufferevent_free(bev);
+}
+
 struct testcase_t bufferevent_testcases[] = {
 
 	LEGACY(bufferevent, TT_ISOLATED),
@@ -1429,6 +1475,9 @@ struct testcase_t bufferevent_testcases[] = {
 	{ "bufferevent_filter_data_stuck",
 	  test_bufferevent_filter_data_stuck,
 	  TT_FORK|TT_NEED_BASE, &basic_setup, NULL },
+	{ "bufferevent_read_failed",
+	  test_bufferevent_read_failed,
+	  TT_FORK|TT_NEED_SOCKETPAIR|TT_NEED_BASE, &basic_setup, NULL },
 
 	END_OF_TESTCASES,
 };