From 1b707208d331884efa876b94c0b5a7306e1d54af Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Mon, 26 May 2008 01:56:58 +0300 Subject: [PATCH] 1008: New struct http_post. Move connection.post_fd to http_post.post_fd. Make connection.done point to the new done_http_connection(), which calls the new done_http_post(), which closes post_fd. So done_connection() no longer needs to do that. Now that done_http_post() exists, a later commit can add dynamically allocated data in struct http_post and ensure that it will be freed. --- src/network/connection.c | 7 +-- src/network/connection.h | 1 - src/protocol/file/cgi.c | 6 +- src/protocol/http/http.c | 123 ++++++++++++++++++++++++++------------- src/protocol/http/http.h | 36 ++++++++---- 5 files changed, 114 insertions(+), 59 deletions(-) diff --git a/src/network/connection.c b/src/network/connection.c index 2ce72114..62c94121 100644 --- a/src/network/connection.c +++ b/src/network/connection.c @@ -303,7 +303,7 @@ init_connection(struct uri *uri, struct uri *proxied_uri, struct uri *referrer, conn->cache_mode = cache_mode; conn->content_encoding = ENCODING_NONE; - conn->stream_pipes[0] = conn->stream_pipes[1] = conn->post_fd = -1; + conn->stream_pipes[0] = conn->stream_pipes[1] = -1; init_list(conn->downloads); conn->est_length = -1; conn->timer = TIMER_ID_UNDEF; @@ -346,8 +346,8 @@ upload_stat_timer(struct connection *conn) assert(conn->upload_progress && http); if_assert_failed return; - update_progress(conn->upload_progress, http->uploaded, - http->total_upload_length, http->uploaded); + update_progress(conn->upload_progress, http->post.uploaded, + http->post.total_upload_length, http->post.uploaded); notify_connection_callbacks(conn); } @@ -503,7 +503,6 @@ done_connection(struct connection *conn) done_progress(conn->progress); if (conn->upload_progress) done_progress(conn->upload_progress); - if (conn->post_fd != -1) close(conn->post_fd); mem_free(conn); check_queue_bugs(); } diff --git a/src/network/connection.h b/src/network/connection.h index e796091e..0a98bc32 100644 --- a/src/network/connection.h +++ b/src/network/connection.h @@ -54,7 +54,6 @@ struct connection { int tries; timer_id_T timer; int stream_pipes[2]; - int post_fd; /* used when POSTing files */ unsigned int running:1; unsigned int unrestartable:1; diff --git a/src/protocol/file/cgi.c b/src/protocol/file/cgi.c index f7cb873f..bd114970 100644 --- a/src/protocol/file/cgi.c +++ b/src/protocol/file/cgi.c @@ -91,13 +91,13 @@ send_more_post_data(struct socket *socket) unsigned char buffer[POST_BUFFER_SIZE]; int got; - got = http_read_post_data(socket, buffer, POST_BUFFER_SIZE); + got = http_read_post_data(&http->post, buffer, POST_BUFFER_SIZE); if (got < 0) { abort_connection(conn, -errno); } else if (got > 0) { write_to_socket(socket, buffer, got, S_TRANS, send_more_post_data); - http->uploaded += got; + http->post.uploaded += got; } else { /* got == 0, meaning end of data */ close_pipe_and_read(socket); } @@ -115,7 +115,7 @@ send_post_data(struct connection *conn) postend = strchr(post, '\n'); if (postend) post = postend + 1; - http->post_data = post; + http->post.post_data = post; send_more_post_data(conn->data_socket); } diff --git a/src/protocol/http/http.c b/src/protocol/http/http.c index ef749eae..7584b64a 100644 --- a/src/protocol/http/http.c +++ b/src/protocol/http/http.c @@ -471,15 +471,17 @@ static void http_end_request(struct connection *conn, enum connection_state state, int notrunc) { - shutdown_connection_stream(conn); - if (conn->info) { - if (conn->post_fd != -1) { - close(conn->post_fd); - conn->post_fd = -1; - } - } + struct http_connection_info *http; - if (conn->info && !((struct http_connection_info *) conn->info)->close + shutdown_connection_stream(conn); + + /* shutdown_connection_stream() should not change conn->info, + * but in case it does, read conn->info only after the call. */ + http = conn->info; + if (http) + done_http_post(&http->post); + + if (http && !http->close && (!conn->socket->ssl) /* We won't keep alive ssl connections */ && (!get_opt_bool("protocol.http.bugs.post_no_keepalive", NULL) || !conn->uri->post)) { @@ -518,6 +520,19 @@ proxy_protocol_handler(struct connection *conn) #define connection_is_https_proxy(conn) \ (IS_PROXY_URI((conn)->uri) && (conn)->proxied_uri->protocol == PROTOCOL_HTTPS) +/** connection.done points to this function if connection.info points + * to a struct http_connection_info. */ +static void +done_http_connection(struct connection *conn) +{ + struct http_connection_info *http = conn->info; + + done_http_post(&http->post); + mem_free(http); + conn->info = NULL; + conn->done = NULL; +} + struct http_connection_info * init_http_connection_info(struct connection *conn, int major, int minor, int close) { @@ -533,7 +548,7 @@ init_http_connection_info(struct connection *conn, int major, int minor, int clo http->sent_version.minor = minor; http->close = close; - conn->post_fd = -1; + init_http_post(&http->post); /* The CGI code uses this too and blacklisting expects a host name. */ if (conn->proxied_uri->protocol != PROTOCOL_FILE) @@ -552,6 +567,7 @@ init_http_connection_info(struct connection *conn, int major, int minor, int clo conn->done = NULL; } mem_free_set(&conn->info, http); + conn->done = done_http_connection; return http; } @@ -583,6 +599,35 @@ accept_encoding_header(struct string *header) #endif } +/** Initialize *@a http_post so that done_http_post() can be safely + * called. + * + * @relates http_post */ +void +init_http_post(struct http_post *http_post) +{ + http_post->total_upload_length = 0; + http_post->uploaded = 0; + http_post->post_data = NULL; + http_post->post_fd = -1; +} + +/** Free all resources owned by *@a http_post, but do not free the + * structure itself. It is safe to call this multiple times. + * + * @relates http_post */ +void +done_http_post(struct http_post *http_post) +{ + http_post->total_upload_length = 0; + http_post->uploaded = 0; + http_post->post_data = NULL; + if (http_post->post_fd != -1) { + close(http_post->post_fd); + http_post->post_fd = -1; + } +} + /* This sets the Content-Length of POST data and counts files. */ static off_t post_length(unsigned char *post_data, unsigned int *count) @@ -617,17 +662,16 @@ post_length(unsigned char *post_data, unsigned int *count) #define POST_BUFFER_SIZE 4096 #define BIG_READ 655360 +/** @relates http_post */ static int -http_read_post_data_inline(struct socket *socket, +http_read_post_data_inline(struct http_post *http_post, unsigned char buffer[], int max) { - struct connection *const conn = socket->conn; - struct http_connection_info *const http = conn->info; - unsigned char *post = http->post_data; + unsigned char *post = http_post->post_data; unsigned char *end = strchr(post, FILE_CHAR); int total = 0; - assert(conn->post_fd < 0); + assert(http_post->post_fd < 0); if_assert_failed { errno = EINVAL; return -1; } if (!end) @@ -648,78 +692,79 @@ http_read_post_data_inline(struct socket *socket, post += 2; } if (post != end || *end != FILE_CHAR) { - http->post_data = post; + http_post->post_data = post; return total; } end = strchr(post + 1, FILE_CHAR); assert(end); *end = '\0'; - conn->post_fd = open(post + 1, O_RDONLY); + http_post->post_fd = open(post + 1, O_RDONLY); /* Be careful not to change errno here. */ *end = FILE_CHAR; - if (conn->post_fd < 0) { - http->post_data = post; + if (http_post->post_fd < 0) { + http_post->post_data = post; if (total > 0) return total; /* retry the open on the next call */ else return -1; /* caller gets errno from open() */ } - http->post_data = end + 1; + http_post->post_data = end + 1; return total; } +/** @relates http_post */ static int -http_read_post_data_fd(struct socket *socket, +http_read_post_data_fd(struct http_post *http_post, unsigned char buffer[], int max) { - struct connection *const conn = socket->conn; int ret; /* safe_read() would set errno = EBADF anyway, but check this * explicitly to make any such bugs easier to detect. */ - assert(conn->post_fd >= 0); + assert(http_post->post_fd >= 0); if_assert_failed { errno = EBADF; return -1; } - ret = safe_read(conn->post_fd, buffer, max); + ret = safe_read(http_post->post_fd, buffer, max); if (ret <= 0) { const int errno_from_read = errno; - close(conn->post_fd); - conn->post_fd = -1; + close(http_post->post_fd); + http_post->post_fd = -1; errno = errno_from_read; } return ret; } -/** Read data from socket->conn->uri->post or from the files to which - * it refers. +/** Read data from connection.uri->post or from the files to which it + * refers. * * @return >0 if read that many bytes; 0 if EOF; -1 on error and set - * errno. */ + * errno. + * + * @relates http_post */ int -http_read_post_data(struct socket *socket, +http_read_post_data(struct http_post *http_post, unsigned char buffer[], int max) { - struct connection *const conn = socket->conn; int total = 0; while (total < max) { int chunk; - int post_fd = conn->post_fd; + int post_fd = http_post->post_fd; if (post_fd < 0) - chunk = http_read_post_data_inline(socket, + chunk = http_read_post_data_inline(http_post, buffer + total, max - total); else - chunk = http_read_post_data_fd(socket, + chunk = http_read_post_data_fd(http_post, buffer + total, max - total); /* Be careful not to change errno here. */ - if (chunk == 0 && conn->post_fd == post_fd) + if (chunk == 0 && http_post->post_fd == post_fd) return total; /* EOF */ if (chunk < 0) { /* If some data has already been successfully @@ -745,13 +790,13 @@ send_more_post_data(struct socket *socket) unsigned char buffer[POST_BUFFER_SIZE]; int got; - got = http_read_post_data(socket, buffer, POST_BUFFER_SIZE); + got = http_read_post_data(&http->post, buffer, POST_BUFFER_SIZE); if (got < 0) { http_end_request(conn, -errno, 0); } else if (got > 0) { write_to_socket(socket, buffer, got, S_TRANS, send_more_post_data); - http->uploaded += got; + http->post.uploaded += got; } else { /* got == 0, meaning end of data */ /* Can't use request_from_socket() because there's no * more data to write. */ @@ -1123,7 +1168,7 @@ http_send_header(struct socket *socket) post_data = postend ? postend + 1 : uri->post; size = post_length(post_data, &files); - http->total_upload_length = size; + http->post.total_upload_length = size; add_format_to_string(&header, "Content-Length: " "%" OFF_PRINT_FORMAT "\x0D\x0A", (off_print_T) size); @@ -1150,9 +1195,9 @@ http_send_header(struct socket *socket) * in that case. Verified with an assertion below. */ if (post_data) { assert(!use_connect); /* see comment above */ - assert(conn->post_fd == -1); + assert(http->post.post_fd == -1); - http->post_data = post_data; + http->post.post_data = post_data; socket->state = SOCKET_END_ONCLOSE; if (!conn->upload_progress && files) conn->upload_progress = init_progress(0); diff --git a/src/protocol/http/http.h b/src/protocol/http/http.h index f1900c28..ba149142 100644 --- a/src/protocol/http/http.h +++ b/src/protocol/http/http.h @@ -16,17 +16,9 @@ struct http_version { int minor; }; -/** connection.info points to this in HTTP and local CGI connections. */ -struct http_connection_info { - enum blacklist_flags bl_flags; - struct http_version recv_version; - struct http_version sent_version; - - int close; - int length; - int chunk_remaining; - int code; - +/** State of reading POST data from connection.uri->post and related + * files. */ +struct http_post { /** Total size of the POST body to be uploaded */ off_t total_upload_length; @@ -38,11 +30,31 @@ struct http_connection_info { * substitutes a null character for the FILE_CHAR at the end of * each file name. */ unsigned char *post_data; + + /** File descriptor from which data is being read, or -1 if + * none. */ + int post_fd; }; -int http_read_post_data(struct socket *socket, +void init_http_post(struct http_post *http_post); +void done_http_post(struct http_post *http_post); +int http_read_post_data(struct http_post *http_post, unsigned char buffer[], int max); +/** connection.info points to this in HTTP and local CGI connections. */ +struct http_connection_info { + enum blacklist_flags bl_flags; + struct http_version recv_version; + struct http_version sent_version; + + int close; + int length; + int chunk_remaining; + int code; + + struct http_post post; +}; + extern struct module http_protocol_module; extern protocol_handler_T http_protocol_handler;