libsoup r1232 - in trunk: . libsoup tests

[danw svn gnome org]

Author: danw
Date: Thu Jan 29 18:40:24 2009
New Revision: 1232
URL: http://svn.gnome.org/viewvc/libsoup?rev=1232&view=rev

Log:
	* libsoup/soup-message-body.c (soup_message_body_wrote_chunk): Fix
	this; previously it would discard the entire message body after
	writing a SOUP_MEMORY_TEMPORARY chunk. Part of WebKit bug 18343.

	* libsoup/soup-message-io.c (io_write): use
	io->write_chunk->length *before* freeing io->write_chunk.

	* tests/chunk-test.c (do_temporary_test): new test to make sure
	that TEMPORARY buffers are handled properly.


Modified:
   trunk/ChangeLog
   trunk/libsoup/soup-message-body.c
   trunk/libsoup/soup-message-io.c
   trunk/tests/chunk-test.c

Modified: trunk/libsoup/soup-message-body.c
==============================================================================
--- trunk/libsoup/soup-message-body.c	(original)
+++ trunk/libsoup/soup-message-body.c	Thu Jan 29 18:40:24 2009
@@ -593,15 +593,16 @@
 /**
  * soup_message_body_wrote_chunk:
  * @body: a #SoupMessageBody
- * @chunk: a #SoupBuffer received from the network
+ * @chunk: a #SoupBuffer returned from soup_message_body_get_chunk()
  *
  * Handles the #SoupMessageBody part of writing a chunk of data to the
  * network. Normally this is a no-op, but if you have set @body's
- * accumulate flag to %FALSE, then this will cause @chunk (and any
- * chunks preceding it in @body) to be discarded to free up memory.
+ * accumulate flag to %FALSE, then this will cause @chunk to be
+ * discarded to free up memory.
  *
- * This is a low-level method which you should not normally need to
- * use.
+ * This is a low-level method which you should not need to use, and
+ * there are further restrictions on its proper use which are not
+ * documented here.
  **/
 void
 soup_message_body_wrote_chunk (SoupMessageBody *body, SoupBuffer *chunk)
@@ -612,15 +613,16 @@
 	if (priv->accumulate)
 		return;
 
-	do {
-		chunk2 = priv->chunks->data;
-		priv->chunks = g_slist_remove (priv->chunks, chunk2);
-		priv->base_offset += chunk2->length;
-		soup_buffer_free (chunk2);
-	} while (priv->chunks && chunk2 != chunk);
+	chunk2 = priv->chunks->data;
+	g_return_if_fail (chunk->length == chunk2->length);
+	g_return_if_fail (chunk == chunk2 || ((SoupBufferPrivate *)chunk2)->use == SOUP_MEMORY_TEMPORARY);
 
+	priv->chunks = g_slist_remove (priv->chunks, chunk2);
 	if (!priv->chunks)
 		priv->last = NULL;
+
+	priv->base_offset += chunk2->length;
+	soup_buffer_free (chunk2);
 }
 
 static SoupMessageBody *

Modified: trunk/libsoup/soup-message-io.c
==============================================================================
--- trunk/libsoup/soup-message-io.c	(original)
+++ trunk/libsoup/soup-message-io.c	Thu Jan 29 18:40:24 2009
@@ -548,8 +548,8 @@
 			return;
 
 		soup_message_body_wrote_chunk (io->write_body, io->write_chunk);
-		soup_buffer_free (io->write_chunk);
 		io->write_body_offset += io->write_chunk->length;
+		soup_buffer_free (io->write_chunk);
 		io->write_chunk = NULL;
 
 		SOUP_MESSAGE_IO_PREPARE_FOR_CALLBACK;

Modified: trunk/tests/chunk-test.c
==============================================================================
--- trunk/tests/chunk-test.c	(original)
+++ trunk/tests/chunk-test.c	Thu Jan 29 18:40:24 2009
@@ -240,6 +240,66 @@
 	g_checksum_free (gtd.check);
 }
 
+/* Make sure TEMPORARY buffers are handled properly with non-accumulating
+ * message bodies. Part of https://bugs.webkit.org/show_bug.cgi?id=18343
+ */
+
+static void
+temp_test_wrote_chunk (SoupMessage *msg, gpointer session)
+{
+	/* When the bug is present, the second chunk will also be
+	 * discarded after the first is written, which will cause
+	 * the I/O to stall since soup-message-io will think it's
+	 * done, but it hasn't written Content-Length bytes yet.
+	 * So add in another chunk to keep it going.
+	 */
+	if (!soup_message_body_get_chunk (msg->request_body,
+					  5)) {
+		debug_printf (1, "  Lost second chunk!\n");
+		errors++;
+		soup_session_abort (session);
+	}
+
+	g_signal_handlers_disconnect_by_func (msg, temp_test_wrote_chunk, session);
+}
+
+static void
+do_temporary_test (SoupSession *session, SoupURI *base_uri)
+{
+	SoupMessage *msg;
+	const char *client_md5, *server_md5;
+
+	debug_printf (1, "PUT w/ temporary buffers\n");
+
+	msg = soup_message_new_from_uri ("PUT", base_uri);
+	soup_message_body_append (msg->request_body, SOUP_MEMORY_TEMPORARY,
+				  "one\r\n", 5);
+	soup_message_body_append (msg->request_body, SOUP_MEMORY_STATIC,
+				  "two\r\n", 5);
+	soup_message_body_set_accumulate (msg->request_body, FALSE);
+
+	client_md5 = g_compute_checksum_for_string (G_CHECKSUM_MD5,
+						    "one\r\ntwo\r\n", 10);
+	g_signal_connect (msg, "wrote_chunk",
+			  G_CALLBACK (temp_test_wrote_chunk), session);
+	soup_session_send_message (session, msg);
+
+	if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) {
+		debug_printf (1, "  message failed: %d %s\n",
+			      msg->status_code, msg->reason_phrase);
+		errors++;
+	}
+
+	server_md5 = soup_message_headers_get (msg->response_headers, "Content-MD5");
+	if (!server_md5 || strcmp (client_md5, server_md5) != 0) {
+		debug_printf (1, "  client/server data mismatch: %s vs %s\n",
+			      client_md5, server_md5 ? server_md5 : "(null)");
+		errors++;
+	}
+
+	g_object_unref (msg);
+}
+
 static void
 do_chunk_tests (SoupURI *base_uri)
 {
@@ -249,6 +309,8 @@
 	do_request_test (session, base_uri);
 	debug_printf (2, "\n\n");
 	do_response_test (session, base_uri);
+	debug_printf (2, "\n\n");
+	do_temporary_test (session, base_uri);
 	soup_test_session_abort_unref (session);
 }