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

diff --git a/bufferevent.c b/bufferevent.c
index 08c0486c087d5cf7b5859d34965cff855b2436d5..27f2a9ba0742434df0cd67cf08e15cefe344018e 100644 (file)
--- 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 987b7ecab9f7a0f440ed4f6abe84d55243dc1b0a..58baf831087e9e3b9d5aaf9ebf69f89382f38da4 100644 (file)
--- 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 c276a0e5d1c32ea5affea576fb3785e913d9ab67..ffecba567d8e4083c830c7f900ccc77e6116b24b 100644 (file)
--- 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,
 };