From: Stephan Bosch Date: Sat, 17 Feb 2018 00:26:02 +0000 (+0100) Subject: lib-http: server: Perform output stream error handling in one place. X-Git-Tag: 2.3.1~125 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=8acedb011efe7aa80b1d29a00e766cb0e2d34d30;p=thirdparty%2Fdovecot%2Fcore.git lib-http: server: Perform output stream error handling in one place. --- diff --git a/src/lib-http/http-server-connection.c b/src/lib-http/http-server-connection.c index 9f64a5035d..c84ca70989 100644 --- a/src/lib-http/http-server-connection.c +++ b/src/lib-http/http-server-connection.c @@ -818,16 +818,22 @@ int http_server_connection_discard_payload( return http_server_connection_unref_is_closed(conn) ? -1 : 0; } -void http_server_connection_write_failed(struct http_server_connection *conn, - const char *error) +void http_server_connection_handle_output_error( + struct http_server_connection *conn) { + struct ostream *output = conn->conn.output; + if (conn->closed) return; - if (error != NULL) { + if (output->stream_errno != EPIPE && + output->stream_errno != ECONNRESET) { http_server_connection_error(conn, - "Connection lost: %s", error); - http_server_connection_close(&conn, "Write failure"); + "Connection lost: write(%s) failed: %s", + o_stream_get_name(output), + o_stream_get_error(output)); + http_server_connection_close(&conn, + "Write failure"); } else { http_server_connection_debug(conn, "Connection lost: Remote disconnected"); @@ -836,27 +842,10 @@ void http_server_connection_write_failed(struct http_server_connection *conn, } } -void http_server_connection_handle_output_error( - struct http_server_connection *conn) -{ - struct ostream *output = conn->conn.output; - const char *error = NULL; - - if (output->stream_errno != EPIPE && - output->stream_errno != ECONNRESET) { - error = t_strdup_printf("write(%s) failed: %s", - o_stream_get_name(output), - o_stream_get_error(output)); - } - - http_server_connection_write_failed(conn, error); -} - static bool http_server_connection_next_response(struct http_server_connection *conn) { struct http_server_request *req; - const char *error = NULL; int ret; if (conn->output_locked) @@ -910,13 +899,11 @@ http_server_connection_next_response(struct http_server_connection *conn) http_server_connection_timeout_start(conn); http_server_request_ref(req); - ret = http_server_response_send(req->response, &error); + ret = http_server_response_send(req->response); http_server_request_unref(&req); - if (ret < 0) { - http_server_connection_write_failed(conn, error); + if (ret < 0) return FALSE; - } http_server_connection_timeout_reset(conn); return TRUE; @@ -974,20 +961,14 @@ int http_server_connection_output(struct http_server_connection *conn) } else if (conn->request_queue_head != NULL) { struct http_server_request *req = conn->request_queue_head; struct http_server_response *resp = req->response; - const char *error = NULL; http_server_connection_ref(conn); i_assert(resp != NULL); - ret = http_server_response_send_more(resp, &error); - - if (http_server_connection_unref_is_closed(conn)) - return -1; + ret = http_server_response_send_more(resp); - if (ret < 0) { - http_server_connection_write_failed(conn, error); + if (http_server_connection_unref_is_closed(conn) || ret < 0) return -1; - } if (!conn->output_locked) { /* room for more responses */ diff --git a/src/lib-http/http-server-private.h b/src/lib-http/http-server-private.h index b21fae423f..2949661466 100644 --- a/src/lib-http/http-server-private.h +++ b/src/lib-http/http-server-private.h @@ -174,10 +174,8 @@ struct http_server { */ void http_server_response_free(struct http_server_response *resp); -int http_server_response_send(struct http_server_response *resp, - const char **error_r); -int http_server_response_send_more(struct http_server_response *resp, - const char **error_r); +int http_server_response_send(struct http_server_response *resp); +int http_server_response_send_more(struct http_server_response *resp); /* * Request @@ -262,8 +260,6 @@ bool http_server_connection_shut_down(struct http_server_connection *conn); void http_server_connection_switch_ioloop(struct http_server_connection *conn); -void http_server_connection_write_failed(struct http_server_connection *conn, - const char *error); void http_server_connection_handle_output_error( struct http_server_connection *conn); diff --git a/src/lib-http/http-server-response.c b/src/lib-http/http-server-response.c index f340ab09bb..1e8cd22e2c 100644 --- a/src/lib-http/http-server-response.c +++ b/src/lib-http/http-server-response.c @@ -39,6 +39,23 @@ http_server_response_debug(struct http_server_response *resp, } } +static inline void +http_server_response_error(struct http_server_response *resp, + const char *format, ...) ATTR_FORMAT(2, 3); + +static inline void +http_server_response_error(struct http_server_response *resp, + const char *format, ...) +{ + va_list args; + + va_start(args, format); + i_error("http-server: request %s; %u response: %s", + http_server_request_label(resp->request), resp->status, + t_strdup_vprintf(format, args)); + va_end(args); +} + /* * Response */ @@ -235,13 +252,18 @@ http_server_response_finish_payload_out(struct http_server_response *resp) http_server_response_debug(resp, "Finished sending payload"); + http_server_connection_ref(conn); conn->output_locked = FALSE; - if (resp->payload_corked) - o_stream_uncork(conn->conn.output); - o_stream_set_flush_callback(conn->conn.output, - http_server_connection_output, conn); + if (conn->conn.output != NULL && !conn->conn.output->closed) { + if (resp->payload_corked && + o_stream_uncork_flush(conn->conn.output) < 0) + http_server_connection_handle_output_error(conn); + o_stream_set_flush_callback(conn->conn.output, + http_server_connection_output, conn); + } http_server_request_finished(resp->request); + http_server_connection_unref(&conn); } static int @@ -263,15 +285,7 @@ http_server_response_output_direct(struct http_server_response_payload *rpay) iov_count = rpay->iov_count - rpay->iov_idx; if ((ret=o_stream_sendv(output, iov, iov_count)) < 0) { - const char *error = NULL; - - if (output->stream_errno != EPIPE && - output->stream_errno != ECONNRESET) { - error = t_strdup_printf("write(%s) failed: %s", - o_stream_get_name(output), - o_stream_get_error(output)); - } - http_server_connection_write_failed(conn, error); + http_server_connection_handle_output_error(conn); return -1; } if (ret > 0) { @@ -470,16 +484,13 @@ http_server_response_payload_input(struct http_server_response *resp) (void)http_server_connection_output(conn); } -int http_server_response_send_more(struct http_server_response *resp, - const char **error_r) +int http_server_response_send_more(struct http_server_response *resp) { struct http_server_connection *conn = resp->request->conn; struct ostream *output = resp->payload_output; enum ostream_send_istream_result res; int ret = 0; - *error_r = NULL; - i_assert(!resp->payload_blocking); i_assert(resp->payload_input != NULL); i_assert(resp->payload_output != NULL); @@ -497,9 +508,11 @@ int http_server_response_send_more(struct http_server_response *resp, if (!resp->payload_chunked && resp->payload_input->v_offset - resp->payload_offset != resp->payload_size) { - *error_r = t_strdup_printf( - "Input stream %s size changed unexpectedly", + http_server_response_error(resp, + "Payload stream %s size changed unexpectedly", i_stream_get_name(resp->payload_input)); + http_server_connection_close(&conn, + "Payload read failure"); ret = -1; } else { ret = 1; @@ -520,18 +533,17 @@ int http_server_response_send_more(struct http_server_response *resp, case OSTREAM_SEND_ISTREAM_RESULT_ERROR_INPUT: /* we're in the middle of sending a response, so the connection will also have to be aborted */ - *error_r = t_strdup_printf("read(%s) failed: %s", + http_server_response_error(resp, + "read(%s) failed: %s", i_stream_get_name(resp->payload_input), i_stream_get_error(resp->payload_input)); + http_server_connection_close(&conn, + "Payload read failure"); ret = -1; break; case OSTREAM_SEND_ISTREAM_RESULT_ERROR_OUTPUT: /* failed to send response */ - if (output->stream_errno != EPIPE && - output->stream_errno != ECONNRESET) { - *error_r = t_strdup_printf("write(%s) failed: %s", - o_stream_get_name(output), o_stream_get_error(output)); - } + http_server_connection_handle_output_error(conn); ret = -1; break; } @@ -543,8 +555,7 @@ int http_server_response_send_more(struct http_server_response *resp, return ret < 0 ? -1 : 0; } -static int http_server_response_send_real(struct http_server_response *resp, - const char **error_r) +static int http_server_response_send_real(struct http_server_response *resp) { struct http_server_request *req = resp->request; struct http_server_connection *conn = req->conn; @@ -556,8 +567,6 @@ static int http_server_response_send_real(struct http_server_response *resp, bool close = FALSE; int ret = 0; - *error_r = NULL; - i_assert(!conn->output_locked); /* create status line */ @@ -658,11 +667,7 @@ static int http_server_response_send_real(struct http_server_response *resp, o_stream_ref(output); o_stream_cork(output); if (o_stream_sendv(output, iov, N_ELEMENTS(iov)) < 0) { - if (output->stream_errno != EPIPE && - output->stream_errno != ECONNRESET) { - *error_r = t_strdup_printf("write(%s) failed: %s", - o_stream_get_name(output), o_stream_get_error(output)); - } + http_server_connection_handle_output_error(conn); ret = -1; } @@ -676,7 +681,7 @@ static int http_server_response_send_real(struct http_server_response *resp, io_loop_stop(server->ioloop); } else if (resp->payload_output != NULL) { /* non-blocking payload */ - if (http_server_response_send_more(resp, error_r) < 0) + if (http_server_response_send_more(resp) < 0) ret = -1; } else { /* no payload to send */ @@ -686,31 +691,20 @@ static int http_server_response_send_real(struct http_server_response *resp, } if (ret >= 0 && !resp->payload_corked && o_stream_uncork_flush(output) < 0) { - if (output->stream_errno != EPIPE && - output->stream_errno != ECONNRESET) { - *error_r = t_strdup_printf("flush(%s) failed: %s", - o_stream_get_name(output), - o_stream_get_error(output)); - } + http_server_connection_handle_output_error(conn); ret = -1; } o_stream_unref(&output); return ret; } -int http_server_response_send(struct http_server_response *resp, - const char **error_r) +int http_server_response_send(struct http_server_response *resp) { - char *errstr = NULL; int ret; T_BEGIN { - ret = http_server_response_send_real(resp, error_r); - if (ret < 0) - errstr = i_strdup(*error_r); + ret = http_server_response_send_real(resp); } T_END; - *error_r = t_strdup(errstr); - i_free(errstr); return ret; }