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

diff --git a/libsoup/soup-message-body.c b/libsoup/soup-message-body.c
index 8f1b6a7..a1d78f8 100644 (file)
--- 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 ee30b41..213a46b 100644 (file)
--- 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 56f202f..9aa1209 100644 (file)
--- 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 9cf901b..8505f00 100644 (file)
--- 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 8b1a3b6..435f7fa 100644 (file)
--- 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");