From f8e049184dd09117b9f545b4e51cba7785330f72 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 10 Aug 2011 21:50:54 -0400 Subject: [PATCH] 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 --- libsoup/soup-message-body.c | 11 +++-- libsoup/soup-message-io.c | 6 ++- libsoup/soup-message.c | 24 ++++++---- libsoup/soup-message.h | 1 + tests/chunk-test.c | 112 +++++++++++++++++++++++++++----------------- 5 files changed, 96 insertions(+), 58 deletions(-) diff --git a/libsoup/soup-message-body.c b/libsoup/soup-message-body.c index 8f1b6a7..a1d78f8 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 ee30b41..213a46b 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 56f202f..9aa1209 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 9cf901b..8505f00 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 8b1a3b6..435f7fa 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"); -- 2.7.4