Add SOUP_MESSAGE_CAN_REBUILD, for regeneratable streamed request bodies

Long ago, soup_message_body_set_accumulate() was made into a no-op on
request bodies, because otherwise there would be no body available to
send if the request was restarted. Add a SOUP_MESSAGE_CAN_REBUILD
flag, that indicates that the caller takes responsibility for handling
this, and properly discard chunks if that is set.

Also update chunk-test to test this, and (finally!) remove the FIXME
there, since we are now testing the !accumulate codepath again
properly.

https://bugzilla.gnome.org/show_bug.cgi?id=656650

5 files changed, 96 insertions, 58 deletions

diff --git a/libsoup/soup-message-body.c b/libsoup/soup-message-body.c
index 8f1b6a76..a1d78f86 100644
--- a/libsoup/soup-message-body.c
+++ b/libsoup/soup-message-body.c
@@ -396,13 +396,16 @@ soup_message_body_new (void)
  * discarded after its corresponding #SoupMessage::wrote_chunk signal
  * is emitted.
  *
- * (If you set the flag to %FALSE on the %request_body of a
- * client-side message, it will block the accumulation of chunks into
- * @body's %data field, but it will not cause the chunks to be
+ * If you set the flag to %FALSE on the %request_body of a client-side
+ * message, it will block the accumulation of chunks into @body's
+ * %data field, but it will not normally cause the chunks to be
  * discarded after being written like in the server-side
  * %response_body case, because the request body needs to be kept
  * around in case the request needs to be sent a second time due to
- * redirection or authentication.)
+ * redirection or authentication. However, if you set the
+ * %SOUP_MESSAGE_CAN_REBUILD flag on the message, then the chunks will
+ * be discarded, and you will be responsible for recreating the
+ * request body after the #SoupMessage::restarted signal is emitted.
  *
  * Since: 2.4.1
  **/
diff --git a/libsoup/soup-message-io.c b/libsoup/soup-message-io.c
index ee30b41a..213a46bb 100644
--- a/libsoup/soup-message-io.c
+++ b/libsoup/soup-message-io.c
@@ -728,7 +728,8 @@ io_write (SoupSocket *sock, SoupMessage *msg)
 				 io->write_chunk->length, TRUE))
 			return;
 
-		if (io->mode == SOUP_MESSAGE_IO_SERVER)
+		if (io->mode == SOUP_MESSAGE_IO_SERVER ||
+		    priv->msg_flags & SOUP_MESSAGE_CAN_REBUILD)
 			soup_message_body_wrote_chunk (io->write_body, io->write_chunk);
 		io->write_body_offset += io->write_chunk->length;
 		soup_buffer_free (io->write_chunk);
@@ -772,7 +773,8 @@ io_write (SoupSocket *sock, SoupMessage *msg)
 				 io->write_chunk->length, TRUE))
 			return;
 
-		if (io->mode == SOUP_MESSAGE_IO_SERVER)
+		if (io->mode == SOUP_MESSAGE_IO_SERVER ||
+		    priv->msg_flags & SOUP_MESSAGE_CAN_REBUILD)
 			soup_message_body_wrote_chunk (io->write_body, io->write_chunk);
 		soup_buffer_free (io->write_chunk);
 		io->write_chunk = NULL;
diff --git a/libsoup/soup-message.c b/libsoup/soup-message.c
index 56f202f9..9aa12090 100644
--- a/libsoup/soup-message.c
+++ b/libsoup/soup-message.c
@@ -1098,6 +1098,11 @@ soup_message_content_sniffed (SoupMessage *msg, const char *content_type, GHashT
 void
 soup_message_restarted (SoupMessage *msg)
 {
+	SoupMessagePrivate *priv = SOUP_MESSAGE_GET_PRIVATE (msg);
+
+	if (priv->msg_flags & SOUP_MESSAGE_CAN_REBUILD)
+		soup_message_body_truncate (msg->request_body);
+
 	g_signal_emit (msg, signals[RESTARTED], 0);
 }
 
@@ -1411,17 +1416,20 @@ soup_message_cleanup_response (SoupMessage *req)
 /**
  * SoupMessageFlags:
  * @SOUP_MESSAGE_NO_REDIRECT: The session should not follow redirect
- * (3xx) responses received by this message.
+ *   (3xx) responses received by this message.
+ * @SOUP_MESSAGE_CAN_REBUILD: The caller will rebuild the request
+ *   body if the message is restarted; see
+ *   soup_message_body_set_accumulate() for more details.
  * @SOUP_MESSAGE_OVERWRITE_CHUNKS: Deprecated: equivalent to calling
- * soup_message_body_set_accumulate() on the incoming message body
- * (ie, %response_body for a client-side request), passing %FALSE.
+ *   soup_message_body_set_accumulate() on the incoming message body
+ *   (ie, %response_body for a client-side request), passing %FALSE.
  * @SOUP_MESSAGE_CONTENT_DECODED: Set by #SoupContentDecoder to
- * indicate that it has removed the Content-Encoding on a message (and
- * so headers such as Content-Length may no longer accurately describe
- * the body).
+ *   indicate that it has removed the Content-Encoding on a message (and
+ *   so headers such as Content-Length may no longer accurately describe
+ *   the body).
  * @SOUP_MESSAGE_CERTIFICATE_TRUSTED: if %TRUE after an https response
- * has been received, indicates that the server's SSL certificate is
- * trusted according to the session's CA.
+ *   has been received, indicates that the server's SSL certificate is
+ *   trusted according to the session's CA.
  *
  * Various flags that can be set on a #SoupMessage to alter its
  * behavior.
diff --git a/libsoup/soup-message.h b/libsoup/soup-message.h
index 9cf901b4..8505f00b 100644
--- a/libsoup/soup-message.h
+++ b/libsoup/soup-message.h
@@ -113,6 +113,7 @@ void             soup_message_set_first_party     (SoupMessage       *msg,
 
 typedef enum {
 	SOUP_MESSAGE_NO_REDIRECT          = (1 << 1),
+	SOUP_MESSAGE_CAN_REBUILD          = (1 << 2),
 #ifndef LIBSOUP_DISABLE_DEPRECATED
 	SOUP_MESSAGE_OVERWRITE_CHUNKS     = (1 << 3),
 #endif
diff --git a/tests/chunk-test.c b/tests/chunk-test.c
index 8b1a3b60..435f7fa2 100644
--- a/tests/chunk-test.c
+++ b/tests/chunk-test.c
@@ -37,13 +37,9 @@ write_next_chunk (SoupMessage *msg, gpointer user_data)
 
 	debug_printf (2, "  writing chunk %d\n", ptd->next);
 
-	if (ptd->next > 0 && ptd->chunks[ptd->next - 1]) {
-		if (ptd->streaming) {
-			debug_printf (1, "  error: next chunk requested before last one freed!\n");
-			errors++;
-		} else {
-			debug_printf (0, "  ignoring bug in test case... FIXME!\n");
-		}
+	if (ptd->streaming && ptd->next > 0 && ptd->chunks[ptd->next - 1]) {
+		debug_printf (1, "  error: next chunk requested before last one freed!\n");
+		errors++;
 	}
 
 	if (ptd->next < G_N_ELEMENTS (ptd->chunks)) {
@@ -56,7 +52,9 @@ write_next_chunk (SoupMessage *msg, gpointer user_data)
 	soup_session_unpause_message (ptd->session, msg);
 }
 
-/* This is not a supported part of the API. Don't try this at home */
+/* This is not a supported part of the API. Use SOUP_MESSAGE_CAN_REBUILD
+ * instead.
+ */
 static void
 write_next_chunk_streaming_hack (SoupMessage *msg, gpointer user_data)
 {
@@ -114,28 +112,49 @@ make_put_chunk (SoupBuffer **buffer, const char *text)
 }
 
 static void
-restarted_streaming_hack (SoupMessage *msg, gpointer user_data)
+setup_request_body (PutTestData *ptd)
 {
-	PutTestData *ptd = user_data;
-
-	/* We're streaming, and we had to restart. So the data need
-	   to be regenerated. That's the *point* of this test; we don't
-	   *want* it to accumulate in the request body */
 	make_put_chunk (&ptd->chunks[0], "one\r\n");
 	make_put_chunk (&ptd->chunks[1], "two\r\n");
 	make_put_chunk (&ptd->chunks[2], "three\r\n");
 	ptd->next = ptd->nwrote = ptd->nfreed = 0;
+}
 
-	debug_printf (2, "  truncating request body on restart\n");
-	soup_message_body_truncate (msg->request_body);
+static void
+restarted_streaming (SoupMessage *msg, gpointer user_data)
+{
+	PutTestData *ptd = user_data;
 
-	/* The redirect will turn it into a GET request. Fix that... */
-	soup_message_headers_set_encoding (msg->request_headers, SOUP_ENCODING_CHUNKED);
+	debug_printf (2, "  --restarting--\n");
+
+	/* We're streaming, and we had to restart. So the data need
+	 * to be regenerated.
+	 */
+	setup_request_body (ptd);
+
+	/* The 302 redirect will turn it into a GET request and
+	 * reset the body encoding back to "NONE". Fix that.
+	 */
+	soup_message_headers_set_encoding (msg->request_headers,
+					   SOUP_ENCODING_CHUNKED);
 	msg->method = SOUP_METHOD_PUT;
 }
 
 static void
-do_request_test (SoupSession *session, SoupURI *base_uri, int test)
+restarted_streaming_hack (SoupMessage *msg, gpointer user_data)
+{
+	restarted_streaming (msg, user_data);
+	soup_message_body_truncate (msg->request_body);
+}
+
+typedef enum {
+	HACKY_STREAMING  = (1 << 0),
+	PROPER_STREAMING = (1 << 1),
+	RESTART          = (1 << 2)
+} RequestTestFlags;
+
+static void
+do_request_test (SoupSession *session, SoupURI *base_uri, RequestTestFlags flags)
 {
 	SoupURI *uri = base_uri;
 	PutTestData ptd;
@@ -143,32 +162,22 @@ do_request_test (SoupSession *session, SoupURI *base_uri, int test)
 	const char *client_md5, *server_md5;
 	GChecksum *check;
 	int i, length;
-	gboolean streaming = FALSE;
-
-	switch (test) {
-	case 0:
-		debug_printf (1, "PUT\n");
-		break;
 
-	case 1:
-		debug_printf (1, "PUT w/ streaming\n");
-		streaming = TRUE;
-		break;
-
-	case 2:
-		debug_printf (1, "PUT w/ streaming and restart\n");
-		streaming = TRUE;
+	debug_printf (1, "PUT");
+	if (flags & HACKY_STREAMING)
+		debug_printf (1, " w/ hacky streaming");
+	else if (flags & PROPER_STREAMING)
+		debug_printf (1, " w/ proper streaming");
+	if (flags & RESTART) {
+		debug_printf (1, " and restart");
 		uri = soup_uri_copy (base_uri);
 		soup_uri_set_path (uri, "/redirect");
-		break;
 	}
+	debug_printf (1, "\n");
 
 	ptd.session = session;
-	make_put_chunk (&ptd.chunks[0], "one\r\n");
-	make_put_chunk (&ptd.chunks[1], "two\r\n");
-	make_put_chunk (&ptd.chunks[2], "three\r\n");
-	ptd.next = ptd.nwrote = ptd.nfreed = 0;
-	ptd.streaming = streaming;
+	setup_request_body (&ptd);
+	ptd.streaming = flags & (HACKY_STREAMING | PROPER_STREAMING);
 
 	check = g_checksum_new (G_CHECKSUM_MD5);
 	length = 0;
@@ -183,15 +192,26 @@ do_request_test (SoupSession *session, SoupURI *base_uri, int test)
 	soup_message_headers_set_encoding (msg->request_headers, SOUP_ENCODING_CHUNKED);
 	soup_message_body_set_accumulate (msg->request_body, FALSE);
 	soup_message_set_chunk_allocator (msg, error_chunk_allocator, NULL, NULL);
-	if (streaming) {
+	if (flags & HACKY_STREAMING) {
 		g_signal_connect (msg, "wrote_chunk",
 				  G_CALLBACK (write_next_chunk_streaming_hack), &ptd);
-		g_signal_connect (msg, "restarted",
-				  G_CALLBACK (restarted_streaming_hack), &ptd);
+		if (flags & RESTART) {
+			g_signal_connect (msg, "restarted",
+					  G_CALLBACK (restarted_streaming_hack), &ptd);
+		}
 	} else {
 		g_signal_connect (msg, "wrote_chunk",
 				  G_CALLBACK (write_next_chunk), &ptd);
 	}
+
+	if (flags & PROPER_STREAMING) {
+		soup_message_set_flags (msg, SOUP_MESSAGE_CAN_REBUILD);
+		if (flags & RESTART) {
+			g_signal_connect (msg, "restarted",
+					  G_CALLBACK (restarted_streaming), &ptd);
+		}
+	}
+
 	g_signal_connect (msg, "wrote_headers",
 			  G_CALLBACK (write_next_chunk), &ptd);
 	g_signal_connect (msg, "wrote_body_data",
@@ -391,9 +411,13 @@ do_chunk_tests (SoupURI *base_uri)
 	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
 	do_request_test (session, base_uri, 0);
 	debug_printf (2, "\n\n");
-	do_request_test (session, base_uri, 1);
+	do_request_test (session, base_uri, PROPER_STREAMING);
+	debug_printf (2, "\n\n");
+	do_request_test (session, base_uri, PROPER_STREAMING | RESTART);
+	debug_printf (2, "\n\n");
+	do_request_test (session, base_uri, HACKY_STREAMING);
 	debug_printf (2, "\n\n");
-	do_request_test (session, base_uri, 2);
+	do_request_test (session, base_uri, HACKY_STREAMING | RESTART);
 	debug_printf (2, "\n\n");
 	do_response_test (session, base_uri);
 	debug_printf (2, "\n\n");