[libsoup] soup-message-io: fix wrong-Content-Length logic

[Dan Winship <danw src gnome org>]

commit 74e253068cfa2a9d13c8d06f96ea42ad0069a657
Author: Dan Winship <danw gnome org>
Date:   Sat Jul 31 10:32:51 2010 +0200

    soup-message-io: fix wrong-Content-Length logic
    
    Previously we just ignored Content-Length if the server specified
    "Connection: close", which does the right thing if the specified
    Content-Length is too large, but fails if the Content-Length is
    correct but the server "forgets" to close the connection. Fix this so
    that we always stop reading once we get to the expected
    Content-Length, but we also cope with the connection closing before
    that point.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=611481

 libsoup/soup-message-client-io.c |   12 ----
 libsoup/soup-message-io.c        |   21 ++++++-
 tests/misc-test.c                |  109 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 127 insertions(+), 15 deletions(-)
---
diff --git a/libsoup/soup-message-client-io.c b/libsoup/soup-message-client-io.c
index ef97c99..745d921 100644
--- a/libsoup/soup-message-client-io.c
+++ b/libsoup/soup-message-client-io.c
@@ -59,18 +59,6 @@ parse_response_headers (SoupMessage *req,
 	if (*encoding == SOUP_ENCODING_UNRECOGNIZED)
 		return SOUP_STATUS_MALFORMED;
 
-        /* mirror Mozilla here:
-         * (see netwerk/protocol/http/src/nsHttpTransaction.cpp for details)
-         *
-         * HTTP servers have been known to send erroneous Content-Length headers.
-         * So, unless the connection is persistent, we must make allowances for a
-         * possibly invalid Content-Length header. Thus, if NOT persistent, we 
-         * simply accept the whole message.
-         */
-        if (*encoding == SOUP_ENCODING_CONTENT_LENGTH &&
-            !soup_message_is_keepalive (req))
-                *encoding = SOUP_ENCODING_EOF;
-
 	return SOUP_STATUS_OK;
 }
 
diff --git a/libsoup/soup-message-io.c b/libsoup/soup-message-io.c
index ff1fec2..f4f4c96 100644
--- a/libsoup/soup-message-io.c
+++ b/libsoup/soup-message-io.c
@@ -54,6 +54,7 @@ typedef struct {
 	GByteArray           *read_meta_buf;
 	SoupMessageBody      *read_body;
 	goffset               read_length;
+	gboolean              read_eof_ok;
 
 	gboolean              need_content_sniffed, need_got_chunk;
 	SoupMessageBody      *sniff_data;
@@ -212,8 +213,7 @@ io_disconnected (SoupSocket *sock, SoupMessage *msg)
 	SoupMessageIOData *io = priv->io_data;
 
 	/* Closing the connection to signify EOF is sometimes ok */
-	if (io->read_state == SOUP_MESSAGE_IO_STATE_BODY &&
-	    io->read_encoding == SOUP_ENCODING_EOF) {
+	if (io->read_state == SOUP_MESSAGE_IO_STATE_BODY && io->read_eof_ok) {
 		io->read_state = SOUP_MESSAGE_IO_STATE_FINISHING;
 		io_read (sock, msg);
 		return;
@@ -462,8 +462,10 @@ read_body_chunk (SoupMessage *msg)
 			break;
 
 		case SOUP_SOCKET_EOF:
-			if (read_to_eof)
+			if (io->read_eof_ok) {
+				io->read_length = 0;
 				return TRUE;
+			}
 			/* else fall through */
 
 		case SOUP_SOCKET_ERROR:
@@ -842,11 +844,24 @@ io_read (SoupSocket *sock, SoupMessage *msg)
 			break;
 		}
 
+		if (io->read_encoding == SOUP_ENCODING_EOF)
+			io->read_eof_ok = TRUE;
+
 		if (io->read_encoding == SOUP_ENCODING_CONTENT_LENGTH) {
 			SoupMessageHeaders *hdrs =
 				(io->mode == SOUP_MESSAGE_IO_CLIENT) ?
 				msg->response_headers : msg->request_headers;
 			io->read_length = soup_message_headers_get_content_length (hdrs);
+
+			if (io->mode == SOUP_MESSAGE_IO_CLIENT &&
+			    !soup_message_is_keepalive (msg)) {
+				/* Some servers suck and send
+				 * incorrect Content-Length values, so
+				 * allow EOF termination in this case
+				 * (iff the message is too short) too.
+				 */
+				io->read_eof_ok = TRUE;
+			}
 		}
 
 		if (io->mode == SOUP_MESSAGE_IO_CLIENT &&
diff --git a/tests/misc-test.c b/tests/misc-test.c
index dabbb06..8b462c5 100644
--- a/tests/misc-test.c
+++ b/tests/misc-test.c
@@ -29,6 +29,20 @@ auth_callback (SoupAuthDomain *auth_domain, SoupMessage *msg,
 }
 
 static void
+forget_close (SoupMessage *msg, gpointer user_data)
+{
+	soup_message_headers_remove (msg->response_headers, "Connection");
+}
+
+static void
+close_socket (SoupMessage *msg, gpointer user_data)
+{
+	SoupSocket *sock = user_data;
+
+	soup_socket_disconnect (sock);
+}
+
+static void
 server_callback (SoupServer *server, SoupMessage *msg,
 		 const char *path, GHashTable *query,
 		 SoupClientContext *context, gpointer data)
@@ -61,6 +75,41 @@ server_callback (SoupServer *server, SoupMessage *msg,
 		return;
 	}
 
+	if (g_str_has_prefix (path, "/content-length/")) {
+		gboolean too_long = strcmp (path, "/content-length/long") == 0;
+		gboolean no_close = strcmp (path, "/content-length/noclose") == 0;
+
+		soup_message_set_status (msg, SOUP_STATUS_OK);
+		soup_message_set_response (msg, "text/plain",
+					   SOUP_MEMORY_STATIC, "foobar", 6);
+		if (too_long)
+			soup_message_headers_set_content_length (msg->response_headers, 9);
+		soup_message_headers_append (msg->response_headers,
+					     "Connection", "close");
+
+		if (too_long) {
+			SoupSocket *sock;
+
+			/* soup-message-io will wait for us to add
+			 * another chunk after the first, to fill out
+			 * the declared Content-Length. Instead, we
+			 * forcibly close the socket at that point.
+			 */
+			sock = soup_client_context_get_socket (context);
+			g_signal_connect (msg, "wrote-chunk",
+					  G_CALLBACK (close_socket), sock);
+		} else if (no_close) {
+			/* Remove the 'Connection: close' after writing
+			 * the headers, so that when we check it after
+			 * writing the body, we'll think we aren't
+			 * supposed to close it.
+			 */
+			g_signal_connect (msg, "wrote-headers",
+					  G_CALLBACK (forget_close), NULL);
+		}
+		return;
+	}
+
 	soup_message_set_status (msg, SOUP_STATUS_OK);
 	if (!strcmp (uri->host, "foo")) {
 		soup_message_set_response (msg, "text/plain",
@@ -473,6 +522,65 @@ do_early_abort_test (void)
 	soup_test_session_abort_unref (session);
 }
 
+static void
+do_content_length_framing_test (void)
+{
+	SoupSession *session;
+	SoupMessage *msg;
+	SoupURI *request_uri;
+	goffset declared_length;
+
+	debug_printf (1, "\nInvalid Content-Length framing tests\n");
+
+	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
+
+	debug_printf (1, "  Content-Length larger than message body length\n");
+	request_uri = soup_uri_new_with_base (base_uri, "/content-length/long");
+	msg = soup_message_new_from_uri ("GET", request_uri);
+	soup_session_send_message (session, msg);
+	if (msg->status_code != SOUP_STATUS_OK) {
+		debug_printf (1, "    Unexpected response: %d %s\n",
+			      msg->status_code, msg->reason_phrase);
+		errors++;
+	} else {
+		declared_length = soup_message_headers_get_content_length (msg->response_headers);
+		debug_printf (2, "    Content-Length: %lu, body: %s\n",
+			      (gulong)declared_length, msg->response_body->data);
+		if (msg->response_body->length >= declared_length) {
+			debug_printf (1, "    Body length %lu >= declared length %lu\n",
+				      (gulong)msg->response_body->length,
+				      (gulong)declared_length);
+			errors++;
+		}
+	}
+	soup_uri_free (request_uri);
+	g_object_unref (msg);
+
+	debug_printf (1, "  Server claims 'Connection: close' but doesn't\n");
+	request_uri = soup_uri_new_with_base (base_uri, "/content-length/noclose");
+	msg = soup_message_new_from_uri ("GET", request_uri);
+	soup_session_send_message (session, msg);
+	if (msg->status_code != SOUP_STATUS_OK) {
+		debug_printf (1, "    Unexpected response: %d %s\n",
+			      msg->status_code, msg->reason_phrase);
+		errors++;
+	} else {
+		declared_length = soup_message_headers_get_content_length (msg->response_headers);
+		debug_printf (2, "    Content-Length: %lu, body: %s\n",
+			      (gulong)declared_length, msg->response_body->data);
+		if (msg->response_body->length != declared_length) {
+			debug_printf (1, "    Body length %lu != declared length %lu\n",
+				      (gulong)msg->response_body->length,
+				      (gulong)declared_length);
+			errors++;
+		}
+	}
+	soup_uri_free (request_uri);
+	g_object_unref (msg);
+
+	soup_test_session_abort_unref (session);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -498,6 +606,7 @@ main (int argc, char **argv)
 	do_msg_reuse_test ();
 	do_star_test ();
 	do_early_abort_test ();
+	do_content_length_framing_test ();
 
 	soup_uri_free (base_uri);
 	soup_test_server_quit_unref (server);