Re: Big files upload

Witold Filipczyk <[email protected]> writes:

> On Mon, May 12, 2008 at 12:14:52AM +0300, Kalle Olavi Niemitalo wrote:
>> - The code is duplicated between src/protocol/file/cgi.c and
>>   src/protocol/http/http.c.  This may be the best way but it
>>   looks a bit annoying.
>
> I have no idea how to make it better.

How about the following?  I did not test this yet.

1008: Merge POST data sending of HTTP and local CGI.

To reduce code duplication, src/protocol/file/cgi.c no longer parses
connection->uri->post on its own but rather calls the new function
http_read_post_data(), provided by src/protocol/http/http.c.  The same
code is now also used for POST requests that do not include files.

---
commit bd3745a8558bbebdbbe9d4195e095fd14adce24b
tree 05d3fab7c7e99630303bb8c6c73ec6519a7fa6e6
parent 181486696b58d0284fe64a8cadba0720ece87543
author Kalle Olavi Niemitalo <[email protected]> Sun, 18 May 2008 22:27:18 +0300
committer Kalle Olavi Niemitalo <[email protected]> Sun, 18 May 2008 22:34:16 +0300

 src/protocol/file/cgi.c  |  151 +++---------------------------
 src/protocol/http/http.c |  230 ++++++++++++++++++++++++++--------------------
 src/protocol/http/http.h |    3 -
 3 files changed, 146 insertions(+), 238 deletions(-)

diff --git a/src/protocol/file/cgi.c b/src/protocol/file/cgi.c
index ac16335..f7cb873 100644
--- a/src/protocol/file/cgi.c
+++ b/src/protocol/file/cgi.c
@@ -82,161 +82,42 @@ close_pipe_and_read(struct socket *data_socket)
 
 
 #define POST_BUFFER_SIZE 4096
-#define BIG_READ 65536
-static void send_files2(struct socket *socket);
 
 static void
-send_files(struct socket *socket)
+send_more_post_data(struct socket *socket)
 {
 	struct connection *conn = socket->conn;
 	struct http_connection_info *http = conn->info;
-	unsigned char *post = http->post_data;
 	unsigned char buffer[POST_BUFFER_SIZE];
-	unsigned char *file = strchr(post, FILE_CHAR);
-	struct string data;
-	int n = 0;
-	int finish = 0;
-
-	if (!init_string(&data)) {
-		abort_connection(conn, S_OUT_OF_MEM);
-		return;
-	}
-
-	if (!file) {
-		finish = 1;
-		file = strchr(post, '\0');
-	}
-
-	while (post < file) {
-		int h1, h2;
-
-		h1 = unhx(post[0]);
-		assertm(h1 >= 0 && h1 < 16, "h1 in the POST buffer is %d (%d/%c)", h1, post[0], post[0]);
-		if_assert_failed h1 = 0;
-
-		h2 = unhx(post[1]);
-		assertm(h2 >= 0 && h2 < 16, "h2 in the POST buffer is %d (%d/%c)", h2, post[1], post[1]);
-		if_assert_failed h2 = 0;
-
-		buffer[n++] = (h1<<4) + h2;
-		post += 2;
-		if (n == POST_BUFFER_SIZE) {
-			add_bytes_to_string(&data, buffer, n);
-			n = 0;
-		}
-	}
-	if (n) add_bytes_to_string(&data, buffer, n);
-
-	if (finish) {
-		write_to_socket(socket, data.source, data.length, S_SENT,
-			close_pipe_and_read);
-	} else {
-		unsigned char *end = strchr(file + 1, FILE_CHAR);
-
-		assert(end);
-		*end = '\0';
-		conn->post_fd = open(file + 1, O_RDONLY);
-		*end = FILE_CHAR;
-		if (conn->post_fd < 0) {
-			int errno_from_open = errno;
-
-			done_string(&data);
-			abort_connection(conn, -errno_from_open);
-			return;
-		}
-		http->post_data = end + 1;
-		socket->state = SOCKET_END_ONCLOSE;
-		write_to_socket(socket, data.source, data.length, S_TRANS,
-				send_files2);
-	}
-	done_string(&data);
-}
-
-static void
-send_files2(struct socket *socket)
-{
-	struct connection *conn = socket->conn;
-	unsigned char buffer[BIG_READ];
-	int n = safe_read(conn->post_fd, buffer, BIG_READ);
-
-	if (n > 0) {
-		socket->state = SOCKET_END_ONCLOSE;
-		write_to_socket(socket, buffer, n, S_TRANS,
-			send_files2);
-	} else {
-		close(conn->post_fd);
-		conn->post_fd = -1;
-		send_files(socket);
+	int got;
+
+	got = http_read_post_data(socket, 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;
+	} else {		/* got == 0, meaning end of data */
+		close_pipe_and_read(socket);
 	}
 }
 
+#undef POST_BUFFER_SIZE
 
 static void
 send_post_data(struct connection *conn)
 {
+	struct http_connection_info *http = conn->info;
 	unsigned char *post = conn->uri->post;
 	unsigned char *postend;
-	unsigned char buffer[POST_BUFFER_SIZE];
-	struct string data;
-	int n = 0;
 
 	postend = strchr(post, '\n');
 	if (postend) post = postend + 1;
 
-	if (post) {
-		unsigned char *file = strchr(post, FILE_CHAR);
-
-		if (file) {
-			struct http_connection_info *http = conn->info;
-
-			http->post_data = post;
-			send_files(conn->data_socket);
-			return;
-		}
-	}
-
-
-	if (!init_string(&data)) {
-		abort_connection(conn, S_OUT_OF_MEM);
-		return;
-	}
-
-	/* FIXME: Code duplication with protocol/http/http.c! --witekfl */
-	while (post[0] && post[1]) {
-		int h1, h2;
-
-		h1 = unhx(post[0]);
-		assert(h1 >= 0 && h1 < 16);
-		if_assert_failed h1 = 0;
-
-		h2 = unhx(post[1]);
-		assert(h2 >= 0 && h2 < 16);
-		if_assert_failed h2 = 0;
-
-		buffer[n++] = (h1<<4) + h2;
-		post += 2;
-		if (n == POST_BUFFER_SIZE) {
-			add_bytes_to_string(&data, buffer, n);
-			n = 0;
-		}
-	}
-	if (n)
-		add_bytes_to_string(&data, buffer, n);
-
-
-	/* If we're submitting a form whose controls do not have
-	 * names, then the POST has a Content-Type but empty data,
-	 * and an assertion would fail in write_to_socket.  */
-	if (data.length)
-		write_to_socket(conn->data_socket, data.source, data.length,
-				S_SENT, close_pipe_and_read);
-	else
-		close_pipe_and_read(conn->data_socket);
-
-	done_string(&data);
+	http->post_data = post;
+	send_more_post_data(conn->data_socket);
 }
-#undef POST_BUFFER_SIZE
-#undef BIG_READ
 
 static void
 send_request(struct connection *conn)
diff --git a/src/protocol/http/http.c b/src/protocol/http/http.c
index 3dcf83b..d68ad7d 100644
--- a/src/protocol/http/http.c
+++ b/src/protocol/http/http.c
@@ -613,94 +613,151 @@ post_length(unsigned char *post_data, unsigned int *count)
 #define POST_BUFFER_SIZE 4096
 #define BIG_READ 655360
 
-static void send_files2(struct socket *socket);
-
-static void
-send_files(struct socket *socket)
+static int
+http_read_post_data_inline(struct socket *socket,
+			   unsigned char buffer[], int max)
 {
-	struct connection *conn = socket->conn;
-	struct http_connection_info *http = conn->info;
+	struct connection *const conn = socket->conn;
+	struct http_connection_info *const http = conn->info;
 	unsigned char *post = http->post_data;
-	unsigned char buffer[POST_BUFFER_SIZE];
-	unsigned char *file = strchr(post, FILE_CHAR);
-	struct string data;
-	int n = 0;
-	int finish = 0;
+	unsigned char *end = strchr(post, FILE_CHAR);
+	int total = 0;
 
-	if (!init_string(&data)) {
-		http_end_request(conn, S_OUT_OF_MEM, 0);
-		return;
-	}
+	assert(conn->post_fd < 0);
+	if_assert_failed { errno = EINVAL; return -1; }
 
-	if (!file) {
-		finish = 1;
-		file = strchr(post, '\0');
-	}
+	if (!end)
+		end = strchr(post, '\0');
 
-	while (post < file) {
+	while (post < end && total < max) {
 		int h1, h2;
 
 		h1 = unhx(post[0]);
 		assertm(h1 >= 0 && h1 < 16, "h1 in the POST buffer is %d (%d/%c)", h1, post[0], post[0]);
 		if_assert_failed h1 = 0;
 
-		h2 = unhx(post[1]);
-		assertm(h2 >= 0 && h2 < 16, "h2 in the POST buffer is %d (%d/%c)", h2, post[1], post[1]);
+		h2 = unhx(post[0]);
+		assertm(h2 >= 0 && h2 < 16, "h2 in the POST buffer is %d (%d/%c)", h2, post[0], post[0]);
 		if_assert_failed h2 = 0;
 
-		buffer[n++] = (h1<<4) + h2;
+		buffer[total++] = (h1<<4) + h2;
 		post += 2;
-		if (n == POST_BUFFER_SIZE) {
-			add_bytes_to_string(&data, buffer, n);
-			n = 0;
-		}
 	}
-	if (n) add_bytes_to_string(&data, buffer, n);
+	if (post != end || *end != FILE_CHAR) {
+		http->post_data = post;
+		return total;
+	}
+
+	end = strchr(post + 1, FILE_CHAR);
+	assert(end);
+	*end = '\0';
+	conn->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 (total > 0)
+			return total; /* retry the open on the next call */
+		else
+			return -1; /* caller gets errno from open() */
+	}
+	http->post_data = end + 1;
+	return total;
+}
 
-	if (finish) {
-		http->uploaded += data.length;
-		request_from_socket(socket, data.source, data.length, S_SENT,
-		    SOCKET_END_ONCLOSE, http_got_header);
-	} else {
-		unsigned char *end = strchr(file + 1, FILE_CHAR);
+static int
+http_read_post_data_fd(struct socket *socket,
+		       unsigned char buffer[], int max)
+{
+	struct connection *const conn = socket->conn;
+	int ret;
 
-		assert(end);
-		*end = '\0';
-		conn->post_fd = open(file + 1, O_RDONLY);
-		*end = FILE_CHAR;
-		if (conn->post_fd < 0) {
-			int errno_from_open = errno;
+	/* safe_read() would set errno = EBADF anyway, but check this
+	 * explicitly to make any such bugs easier to detect.  */
+	assert(conn->post_fd >= 0);
+	if_assert_failed { errno = EBADF; return -1; }
 
-			done_string(&data);
-			http_end_request(conn, -errno_from_open, 0);
-			return;
+	ret = safe_read(conn->post_fd, buffer, max);
+	if (ret <= 0) {
+		const int errno_from_read = errno;
+
+		close(conn->post_fd);
+		conn->post_fd = -1;
+
+		errno = errno_from_read;
+	}
+	return ret;
+}
+
+/** Read data from socket->conn->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.  */
+int
+http_read_post_data(struct socket *socket,
+		    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;
+
+		if (post_fd < 0)
+			chunk = http_read_post_data_inline(socket,
+							   buffer + total,
+							   max - total);
+		else
+			chunk = http_read_post_data_fd(socket,
+						       buffer + total,
+						       max - total);
+		/* Be careful not to change errno here.  */
+
+		if (chunk == 0 && conn->post_fd == post_fd)
+			return total; /* EOF */
+		if (chunk < 0) {
+			/* If some data has already been successfully
+			 * read to buffer[], tell the caller about
+			 * that and forget about the error.  The next
+			 * http_read_post_data() call will retry the
+			 * operation that failed.  */
+			if (total != 0)
+				return total;
+			else
+				return chunk; /* caller gets errno from above */
 		}
-		http->post_data = end + 1;
-		socket->state = SOCKET_END_ONCLOSE;
-		http->uploaded += data.length;
-		write_to_socket(socket, data.source, data.length, S_TRANS,
-				send_files2);
+		total += chunk;
 	}
-	done_string(&data);
+	return total;
 }
 
 static void
-send_files2(struct socket *socket)
+send_more_post_data(struct socket *socket)
 {
 	struct connection *conn = socket->conn;
 	struct http_connection_info *http = conn->info;
-	unsigned char buffer[BIG_READ];
-	int n = safe_read(conn->post_fd, buffer, BIG_READ);
-
-	if (n > 0) {
+	unsigned char buffer[POST_BUFFER_SIZE];
+	int got;
+
+	got = http_read_post_data(socket, 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;
+	} else {		/* got == 0, meaning end of data */
+		/* Can't use request_from_socket() because there's no
+		 * more data to write.  */
+		struct read_buffer *rb = alloc_read_buffer(socket);
+		
 		socket->state = SOCKET_END_ONCLOSE;
-		http->uploaded += n;
-		write_to_socket(socket, buffer, n, S_TRANS,
-			send_files2);
-	} else {
-		close(conn->post_fd);
-		conn->post_fd = -1;
-		send_files(socket);
+		if (rb)
+			read_from_socket(socket, rb, S_SENT, http_got_header);
+		else
+			http_end_request(conn, S_OUT_OF_MEM, 0);
 	}
 }
 
@@ -1084,53 +1141,22 @@ http_send_header(struct socket *socket)
 
 	add_crlf_to_string(&header);
 
-	if (files) {
-		assert(!use_connect && post_data);
-		assert(conn->post_fd == -1);
-		http->post_data = post_data;
-		socket->state = SOCKET_END_ONCLOSE;
-		if (!conn->upload_progress)
-			conn->upload_progress = init_progress(0);
-		write_to_socket(socket, header.source, header.length, S_TRANS,
-			send_files);
-		done_string(&header);
-		return;
-	}
 	/* CONNECT: Any POST data is for the origin server only.
 	 * This was already checked above and post_data is NULL
 	 * in that case.  Verified with an assertion below.  */
 	if (post_data) {
-		unsigned char *post = post_data;
-		unsigned char buffer[POST_BUFFER_SIZE];
-		int n = 0;
-
 		assert(!use_connect); /* see comment above */
+		assert(conn->post_fd == -1);
 
-		while (post[0] && post[1]) {
-			int h1, h2;
-
-			h1 = unhx(post[0]);
-			assertm(h1 >= 0 && h1 < 16, "h1 in the POST buffer is %d (%d/%c)", h1, post[0], post[0]);
-			if_assert_failed h1 = 0;
-
-			h2 = unhx(post[1]);
-			assertm(h2 >= 0 && h2 < 16, "h2 in the POST buffer is %d (%d/%c)", h2, post[1], post[1]);
-			if_assert_failed h2 = 0;
-
-			buffer[n++] = (h1<<4) + h2;
-			post += 2;
-			if (n == POST_BUFFER_SIZE) {
-				add_bytes_to_string(&header, buffer, n);
-				n = 0;
-			}
-		}
-
-		if (n)
-			add_bytes_to_string(&header, buffer, n);
-	}
-
-	request_from_socket(socket, header.source, header.length, S_SENT,
-			    SOCKET_END_ONCLOSE, http_got_header);
+		http->post_data = post_data;
+		socket->state = SOCKET_END_ONCLOSE;
+		if (!conn->upload_progress && files)
+			conn->upload_progress = init_progress(0);
+		write_to_socket(socket, header.source, header.length, S_TRANS,
+				send_more_post_data);
+	} else
+		request_from_socket(socket, header.source, header.length, S_SENT,
+				    SOCKET_END_ONCLOSE, http_got_header);
 	done_string(&header);
 }
 
diff --git a/src/protocol/http/http.h b/src/protocol/http/http.h
index c2950d9..305acaf 100644
--- a/src/protocol/http/http.h
+++ b/src/protocol/http/http.h
@@ -40,7 +40,8 @@ struct http_connection_info {
 	unsigned char *post_data;
 };
 
-
+int http_read_post_data(struct socket *socket,
+			unsigned char buffer[], int max);
 
 extern struct module http_protocol_module;

_______________________________________________
elinks-dev mailing list
[email protected]
http://linuxfromscratch.org/mailman/listinfo/elinks-dev

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIMIXUHm9IGt60eMgRAn2vAJ9AWs+2YxOlKIz311qOYtEki4UuJwCgi4p3
UetTMK1glD8lJtXw+1nYH/U=
=4gdZ
-----END PGP SIGNATURE-----