This patch set fixes bufferevent via http request cancellations (connect() and
dns-request), it survives tests and cancel.. with --no-fork, so this must be ok
(though I have one patch for dns layer pending).
But I'm not sure about cancel.. unit tests on win32, will fix disable them
later if they will differs (plus maybe we must make them skip-by-default?).
Fixes: #333
* bufev-cancellations-v5:
http: set fd to -1 unconditioally, to avoid leaking of DNS requests
test/http: cover NS timed out during request cancellations separatelly
http: avoid leaking of fd in evhttp_connection_free()
http: get fd from be layer during connection reset
be_sock: cancel in-progress dns requests
evdns: export cancel via callbacks in util (like async lib core/extra issues)
test/http: request cancellation with resolving/{conn,write}-timeouts in progress
Otherwise:
http/cancel_by_host_ns_timeout_inactive_server: [msg] Nameserver 127.0.0.1:37035 has failed: request timed out.
[msg] All nameservers have failed
OK
1 tests ok. (0 skipped)
==26211==
==26211== FILE DESCRIPTORS: 3 open at exit.
==26211== Open file descriptor 2: /dev/pts/47
==26211== <inherited from parent>
==26211==
==26211== Open file descriptor 1: /dev/pts/47
==26211== <inherited from parent>
==26211==
==26211== Open file descriptor 0: /dev/pts/47
==26211== <inherited from parent>
==26211==
==26211==
==26211== HEAP SUMMARY:
==26211== in use at exit: 1,112 bytes in 5 blocks
==26211== total heap usage: 149 allocs, 144 frees, 18,826 bytes allocated
==26211==
==26211== 40 bytes in 1 blocks are indirectly lost in loss record 1 of 5
==26211== at 0x4C2BBD5: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26211== by 0x4AAEB2: event_mm_calloc_ (event.c:3459)
==26211== by 0x498F5B: evbuffer_add_cb (buffer.c:3309)
==26211== by 0x4A0EF5: bufferevent_socket_new (bufferevent_sock.c:366)
==26211== by 0x4BFADF: evhttp_connection_base_bufferevent_new (http.c:2375)
==26211== by 0x4BFC8F: evhttp_connection_base_new (http.c:2427)
==26211== by 0x460DAA: http_cancel_test (regress_http.c:1417)
==26211== by 0x490A78: testcase_run_bare_ (tinytest.c:105)
==26211== by 0x490D5A: testcase_run_one (tinytest.c:252)
==26211== by 0x491699: tinytest_main (tinytest.c:434)
==26211== by 0x47E0E0: main (regress_main.c:461)
==26211==
==26211== 136 bytes in 1 blocks are indirectly lost in loss record 2 of 5
==26211== at 0x4C2BBD5: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26211== by 0x4AAEB2: event_mm_calloc_ (event.c:3459)
==26211== by 0x491FF0: evbuffer_new (buffer.c:365)
==26211== by 0x49A1BE: bufferevent_init_common_ (bufferevent.c:300)
==26211== by 0x4A0E44: bufferevent_socket_new (bufferevent_sock.c:353)
==26211== by 0x4BFADF: evhttp_connection_base_bufferevent_new (http.c:2375)
==26211== by 0x4BFC8F: evhttp_connection_base_new (http.c:2427)
==26211== by 0x460DAA: http_cancel_test (regress_http.c:1417)
==26211== by 0x490A78: testcase_run_bare_ (tinytest.c:105)
==26211== by 0x490D5A: testcase_run_one (tinytest.c:252)
==26211== by 0x491699: tinytest_main (tinytest.c:434)
==26211== by 0x47E0E0: main (regress_main.c:461)
==26211==
==26211== 136 bytes in 1 blocks are indirectly lost in loss record 3 of 5
==26211== at 0x4C2BBD5: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26211== by 0x4AAEB2: event_mm_calloc_ (event.c:3459)
==26211== by 0x491FF0: evbuffer_new (buffer.c:365)
==26211== by 0x49A1FB: bufferevent_init_common_ (bufferevent.c:305)
==26211== by 0x4A0E44: bufferevent_socket_new (bufferevent_sock.c:353)
==26211== by 0x4BFADF: evhttp_connection_base_bufferevent_new (http.c:2375)
==26211== by 0x4BFC8F: evhttp_connection_base_new (http.c:2427)
==26211== by 0x460DAA: http_cancel_test (regress_http.c:1417)
==26211== by 0x490A78: testcase_run_bare_ (tinytest.c:105)
==26211== by 0x490D5A: testcase_run_one (tinytest.c:252)
==26211== by 0x491699: tinytest_main (tinytest.c:434)
==26211== by 0x47E0E0: main (regress_main.c:461)
==26211==
==26211== 536 bytes in 1 blocks are indirectly lost in loss record 4 of 5
==26211== at 0x4C2BBD5: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26211== by 0x4AAEB2: event_mm_calloc_ (event.c:3459)
==26211== by 0x4A0E15: bufferevent_socket_new (bufferevent_sock.c:350)
==26211== by 0x4BFADF: evhttp_connection_base_bufferevent_new (http.c:2375)
==26211== by 0x4BFC8F: evhttp_connection_base_new (http.c:2427)
==26211== by 0x460DAA: http_cancel_test (regress_http.c:1417)
==26211== by 0x490A78: testcase_run_bare_ (tinytest.c:105)
==26211== by 0x490D5A: testcase_run_one (tinytest.c:252)
==26211== by 0x491699: tinytest_main (tinytest.c:434)
==26211== by 0x47E0E0: main (regress_main.c:461)
==26211==
==26211== 1,112 (264 direct, 848 indirect) bytes in 1 blocks are definitely lost in loss record 5 of 5
==26211== at 0x4C2BBD5: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26211== by 0x4AAEB2: event_mm_calloc_ (event.c:3459)
==26211== by 0x4D0564: evdns_getaddrinfo (evdns.c:4685)
==26211== by 0x4B13BA: evutil_getaddrinfo_async_ (evutil.c:1575)
==26211== by 0x4A139E: bufferevent_socket_connect_hostname (bufferevent_sock.c:524)
==26211== by 0x4C02DB: evhttp_connection_connect_ (http.c:2588)
==26211== by 0x4C04DD: evhttp_make_request (http.c:2643)
==26211== by 0x4615FF: http_cancel_test (regress_http.c:1504)
==26211== by 0x490A78: testcase_run_bare_ (tinytest.c:105)
==26211== by 0x490D5A: testcase_run_one (tinytest.c:252)
==26211== by 0x491699: tinytest_main (tinytest.c:434)
==26211== by 0x47E0E0: main (regress_main.c:461)
==26211==
==26211== LEAK SUMMARY:
==26211== definitely lost: 264 bytes in 1 blocks
==26211== indirectly lost: 848 bytes in 4 blocks
==26211== possibly lost: 0 bytes in 0 blocks
==26211== still reachable: 0 bytes in 0 blocks
==26211== suppressed: 0 bytes in 0 blocks
Since we do close fd there if we don't have BEV_OPT_CLOSE_ON_FREE, and
evcon->fd can be incorrect (non -1), so just get it from the underlying
bufferevent to fix this.
And after this patch the following tests report 0 instead of 2307 fd leaks:
$ valgrind --leak-check=full --show-reachable=yes --track-fds=yes --error-exitcode=1 regress --no-fork http/cancel..
==11299== FILE DESCRIPTORS: 3 open at exit.
And this is stdin/stderr/stdout.
Since it can be non -1, and we must close it, otherwise we will have problems.
And after this patch the following tests report fd 2307 instead of 2309 fd leaks:
$ valgrind --leak-check=full --show-reachable=yes --track-fds=yes --error-exitcode=1 regress --no-fork http/cancel..
==10853== FILE DESCRIPTORS: 2307 open at exit.
Before this patch we didn't have such functionality and this can cause some
issues when we are cancelling HTTP request while after this we will get a
response/timeout from the NS and in this case error_cb will be called and can
damage newly started HTTP request.
We could also have problems with connect, but we don't have them since we
closes the fd in HTTP layer.
This is not so good that this is done in be_sock, but it is internal, so we can
change without pain. Plus I don't like that callback-via-evutil, but since we
have event_extra we need do like that.
And after this patch the following tests doesn't report leaks:
$ valgrind --leak-check=full --show-reachable=yes --track-fds=yes --error-exitcode=1 regress --no-fork http/cancel..
...
==10469== FILE DESCRIPTORS: 2309 open at exit.
...
==10469== HEAP SUMMARY:
==10469== in use at exit: 0 bytes in 0 blocks
==10469== total heap usage: 33,846 allocs, 33,846 frees, 4,617,651 bytes allocated
==10469==
==10469== All heap blocks were freed -- no leaks are possible
v2: do under lock
v3: reset dns request
v4: ignore EVUTIL_EAI_CANCEL in regular callback
This patch adds 8 new tests:
- http/cancel
- http/cancel_by_host
- http/cancel_by_host_no_ns
- http/cancel_by_host_inactive_server
- http/cancel_inactive_server
- http/cancel_by_host_no_ns_inactive_server
- http/cancel_by_host_server_timeout
- http/cancel_server_timeout
- http/cancel_by_host_no_ns_server_timeout
This patches not 100% for http layer, but more for be_sock, but it was simpler
to add them here, plus it also shows some bugs with fd leaking in http layer.
Right now we have next picture (we can also play with timeouts/attempts for
evdns to make tests fail, IOW to track the failures even without valgrind):
$ valgrind --leak-check=full --show-reachable=yes --track-fds=yes --error-exitcode=1 regress --no-fork http/cancel..
http/cancel: OK
http/cancel_by_host: OK
http/cancel_by_host_no_ns: [msg] Nameserver 127.0.0.1:42489 has failed: request timed out.
[msg] All nameservers have failed
OK
http/cancel_by_host_inactive_server: OK
http/cancel_inactive_server: OK
http/cancel_by_host_no_ns_inactive_server: [msg] Nameserver 127.0.0.1:51370 has failed: request timed out.
[msg] All nameservers have failed
OK
http/cancel_by_host_server_timeout: OK
http/cancel_server_timeout: OK
http/cancel_by_host_no_ns_server_timeout: [msg] Nameserver 127.0.0.1:45054 has failed: request timed out.
[msg] All nameservers have failed
OK
9 tests ok. (0 skipped)
==3202==
==3202== FILE DESCRIPTORS: 2309 open at exit.
...
==8403== HEAP SUMMARY:
==8403== in use at exit: 1,104 bytes in 5 blocks
==8403== total heap usage: 10,916 allocs, 10,911 frees, 1,458,818 bytes allocated
==8403==
==8403== 40 bytes in 1 blocks are indirectly lost in loss record 1 of 5
==8403== at 0x4C2BBD5: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8403== by 0x4AAD2D: event_mm_calloc_ (event.c:3459)
==8403== by 0x498E48: evbuffer_add_cb (buffer.c:3309)
==8403== by 0x4A0DE2: bufferevent_socket_new (bufferevent_sock.c:366)
==8403== by 0x4BF8BA: evhttp_connection_base_bufferevent_new (http.c:2369)
==8403== by 0x4BFA6A: evhttp_connection_base_new (http.c:2421)
==8403== by 0x460CFC: http_cancel_test (regress_http.c:1413)
==8403== by 0x490965: testcase_run_bare_ (tinytest.c:105)
==8403== by 0x490C47: testcase_run_one (tinytest.c:252)
==8403== by 0x491586: tinytest_main (tinytest.c:434)
==8403== by 0x47DFCD: main (regress_main.c:461)
==8403==
==8403== 136 bytes in 1 blocks are indirectly lost in loss record 2 of 5
==8403== at 0x4C2BBD5: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8403== by 0x4AAD2D: event_mm_calloc_ (event.c:3459)
==8403== by 0x491EDD: evbuffer_new (buffer.c:365)
==8403== by 0x49A0AB: bufferevent_init_common_ (bufferevent.c:300)
==8403== by 0x4A0D31: bufferevent_socket_new (bufferevent_sock.c:353)
==8403== by 0x4BF8BA: evhttp_connection_base_bufferevent_new (http.c:2369)
==8403== by 0x4BFA6A: evhttp_connection_base_new (http.c:2421)
==8403== by 0x460CFC: http_cancel_test (regress_http.c:1413)
==8403== by 0x490965: testcase_run_bare_ (tinytest.c:105)
==8403== by 0x490C47: testcase_run_one (tinytest.c:252)
==8403== by 0x491586: tinytest_main (tinytest.c:434)
==8403== by 0x47DFCD: main (regress_main.c:461)
==8403==
==8403== 136 bytes in 1 blocks are indirectly lost in loss record 3 of 5
==8403== at 0x4C2BBD5: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8403== by 0x4AAD2D: event_mm_calloc_ (event.c:3459)
==8403== by 0x491EDD: evbuffer_new (buffer.c:365)
==8403== by 0x49A0E8: bufferevent_init_common_ (bufferevent.c:305)
==8403== by 0x4A0D31: bufferevent_socket_new (bufferevent_sock.c:353)
==8403== by 0x4BF8BA: evhttp_connection_base_bufferevent_new (http.c:2369)
==8403== by 0x4BFA6A: evhttp_connection_base_new (http.c:2421)
==8403== by 0x460CFC: http_cancel_test (regress_http.c:1413)
==8403== by 0x490965: testcase_run_bare_ (tinytest.c:105)
==8403== by 0x490C47: testcase_run_one (tinytest.c:252)
==8403== by 0x491586: tinytest_main (tinytest.c:434)
==8403== by 0x47DFCD: main (regress_main.c:461)
==8403==
==8403== 528 bytes in 1 blocks are indirectly lost in loss record 4 of 5
==8403== at 0x4C2BBD5: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8403== by 0x4AAD2D: event_mm_calloc_ (event.c:3459)
==8403== by 0x4A0D02: bufferevent_socket_new (bufferevent_sock.c:350)
==8403== by 0x4BF8BA: evhttp_connection_base_bufferevent_new (http.c:2369)
==8403== by 0x4BFA6A: evhttp_connection_base_new (http.c:2421)
==8403== by 0x460CFC: http_cancel_test (regress_http.c:1413)
==8403== by 0x490965: testcase_run_bare_ (tinytest.c:105)
==8403== by 0x490C47: testcase_run_one (tinytest.c:252)
==8403== by 0x491586: tinytest_main (tinytest.c:434)
==8403== by 0x47DFCD: main (regress_main.c:461)
==8403==
==8403== 1,104 (264 direct, 840 indirect) bytes in 1 blocks are definitely lost in loss record 5 of 5
==8403== at 0x4C2BBD5: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8403== by 0x4AAD2D: event_mm_calloc_ (event.c:3459)
==8403== by 0x4D0326: evdns_getaddrinfo (evdns.c:4682)
==8403== by 0x4B1213: evutil_getaddrinfo_async_ (evutil.c:1568)
==8403== by 0x4A1255: bufferevent_socket_connect_hostname (bufferevent_sock.c:517)
==8403== by 0x4C00B6: evhttp_connection_connect_ (http.c:2582)
==8403== by 0x4C02B8: evhttp_make_request (http.c:2637)
==8403== by 0x4614EC: http_cancel_test (regress_http.c:1496)
==8403== by 0x490965: testcase_run_bare_ (tinytest.c:105)
==8403== by 0x490C47: testcase_run_one (tinytest.c:252)
==8403== by 0x491586: tinytest_main (tinytest.c:434)
==8403== by 0x47DFCD: main (regress_main.c:461)
==8403==
==8403== LEAK SUMMARY:
==8403== definitely lost: 264 bytes in 1 blocks
==8403== indirectly lost: 840 bytes in 4 blocks
==8403== possibly lost: 0 bytes in 0 blocks
==8403== still reachable: 0 bytes in 0 blocks
==8403== suppressed: 0 bytes in 0 blocks
This will make cancellation tests more graceful, that said that error_cb can
not be called sometimes if you will break the loop in cancel.
Plus drop that define for function generations, since function body changed,
and it is not generic anymore, plus that macro didn't used by anyone else.
For example win32 doesn't accept such things (maybe via overloaded IO, I'm not
sure), also I looked into curl and seems that the behaviour is the same (IOW
like with EVHTTP_CON_READ_ON_WRITE_ERROR on linux/win32).
Fixes: https://ci.appveyor.com/project/nmathewson/libevent/build/2.1.5.216#L499 (win32)
Fixes: 680742e1665b85487f10c0ef3df021e3b8e98634 ("http: read server response
even after server closed the connection")
v2: v0 was just removing that flag, i.e. make it deprecated and set_flags() will return -1
This patch set fixes and covers http client and "Expect: 100-continue"
functionality (plus increase coverage under some related options, to avoid
further regressions).
* http-client-fix-expect-100-continue-v6:
http: fix leaking of response_code_line
http: fix "Expect: 100-continue" client side
test/http: separate coverage for EVHTTP_CON_READ_ON_WRITE_ERROR
test/http: cover "Expect: 100-continue" client-server interaction
test/http: *lingering tests shouldn't have "Expect: 100-continue"
Since now evhttp_parse_response_line() can be called twice because after
"HTTP/1.1 100 Continue" we can have "HTTP/1.1 200"
==29162== 9 bytes in 1 blocks are definitely lost in loss record 1 of 1
==29162== at 0x4C29C0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==29162== by 0x5CBF0A9: strdup (in /lib/x86_64-linux-gnu/libc-2.21.so)
==29162== by 0x4AA3AC: event_mm_strdup_ (event.c:3493)
==29162== by 0x4BD843: evhttp_parse_response_line (http.c:1680)
==29162== by 0x4BE333: evhttp_parse_firstline_ (http.c:2013)
==29162== by 0x4BEA4F: evhttp_read_firstline (http.c:2243)
==29162== by 0x4BC5F8: evhttp_read_cb (http.c:1136)
==29162== by 0x4993F1: bufferevent_run_readcb_ (bufferevent.c:233)
==29162== by 0x49FBC0: bufferevent_trigger_nolock_ (bufferevent-internal.h:392)
==29162== by 0x49FF10: bufferevent_readcb (bufferevent_sock.c:208)
==29162== by 0x4A474A: event_persist_closure (event.c:1580)
==29162== by 0x4A49F5: event_process_active_single_queue (event.c:1639)
Fixes: 0b46b39e95ad77951176f09782138305ba34edf3 ("http: fix "Expect:
100-continue" client side")
Instead of sending data always at the beginning of the request wait until the
server will respond with "HTTP/1.1 100 Continue".
Before this patch server do send "HTTP/1.1 100 Continue" but client always send
post data even without waiting server response.
P.S. this patch also touches some not 100% related tab-align issues.
Covered-by: http/data_length_constraints
Covered-by: http/read_on_write_error
Only bufferevent_sock have evbuffer_freeze()/evbuffer_unfreeze() & ctrl ops, so
we don't need to fix other bufferevents (be_pair doesn't have ctrl op).
Found during draining buffers in http layer, and hence 501-not-implemented
error in regress http/.. (with some custom hacking).
Since otherwise we can have nasty bugs with part of previous *request* in
current *request* and hence some parsing errors.
And now we have failures:
http/non_lingering_close: [forking] [err] ../http.c:1326: Assertion !evbuffer_drain(tmp, -1) failed in ../http.c
- add_compiler_flags() must accept array IOW just ARGN will be enoough
- add_compiler_flags() called with variable name instead of it's value
P.S. and fix some alignments issues
P.P.S. more cmake issues expected since now CFLAGS actually works
P.P.P.S. some issues with cmake cache is possible, so just reset it
According to https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html:
-Wswitch-enum
Warn whenever a switch statement has an index of enumerated type and lacks a
case for one or more of the named codes of that enumeration. case labels
outside the enumeration range also provoke warnings when this option is used.
The only difference between -Wswitch and this option is that this option
gives a warning about an omitted enumeration code even if there is a *default
label*.
In short: now `evhttp_set_max_body_size()` is browser-friendly.
This patch set implements lingering close (something like nginx have), this
will make web-server more graceful for currently existing web browsers, so:
- without EVHTTP_CON_LINGERING_CLOSE/EVHTTP_SERVER_LINGERING_CLOSE
before: it will read min(max_body_size, Content-Length)
- with EVHTTP_CON_LINGERING_CLOSE/EVHTTP_SERVER_LINGERING_CLOSE:
it will read max(max_body_size, Content-Length), and web browsers will show
"413 Request Entity Too Large".
Also it fixes a bug on client-side with non-lingering close web-servers
(EVHTTP_CON_READ_ON_WRITE_ERROR), found during implementing web-server
lingering close.
* more-graceful-http-v10:
test: http/lingering_close: cover EVHTTP_SERVER_LINGERING_CLOSE
test: http/non_lingering_close: cover ~EVHTTP_SERVER_LINGERING_CLOSE
test: http/*: update expected HTTP codes for body exceeds `max_body_size`
http: take EVHTTP_CON_LINGERING_CLOSE into account for "Expect: 100-Continue"
test: http/data_length_constrains: set EVHTTP_CON_READ_ON_WRITE_ERROR
http: lingering close (like nginx have) for entity-too-large
http: read server response even after server closed the connection
test: increase buffer size for http/data_length_constraints to trigger EPIPE
Fixes: #321
Covered-by: http/non_lingering_close
Covered-by: http/lingering_close
Also since after this patch code became more generic, we now respond with
HTTP_ENTITYTOOLARGE even without "Expect: 100-Continue", which is correct by
RFC.
Refs: #321
v2: remove EVHTTP_CON_ABOUT_TO_CLOSE
By lingering close I mean something what nginx have for this name, by this term
I mean that we need to read all the body even if it's size greater then
`max_body_size`, otherwise browsers on win32 (including chrome) failed read the
http status - entity-too-large (while on linux chrome for instance are good),
and also this includes badly written http clients.
Refs: #321
v2: do this only under EVHTTP_SERVER_LINGERING_CLOSE
Otherwise if we will try to write more data than server can accept
(see `evhttp_set_max_body_size()` for libevent server) we will get `EPIPE` and
will not try to read server's response which must contain 400 error for now
(which is not strictly correct though, it must 413).
```
$ strace regress --no-fork http/data_length_constraints
...
connect(10, {sa_family=AF_INET, sin_port=htons(43988), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
...
writev(10, [{"POST / HTTP/1.1\r\nHost: somehost\r"..., 60}, {"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 16324}], 2) = 16384
epoll_wait(5, [{EPOLLOUT, {u32=10, u64=10}}, {EPOLLIN, {u32=11, u64=11}}], 32, 50000) = 2
writev(10, [{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 16384}], 1) = 16384
ioctl(11, FIONREAD, [32768]) = 0
readv(11, [{"POST / HTTP/1.1\r\nHost: somehost\r"..., 4096}], 1) = 4096
epoll_ctl(5, EPOLL_CTL_DEL, 11, 0x7fff09d41e50) = 0
epoll_ctl(5, EPOLL_CTL_ADD, 11, {EPOLLOUT, {u32=11, u64=11}}) = 0
epoll_wait(5, [{EPOLLOUT, {u32=10, u64=10}}, {EPOLLOUT, {u32=11, u64=11}}], 32, 50000) = 2
writev(10, [{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 16384}], 1) = 16384
writev(11, [{"HTTP/1.1 400 Bad Request\r\nConten"..., 129}, {"<HTML><HEAD>\n<TITLE>400 Bad Requ"..., 94}], 2) = 223
epoll_ctl(5, EPOLL_CTL_DEL, 11, 0x7fff09d42080) = 0
shutdown(11, SHUT_WR) = 0
close(11) = 0
epoll_wait(5, [{EPOLLOUT|EPOLLERR|EPOLLHUP, {u32=10, u64=10}}], 32, 50000) = 1
writev(10, [{"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 16384}], 1) = -1 EPIPE (Broken pipe)
--- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=13954, si_uid=1000} ---
epoll_ctl(5, EPOLL_CTL_DEL, 10, 0x7fff09d42010) = 0
shutdown(10, SHUT_WR) = -1 ENOTCONN (Transport endpoint is not connected)
close(10) = 0
write(1, "\n FAIL ../test/regress_http.c:3"..., 37
```
Careful reader can ask why it send error even when it didn't read
`evcon->max_body_size`, and the answer will be checks for `evcon->max_body_size
against `Content-Length` header, which contains ~8MB (-2 bytes).
And also if we will not drain the output buffer than we will send buffer that
we didn't send in previous request and instead of sending method via
`evhttp_make_header()`.
Fixes: http/data_length_constraints
Refs: #321
v2: do this only under EVHTTP_CON_READ_ON_WRITE_ERROR flag
With greater buffer it can't be written with one writev(2), and hence we can
trigger more tricky cases, like calling writecb/readcb more then once.
Refs: #321
And we can't make them continuous, since the latest is a public API, and
otherwise we will break binary compatibility.
Also extra check for EVHTTP_CON_PUBLIC_FLAGS_END, in case somebody forgot about
this (implementer I mean).
Refs: #182
@asn-the-goblin-slayer:
"the name_parse() function in libevent's DNS code is vulnerable to a buffer overread.
971 if (cp != name_out) {
972 if (cp + 1 >= end) return -1;
973 *cp++ = '.';
974 }
975 if (cp + label_len >= end) return -1;
976 memcpy(cp, packet + j, label_len);
977 cp += label_len;
978 j += label_len;
No check is made against length before the memcpy occurs.
This was found through the Tor bug bounty program and the discovery should be credited to 'Guido Vranken'."
Reproducer for gdb (https://gist.github.com/azat/e4fcf540e9b89ab86d02):
set $PROT_NONE=0x0
set $PROT_READ=0x1
set $PROT_WRITE=0x2
set $MAP_ANONYMOUS=0x20
set $MAP_SHARED=0x01
set $MAP_FIXED=0x10
set $MAP_32BIT=0x40
start
set $length=202
# overread
set $length=2
# allocate with mmap to have a seg fault on page boundary
set $l=(1<<20)*2
p mmap(0, $l, $PROT_READ|$PROT_WRITE, $MAP_ANONYMOUS|$MAP_SHARED|$MAP_32BIT, -1, 0)
set $packet=(char *)$1+$l-$length
# hack the packet
set $packet[0]=63
set $packet[1]='/'
p malloc(sizeof(int))
set $idx=(int *)$2
set $idx[0]=0
set $name_out_len=202
p malloc($name_out_len)
set $name_out=$3
# have WRITE only mapping to fail on read
set $end=$1+$l
p (void *)mmap($end, 1<<12, $PROT_NONE, $MAP_ANONYMOUS|$MAP_SHARED|$MAP_FIXED|$MAP_32BIT, -1, 0)
set $m=$4
p name_parse($packet, $length, $idx, $name_out, $name_out_len)
x/2s (char *)$name_out
Before this patch:
$ gdb -ex 'source gdb' dns-example
$1 = 1073741824
$2 = (void *) 0x633010
$3 = (void *) 0x633030
$4 = (void *) 0x40200000
Program received signal SIGSEGV, Segmentation fault.
__memcpy_sse2_unaligned () at memcpy-sse2-unaligned.S:33
After this patch:
$ gdb -ex 'source gdb' dns-example
$1 = 1073741824
$2 = (void *) 0x633010
$3 = (void *) 0x633030
$4 = (void *) 0x40200000
$5 = -1
0x633030: "/"
0x633032: ""
(gdb) p $m
$6 = (void *) 0x40200000
(gdb) p $1
$7 = 1073741824
(gdb) p/x $1
$8 = 0x40000000
(gdb) quit
P.S. plus drop one condition duplicate.
Fixes: #317
@asn-the-goblin-slayer:
"Length between '[' and ']' is cast to signed 32 bit integer on line 1815. Is
the length is more than 2<<31 (INT_MAX), len will hold a negative value.
Consequently, it will pass the check at line 1816. Segfault happens at line
1819.
Generate a resolv.conf with generate-resolv.conf, then compile and run
poc.c. See entry-functions.txt for functions in tor that might be
vulnerable.
Please credit 'Guido Vranken' for this discovery through the Tor bug bounty
program."
Reproducer for gdb (https://gist.github.com/azat/be2b0d5e9417ba0dfe2c):
start
p (1ULL<<31)+1ULL
# $1 = 2147483649
p malloc(sizeof(struct sockaddr))
# $2 = (void *) 0x646010
p malloc(sizeof(int))
# $3 = (void *) 0x646030
p malloc($1)
# $4 = (void *) 0x7fff76a2a010
p memset($4, 1, $1)
# $5 = 1990369296
p (char *)$4
# $6 = 0x7fff76a2a010 '\001' <repeats 200 times>...
set $6[0]='['
set $6[$1]=']'
p evutil_parse_sockaddr_port($4, $2, $3)
# $7 = -1
Before:
$ gdb bin/http-connect < gdb
(gdb) $1 = 2147483649
(gdb) (gdb) $2 = (void *) 0x646010
(gdb) (gdb) $3 = (void *) 0x646030
(gdb) (gdb) $4 = (void *) 0x7fff76a2a010
(gdb) (gdb) $5 = 1990369296
(gdb) (gdb) $6 = 0x7fff76a2a010 '\001' <repeats 200 times>...
(gdb) (gdb) (gdb) (gdb)
Program received signal SIGSEGV, Segmentation fault.
__memcpy_sse2_unaligned () at memcpy-sse2-unaligned.S:36
After:
$ gdb bin/http-connect < gdb
(gdb) $1 = 2147483649
(gdb) (gdb) $2 = (void *) 0x646010
(gdb) (gdb) $3 = (void *) 0x646030
(gdb) (gdb) $4 = (void *) 0x7fff76a2a010
(gdb) (gdb) $5 = 1990369296
(gdb) (gdb) $6 = 0x7fff76a2a010 '\001' <repeats 200 times>...
(gdb) (gdb) (gdb) (gdb) $7 = -1
(gdb) (gdb) quit
Fixes: #318
Otherwise that #ifdef in visibility.h is useless, and __declspec(dllimport)
will be always on.
Fixes: #314
Fixes: 4545fa9b6866df47ce2f908631a84477a94d5f49 ("Add option to build shared
library")
