From 64b6eceaba1a40ab0b175fa9fd9329d3e978ce6e Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Thu, 28 Feb 2013 17:19:44 +0400 Subject: [PATCH 01/14] uri decode: fix for warning "use of uninitialised value" This patch add check in evhttp_decode_uri_internal() that next 2 symbols are exists in array of chars for decoding, if don't have two next 2 symbols don't try to decode '%FF' --- http.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/http.c b/http.c index b94fed8c..ff331e6c 100644 --- a/http.c +++ b/http.c @@ -2889,8 +2889,8 @@ evhttp_decode_uri_internal( decode_plus = 1; } else if (c == '+' && decode_plus) { c = ' '; - } else if (c == '%' && EVUTIL_ISXDIGIT_(uri[i+1]) && - EVUTIL_ISXDIGIT_(uri[i+2])) { + } else if (length > 2 && i < (length - 2) && c == '%' && + EVUTIL_ISXDIGIT_(uri[i+1]) && EVUTIL_ISXDIGIT_(uri[i+2])) { char tmp[3]; tmp[0] = uri[i+1]; tmp[1] = uri[i+2]; From e1903e3ace4874dcb32458cd187f7361edf1962c Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Thu, 28 Feb 2013 23:10:02 +0400 Subject: [PATCH 02/14] uri decode: changed the test for the existence of the next character Fix for 64b6eceaba1a4 More info here https://github.com/azat/libevent/commit/64b6eceaba1a40ab0b175fa9fd9329d3e978ce6e#commitcomment-2714685 --- http.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http.c b/http.c index ff331e6c..15a035fa 100644 --- a/http.c +++ b/http.c @@ -2889,7 +2889,7 @@ evhttp_decode_uri_internal( decode_plus = 1; } else if (c == '+' && decode_plus) { c = ' '; - } else if (length > 2 && i < (length - 2) && c == '%' && + } else if ((i + 2) < length && c == '%' && EVUTIL_ISXDIGIT_(uri[i+1]) && EVUTIL_ISXDIGIT_(uri[i+2])) { char tmp[3]; tmp[0] = uri[i+1]; From de8101a884ae6d815d6006829fa4d3c3e8649a52 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Fri, 1 Mar 2013 12:00:24 +0400 Subject: [PATCH 03/14] Move prototype of evhttp_decode_uri_internal() to http-internal.h Make it non static, that can be called from tests --- http-internal.h | 3 +++ http.c | 4 +--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/http-internal.h b/http-internal.h index 83aa6ef1..0d4475f5 100644 --- a/http-internal.h +++ b/http-internal.h @@ -197,4 +197,7 @@ void evhttp_start_read_(struct evhttp_connection *); void evhttp_response_code_(struct evhttp_request *, int, const char *); void evhttp_send_page_(struct evhttp_request *, struct evbuffer *); +int evhttp_decode_uri_internal(const char *uri, size_t length, + char *ret, int decode_plus); + #endif /* _HTTP_H */ diff --git a/http.c b/http.c index 15a035fa..f571d356 100644 --- a/http.c +++ b/http.c @@ -193,8 +193,6 @@ static void evhttp_make_header(struct evhttp_connection *, struct evhttp_request static void evhttp_read_cb(struct bufferevent *, void *); static void evhttp_write_cb(struct bufferevent *, void *); static void evhttp_error_cb(struct bufferevent *bufev, short what, void *arg); -static int evhttp_decode_uri_internal(const char *uri, size_t length, - char *ret, int decode_plus); static int evhttp_find_vhost(struct evhttp *http, struct evhttp **outhttp, const char *hostname); @@ -2873,7 +2871,7 @@ evhttp_encode_uri(const char *str) * a ?. -1 is deprecated. * @return the number of bytes written to 'ret'. */ -static int +int evhttp_decode_uri_internal( const char *uri, size_t length, char *ret, int decode_plus_ctl) { From 13676535c83448b957ae00772c13f823cc9d3503 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Fri, 1 Mar 2013 12:01:42 +0400 Subject: [PATCH 04/14] Test: decoding just part of string with evhttp_decode_uri_internal() --- test/regress_http.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/regress_http.c b/test/regress_http.c index 3ed314a0..70b40d17 100644 --- a/test/regress_http.c +++ b/test/regress_http.c @@ -2396,6 +2396,7 @@ http_uriencode_test(void *ptr) { char *s=NULL, *s2=NULL; size_t sz; + int bytes_decoded; #define ENC(from,want,plus) do { \ s = evhttp_uriencode((from), -1, (plus)); \ @@ -2452,6 +2453,15 @@ http_uriencode_test(void *ptr) free(s); s = NULL; + /* Now try decoding just part of string. */ + s = malloc(6 + 1 /* NUL byte */); + bytes_decoded = evhttp_decode_uri_internal("hello%20%20", 6, s, 0); + tt_assert(s); + tt_int_op(bytes_decoded,==,6); + tt_str_op(s,==,"hello%"); + free(s); + s = NULL; + /* Now try out some decoding cases that we don't generate with * encode_uri: Make sure that malformed stuff doesn't crash... */ DEC("%%xhello th+ere \xff", From 9443868d55a829cd66a2dce24daad84c5f1ca2ed Mon Sep 17 00:00:00 2001 From: Nate Rosenblum Date: Tue, 5 Mar 2013 11:29:33 -0800 Subject: [PATCH 05/14] Double-check next timeout when adding events When resuming the system from a suspended state, the ev_timeout field of a scheduled timer event may be in the past. This leads to unexpected behavior when scheduling a short-duration timer event immediately after returning from suspension, because the new event does not land on top of the timeout minheap and so the event loop (blocked on a possibly long-duration timeout) is not notified. This patch checks for this condition and, if it obtains, notifies the event loop. --- event.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/event.c b/event.c index 51e34a99..9286c36f 100644 --- a/event.c +++ b/event.c @@ -2378,12 +2378,18 @@ event_add_nolock_(struct event *ev, const struct timeval *tv, common_timeout_schedule(ctl, &now, ev); } } else { + struct event* top = NULL; /* See if the earliest timeout is now earlier than it * was before: if so, we will need to tell the main - * thread to wake up earlier than it would - * otherwise. */ + * thread to wake up earlier than it would otherwise. + * We double check the timeout of the top element to + * handle time distortions due to system suspension. + */ if (min_heap_elt_is_top_(ev)) notify = 1; + else if ((top = min_heap_top_(&base->timeheap)) != NULL && + evutil_timercmp(&top->ev_timeout, &now, <)) + notify = 1; } } From 787fd7489f20c79b3d00fd2199a339cf98f68df2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 15 Mar 2013 09:33:13 -0400 Subject: [PATCH 06/14] Make --disable-libevent-regress work again --- test/include.am | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/include.am b/test/include.am index 82cfe4eb..56af58ec 100644 --- a/test/include.am +++ b/test/include.am @@ -29,12 +29,10 @@ TESTPROGRAMS = \ test/test-weof \ test/regress -noinst_PROGRAMS += $(TESTPROGRAMS) - if BUILD_REGRESS -noinst_PROGRAMS += test/regress -endif +noinst_PROGRAMS += $(TESTPROGRAMS) EXTRA_PROGRAMS+= test/regress +endif noinst_HEADERS+= \ test/regress.h \ From f935e2159a128f07a2172ae139236e7f0b415e40 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 15 Mar 2013 09:33:28 -0400 Subject: [PATCH 07/14] build test/test-script.sh on systems with a less-featureful $< --- test/include.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/include.am b/test/include.am index 56af58ec..324df347 100644 --- a/test/include.am +++ b/test/include.am @@ -45,7 +45,7 @@ noinst_HEADERS+= \ TESTS = test/test-script.sh test/test-script.sh: test/test.sh - cp $< $@ + cp $(top_srcdir)/test/test.sh $@ DISTCLEANFILES += test/test-script.sh From 773b0a5d886534b057f5589814a284dcd85c73d8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 25 Mar 2013 21:12:49 -0400 Subject: [PATCH 08/14] Fix a typo in a comment in buffer.h. Spotted by Alt_F4 --- include/event2/buffer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/event2/buffer.h b/include/event2/buffer.h index 37f424e1..5c896558 100644 --- a/include/event2/buffer.h +++ b/include/event2/buffer.h @@ -769,7 +769,7 @@ void evbuffer_cb_unsuspend(struct evbuffer *buffer, struct evbuffer_cb_entry *cb #endif /** - Makes the data at the begging of an evbuffer contiguous. + Makes the data at the beginning of an evbuffer contiguous. @param buf the evbuffer to make contiguous @param size the number of bytes to make contiguous, or -1 to make the From cf8d1cdb20555165b617cffb3cb89b89ff4260e7 Mon Sep 17 00:00:00 2001 From: Dan Petro Date: Fri, 29 Mar 2013 09:28:35 -0700 Subject: [PATCH 09/14] Specify return behavior in header for evbuffer_pullup() in corner case Function returns NULL when told to pullup more data than exists --- include/event2/buffer.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/event2/buffer.h b/include/event2/buffer.h index b36ee145..712e4d79 100644 --- a/include/event2/buffer.h +++ b/include/event2/buffer.h @@ -944,7 +944,8 @@ void evbuffer_cb_unsuspend(struct evbuffer *buffer, struct evbuffer_cb_entry *cb @param buf the evbuffer to make contiguous @param size the number of bytes to make contiguous, or -1 to make the entire buffer contiguous. - @return a pointer to the contiguous memory array + @return a pointer to the contiguous memory array, or NULL if param size + requested more data than is present in the buffer. */ unsigned char *evbuffer_pullup(struct evbuffer *buf, ev_ssize_t size); From 4914620025a7e457fc4c3a936441fce681bb6c38 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 31 Mar 2013 14:05:26 -0400 Subject: [PATCH 10/14] Do not build strlcpy.c when it will have no code. --- Makefile.am | 4 +++- configure.ac | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index baf15247..24853c9e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -156,6 +156,9 @@ SYS_INCLUDES = endif +if STRLCPY_IMPL +SYS_SRC += strlcpy.c +endif if SELECT_BACKEND SYS_SRC += select.c endif @@ -200,7 +203,6 @@ CORE_SRC = \ evutil_time.c \ listener.c \ log.c \ - strlcpy.c \ $(SYS_SRC) EXTRAS_SRC = \ diff --git a/configure.ac b/configure.ac index 9da77fc9..dd79e4da 100644 --- a/configure.ac +++ b/configure.ac @@ -362,6 +362,7 @@ AC_CHECK_FUNCS([ \ usleep \ vasprintf \ ]) +AM_CONDITIONAL(STRLCPY_IMPL, [test x"$ac_cv_func_strlcpy" = xno]) AC_CACHE_CHECK( [for getaddrinfo], From 2fad0f3d52c6c7511231b1b2eced484306835a52 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 5 Apr 2013 15:06:54 -0400 Subject: [PATCH 11/14] Add an environment variable (EVENT_DEBUG_MODE) to run unit tests in debug mode Not all tests currently pass with debug mode on. --- test/regress_main.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/regress_main.c b/test/regress_main.c index 11e5f8d9..a47a78ae 100644 --- a/test/regress_main.c +++ b/test/regress_main.c @@ -438,6 +438,9 @@ main(int argc, const char **argv) evthread_enable_lock_debugging(); #endif + if (getenv("EVENT_DEBUG_MODE")) + event_enable_debug_mode(); + tinytest_set_aliases(testaliases); if (tinytest_main(argc,argv,testgroups)) From 8a90a850fccdd93550b12a1d37e41b1b732fccbe Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 10 Apr 2013 13:53:44 -0400 Subject: [PATCH 12/14] Remove http_struct.h usage in sample/https-client.c --- sample/https-client.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sample/https-client.c b/sample/https-client.c index d6e5d880..82351020 100644 --- a/sample/https-client.c +++ b/sample/https-client.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include @@ -72,7 +71,8 @@ http_request_done(struct evhttp_request *req, void *ctx) } fprintf(stderr, "Response line: %d %s\n", - req->response_code, req->response_code_line); + evhttp_request_get_response_code(req), + evhttp_request_get_response_code_line(req)); while ((nread = evbuffer_remove(req->input_buffer, buffer, sizeof(buffer))) > 0) { @@ -182,6 +182,7 @@ main(int argc, char **argv) struct bufferevent *bev; struct evhttp_connection *evcon; struct evhttp_request *req; + struct evkeyvalq *output_headers; if (argc != 2) syntax(); @@ -314,8 +315,9 @@ main(int argc, char **argv) return 1; } - evhttp_add_header(req->output_headers, "Host", host); - evhttp_add_header(req->output_headers, "Connection", "close"); + output_headers = evhttp_request_get_output_headers(req); + evhttp_add_header(output_headers, "Host", host); + evhttp_add_header(output_headers, "Connection", "close"); r = evhttp_make_request(evcon, req, EVHTTP_REQ_GET, uri); if (r != 0) { From 95acdaa353419fc53e4324f34fa7e591f5bb5b38 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 10 Apr 2013 17:56:54 -0400 Subject: [PATCH 13/14] Another tweak to https-client.c --- sample/https-client.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sample/https-client.c b/sample/https-client.c index 82351020..b32e4304 100644 --- a/sample/https-client.c +++ b/sample/https-client.c @@ -74,7 +74,8 @@ http_request_done(struct evhttp_request *req, void *ctx) evhttp_request_get_response_code(req), evhttp_request_get_response_code_line(req)); - while ((nread = evbuffer_remove(req->input_buffer, buffer, sizeof(buffer))) + while ((nread = evbuffer_remove(evhttp_request_get_input_buffer(req), + buffer, sizeof(buffer))) > 0) { /* These are just arbitrary chunks of 256 bytes. * They are not lines, so we can't treat them as such. */ From 1c3147f5e7b398fc514d2cef8c9a40c036edf8fd Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 10 Apr 2013 18:03:16 -0400 Subject: [PATCH 14/14] Add a test with an active_later event at event_base_free time. --- test/regress.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/regress.c b/test/regress.c index 86b3d197..bfd2e78b 100644 --- a/test/regress.c +++ b/test/regress.c @@ -1406,6 +1406,13 @@ test_active_later(void *ptr) tt_int_op(n_write_a_byte_cb, >, 100); tt_int_op(n_read_and_drain_cb, >, 100); tt_int_op(n_activate_other_event_cb, >, 100); + + /* Now leave this one around, so that event_free sees it and removes + * it. */ + event_active_later_(&ev3, EV_READ); + event_base_assert_ok_(data->base); + event_base_free(data->base); + data->base = NULL; end: ; }