From 680742e1665b85487f10c0ef3df021e3b8e98634 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Wed, 10 Feb 2016 14:43:18 +0300 Subject: [PATCH] http: read server response even after server closed the connection 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}, {"\n400 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 --- http.c | 29 +++++++++++++++++++++++++++++ include/event2/http.h | 4 ++++ 2 files changed, 33 insertions(+) diff --git a/http.c b/http.c index fd7ce3cb..a2d9f878 100644 --- a/http.c +++ b/http.c @@ -1372,6 +1372,28 @@ evhttp_connection_cb_cleanup(struct evhttp_connection *evcon) } } +static void +evhttp_connection_read_on_write_error(struct evhttp_connection *evcon, + struct evhttp_request *req) +{ + struct evbuffer *buf; + + evcon->state = EVCON_READING_FIRSTLINE; + req->kind = EVHTTP_RESPONSE; + + buf = bufferevent_get_output(evcon->bufev); + evbuffer_unfreeze(buf, 1); + evbuffer_drain(buf, evbuffer_get_length(buf)); + evbuffer_freeze(buf, 1); + + bufferevent_setcb(evcon->bufev, + evhttp_read_cb, + NULL, /* evhttp_write_cb */ + evhttp_error_cb, + evcon); + evhttp_connection_start_detectclose(evcon); +} + static void evhttp_error_cb(struct bufferevent *bufev, short what, void *arg) { @@ -1441,6 +1463,12 @@ evhttp_error_cb(struct bufferevent *bufev, short what, void *arg) if (what & BEV_EVENT_TIMEOUT) { evhttp_connection_fail_(evcon, EVREQ_HTTP_TIMEOUT); } else if (what & (BEV_EVENT_EOF|BEV_EVENT_ERROR)) { + if (what & BEV_EVENT_WRITING && + evcon->flags & EVHTTP_CON_READ_ON_WRITE_ERROR) { + evhttp_connection_read_on_write_error(evcon, req); + return; + } + evhttp_connection_fail_(evcon, EVREQ_HTTP_EOF); } else if (what == BEV_EVENT_CONNECTED) { } else { @@ -2344,6 +2372,7 @@ int evhttp_connection_set_flags(struct evhttp_connection *evcon, { int avail_flags = 0; avail_flags |= EVHTTP_CON_REUSE_CONNECTED_ADDR; + avail_flags |= EVHTTP_CON_READ_ON_WRITE_ERROR; if (flags & ~avail_flags || flags > EVHTTP_CON_PUBLIC_FLAGS_END) return 1; diff --git a/include/event2/http.h b/include/event2/http.h index e9978207..c7ed4ccf 100644 --- a/include/event2/http.h +++ b/include/event2/http.h @@ -639,6 +639,10 @@ void evhttp_connection_set_family(struct evhttp_connection *evcon, /* reuse connection address on retry */ #define EVHTTP_CON_REUSE_CONNECTED_ADDR 0x0008 +/* Try to read error, since server may already send and close + * connection, but if at that time we have some data to send then we + * can send get EPIPE and fail, while we can read that HTTP error. */ +#define EVHTTP_CON_READ_ON_WRITE_ERROR 0x0010 /* Padding for public flags, @see EVHTTP_CON_* in http-internal.h */ #define EVHTTP_CON_PUBLIC_FLAGS_END 0x100000 /** -- 2.40.0