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 9f64a5035d82e03ad2ee26f6334f92b344f0ad80..c84ca70989a51ce0d7a6cb61785d33759e85dfa4 100644 (file)
--- 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 b21fae423fa7ef572f4511470e5a49a59d710cc9..29496614663d8049c6deacf76e9f3053f8a8143b 100644 (file)
--- 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 f340ab09bbc082fc4fddf1e25a379d49bac4087c..1e8cd22e2c3fa34a13bdc56dc66553ffbf9b83e6 100644 (file)
--- 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;
 }