Skip to content

Commit

Permalink
lib-http: server: Perform output stream error handling in one place.
Browse files Browse the repository at this point in the history
  • Loading branch information
stephanbosch authored and villesavolainen committed Mar 12, 2018
1 parent 5bf01fc commit 8acedb0
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 89 deletions.
49 changes: 15 additions & 34 deletions src/lib-http/http-server-connection.c
Expand Up @@ -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");
Expand All @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down
8 changes: 2 additions & 6 deletions src/lib-http/http-server-private.h
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down
92 changes: 43 additions & 49 deletions src/lib-http/http-server-response.c
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -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;
Expand All @@ -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 */
Expand Down Expand Up @@ -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;
}

Expand All @@ -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 */
Expand All @@ -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;
}

Expand Down

0 comments on commit 8acedb0

Please sign in to comment.