diff options
author | Dan Winship <danw@src.gnome.org> | 2008-04-04 13:20:01 +0000 |
---|---|---|
committer | Dan Winship <danw@src.gnome.org> | 2008-04-04 13:20:01 +0000 |
commit | 230333d817d6d42923e278ec49b6d257d59abf17 (patch) | |
tree | 0748cadbc4f615add0fc3369d1bb7426ba14d829 | |
parent | 0fe9af2d15c4bc8dcd3413ace0c8e3f6dbe4b487 (diff) | |
download | libsoup-230333d817d6d42923e278ec49b6d257d59abf17.tar.gz |
add a new signal "wrote-body-data" to address the problem that
* libsoup/soup-message.c: add a new signal "wrote-body-data" to
address the problem that "wrote-chunk" is not usable for progress
info (especially with non-chunked encoding). #525101, suggested by
Christian Kellner.
* libsoup/soup-message-io.c (write_data): emit wrote-body-data as
appropriate.
(io_write): update so that (a) Content-Length writes can be done
in multiple chunks (as long as the caller explicitly sets the
Content-Length header beforehand), and (b) the body data doesn't
get copied an extra time. Based on a patch from Christian.
* libsoup/soup-message-client-io.c (get_request_headers): Don't
update the Content-Length header if it's already set, even if it
doesn't match the (current) body length.
* tests/chunk-test.c: test some chunk-encoding-related behavior
svn path=/trunk/; revision=1120
-rw-r--r-- | ChangeLog | 20 | ||||
-rw-r--r-- | libsoup/soup-message-client-io.c | 3 | ||||
-rw-r--r-- | libsoup/soup-message-io.c | 75 | ||||
-rw-r--r-- | libsoup/soup-message.c | 52 | ||||
-rw-r--r-- | libsoup/soup-message.h | 1 | ||||
-rw-r--r-- | tests/Makefile.am | 3 | ||||
-rw-r--r-- | tests/chunk-test.c | 314 |
7 files changed, 453 insertions, 15 deletions
@@ -1,3 +1,23 @@ +2008-04-04 Dan Winship <danw@gnome.org> + + * libsoup/soup-message.c: add a new signal "wrote-body-data" to + address the problem that "wrote-chunk" is not usable for progress + info (especially with non-chunked encoding). #525101, suggested by + Christian Kellner. + + * libsoup/soup-message-io.c (write_data): emit wrote-body-data as + appropriate. + (io_write): update so that (a) Content-Length writes can be done + in multiple chunks (as long as the caller explicitly sets the + Content-Length header beforehand), and (b) the body data doesn't + get copied an extra time. Based on a patch from Christian. + + * libsoup/soup-message-client-io.c (get_request_headers): Don't + update the Content-Length header if it's already set, even if it + doesn't match the (current) body length. + + * tests/chunk-test.c: test some chunk-encoding-related behavior + 2008-04-03 Dan Winship <danw@gnome.org> Be more aggressive about closing unused persistent connections diff --git a/libsoup/soup-message-client-io.c b/libsoup/soup-message-client-io.c index 777ca083..1eae8f5d 100644 --- a/libsoup/soup-message-client-io.c +++ b/libsoup/soup-message-client-io.c @@ -106,7 +106,8 @@ get_request_headers (SoupMessage *req, GString *header, *encoding = soup_message_headers_get_encoding (req->request_headers); if (*encoding != SOUP_ENCODING_CHUNKED && - req->request_body->length > 0) { + req->request_body->length > 0 && + !soup_message_headers_get_content_length (req->request_headers)) { soup_message_headers_set_content_length (req->request_headers, req->request_body->length); } diff --git a/libsoup/soup-message-io.c b/libsoup/soup-message-io.c index 2bc1fac2..17d24b54 100644 --- a/libsoup/soup-message-io.c +++ b/libsoup/soup-message-io.c @@ -59,6 +59,7 @@ typedef struct { SoupMessageBody *write_body; SoupBuffer *write_chunk; gsize write_body_offset; + guint write_length; guint written; guint read_tag, write_tag, err_tag; @@ -343,13 +344,15 @@ read_body_chunk (SoupMessage *msg) * read_metadata() for an explanation of the return value. */ static gboolean -write_data (SoupMessage *msg, const char *data, guint len) +write_data (SoupMessage *msg, const char *data, guint len, gboolean body) { SoupMessagePrivate *priv = SOUP_MESSAGE_GET_PRIVATE (msg); SoupMessageIOData *io = priv->io_data; SoupSocketIOStatus status; gsize nwrote; GError *error = NULL; + SoupBuffer *chunk; + const char *start; while (len > io->written) { status = soup_socket_write (io->sock, @@ -366,7 +369,20 @@ write_data (SoupMessage *msg, const char *data, guint len) return FALSE; case SOUP_SOCKET_OK: + start = data + io->written; io->written += nwrote; + + if (body) { + if (io->write_length) + io->write_length -= nwrote; + + chunk = soup_buffer_new (SOUP_MEMORY_TEMPORARY, + start, nwrote); + SOUP_MESSAGE_IO_PREPARE_FOR_CALLBACK; + soup_message_wrote_body_data (msg, chunk); + soup_buffer_free (chunk); + SOUP_MESSAGE_IO_RETURN_VAL_IF_CANCELLED_OR_PAUSED (FALSE); + } break; } } @@ -433,11 +449,19 @@ io_write (SoupSocket *sock, SoupMessage *msg) } } - if (!write_data (msg, io->write_buf->str, io->write_buf->len)) + if (!write_data (msg, io->write_buf->str, + io->write_buf->len, FALSE)) return; g_string_truncate (io->write_buf, 0); + if (io->write_encoding != SOUP_ENCODING_CHUNKED) { + SoupMessageHeaders *hdrs = + (io->mode == SOUP_MESSAGE_IO_CLIENT) ? + msg->request_headers : msg->response_headers; + io->write_length = soup_message_headers_get_content_length (hdrs); + } + if (io->mode == SOUP_MESSAGE_IO_SERVER && SOUP_STATUS_IS_INFORMATIONAL (msg->status_code)) { if (msg->status_code == SOUP_STATUS_CONTINUE) { @@ -494,22 +518,46 @@ io_write (SoupSocket *sock, SoupMessage *msg) case SOUP_MESSAGE_IO_STATE_BODY: - if (!io->write_chunk) - io->write_chunk = soup_message_body_flatten (io->write_body); + if (!io->write_length) { + io->write_state = SOUP_MESSAGE_IO_STATE_FINISHING; + + SOUP_MESSAGE_IO_PREPARE_FOR_CALLBACK; + soup_message_wrote_body (msg); + SOUP_MESSAGE_IO_RETURN_IF_CANCELLED_OR_PAUSED; + break; + } + + if (!io->write_chunk) { + io->write_chunk = soup_message_body_get_chunk (io->write_body, io->write_body_offset); + if (!io->write_chunk) { + soup_message_io_pause (msg); + return; + } + if (io->write_chunk->length > io->write_length) { + /* App is trying to write more than it + * claimed it would; we have to truncate. + */ + SoupBuffer *truncated = + soup_buffer_new_subbuffer (io->write_chunk, + 0, io->write_length); + soup_buffer_free (io->write_chunk); + io->write_chunk = truncated; + } + } + if (!write_data (msg, io->write_chunk->data, - io->write_chunk->length)) + io->write_chunk->length, TRUE)) return; + soup_buffer_free (io->write_chunk); + io->write_body_offset += io->write_chunk->length; io->write_chunk = NULL; - io->write_state = SOUP_MESSAGE_IO_STATE_FINISHING; - SOUP_MESSAGE_IO_PREPARE_FOR_CALLBACK; - soup_message_wrote_body (msg); + soup_message_wrote_chunk (msg); SOUP_MESSAGE_IO_RETURN_IF_CANCELLED_OR_PAUSED; break; - case SOUP_MESSAGE_IO_STATE_CHUNK_SIZE: if (!io->write_chunk) { io->write_chunk = soup_message_body_get_chunk (io->write_body, io->write_body_offset); @@ -522,7 +570,8 @@ io_write (SoupSocket *sock, SoupMessage *msg) io->write_body_offset += io->write_chunk->length; } - if (!write_data (msg, io->write_buf->str, io->write_buf->len)) + if (!write_data (msg, io->write_buf->str, + io->write_buf->len, FALSE)) return; g_string_truncate (io->write_buf, 0); @@ -539,7 +588,7 @@ io_write (SoupSocket *sock, SoupMessage *msg) case SOUP_MESSAGE_IO_STATE_CHUNK: if (!write_data (msg, io->write_chunk->data, - io->write_chunk->length)) + io->write_chunk->length, TRUE)) return; soup_buffer_free (io->write_chunk); @@ -556,7 +605,7 @@ io_write (SoupSocket *sock, SoupMessage *msg) case SOUP_MESSAGE_IO_STATE_CHUNK_END: if (!write_data (msg, SOUP_MESSAGE_IO_EOL, - SOUP_MESSAGE_IO_EOL_LEN)) + SOUP_MESSAGE_IO_EOL_LEN, FALSE)) return; io->write_state = SOUP_MESSAGE_IO_STATE_CHUNK_SIZE; @@ -565,7 +614,7 @@ io_write (SoupSocket *sock, SoupMessage *msg) case SOUP_MESSAGE_IO_STATE_TRAILERS: if (!write_data (msg, SOUP_MESSAGE_IO_EOL, - SOUP_MESSAGE_IO_EOL_LEN)) + SOUP_MESSAGE_IO_EOL_LEN, FALSE)) return; io->write_state = SOUP_MESSAGE_IO_STATE_FINISHING; diff --git a/libsoup/soup-message.c b/libsoup/soup-message.c index 541cc4b6..83d3dff6 100644 --- a/libsoup/soup-message.c +++ b/libsoup/soup-message.c @@ -75,6 +75,7 @@ enum { WROTE_INFORMATIONAL, WROTE_HEADERS, WROTE_CHUNK, + WROTE_BODY_DATA, WROTE_BODY, GOT_INFORMATIONAL, @@ -212,6 +213,13 @@ soup_message_class_init (SoupMessageClass *message_class) * @msg: the message * * Emitted immediately after writing a body chunk for a message. + * + * Note that this signal is not parallel to + * #SoupMessage::got_chunk; it is emitted only when a complete + * chunk (added with soup_message_body_append() or + * soup_message_body_append_buffer()) has been written. To get + * more useful continuous progress information, use + * #SoupMessage::wrote_body_data. **/ signals[WROTE_CHUNK] = g_signal_new ("wrote_chunk", @@ -223,6 +231,28 @@ soup_message_class_init (SoupMessageClass *message_class) G_TYPE_NONE, 0); /** + * SoupMessage::wrote-body-data: + * @msg: the message + * @chunk: the data written + * + * Emitted immediately after writing a portion of the message + * body to the network. + * + * Unlike #SoupMessage::wrote_chunk, this is emitted after + * every successful write() call, not only after finishing a + * complete "chunk". + **/ + signals[WROTE_BODY_DATA] = + g_signal_new ("wrote_body_data", + G_OBJECT_CLASS_TYPE (object_class), + G_SIGNAL_RUN_FIRST, + 0, /* FIXME after next ABI break */ + NULL, NULL, + soup_marshal_NONE__BOXED, + G_TYPE_NONE, 1, + SOUP_TYPE_BUFFER); + + /** * SoupMessage::wrote-body: * @msg: the message * @@ -319,7 +349,13 @@ soup_message_class_init (SoupMessageClass *message_class) NULL, NULL, soup_marshal_NONE__BOXED, G_TYPE_NONE, 1, - SOUP_TYPE_BUFFER); + /* Use %G_SIGNAL_TYPE_STATIC_SCOPE so that + * the %SOUP_MEMORY_TEMPORARY buffers used + * by soup-message-io.c when emitting this + * signal don't get forcibly copied by + * g_signal_emit(). + */ + SOUP_TYPE_BUFFER | G_SIGNAL_TYPE_STATIC_SCOPE); /** * SoupMessage::got-body: @@ -646,6 +682,20 @@ soup_message_wrote_chunk (SoupMessage *msg) } /** + * soup_message_wrote_body_data: + * @msg: a #SoupMessage + * @chunk: the data written + * + * Emits the %wrote_body_data signal, indicating that the IO layer + * finished writing a portion of @msg's body. + **/ +void +soup_message_wrote_body_data (SoupMessage *msg, SoupBuffer *chunk) +{ + g_signal_emit (msg, signals[WROTE_BODY_DATA], 0, chunk); +} + +/** * soup_message_wrote_body: * @msg: a #SoupMessage * diff --git a/libsoup/soup-message.h b/libsoup/soup-message.h index 002067d4..262994cc 100644 --- a/libsoup/soup-message.h +++ b/libsoup/soup-message.h @@ -145,6 +145,7 @@ void soup_message_set_chunk_allocator (SoupMessage *msg, void soup_message_wrote_informational (SoupMessage *msg); void soup_message_wrote_headers (SoupMessage *msg); void soup_message_wrote_chunk (SoupMessage *msg); +void soup_message_wrote_body_data (SoupMessage *msg, SoupBuffer *chunk); void soup_message_wrote_body (SoupMessage *msg); void soup_message_got_informational (SoupMessage *msg); void soup_message_got_headers (SoupMessage *msg); diff --git a/tests/Makefile.am b/tests/Makefile.am index 47f8317d..137b165f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -9,6 +9,7 @@ INCLUDES = \ LIBS = $(top_builddir)/libsoup/libsoup-$(SOUP_API_VERSION).la noinst_PROGRAMS = \ + chunk-test \ context-test \ continue-test \ date \ @@ -29,6 +30,7 @@ noinst_PROGRAMS = \ TEST_SRCS = test-utils.c test-utils.h auth_test_SOURCES = auth-test.c $(TEST_SRCS) +chunk_test_SOURCES = chunk-test.c $(TEST_SRCS) context_test_SOURCES = context-test.c $(TEST_SRCS) continue_test_SOURCES = continue-test.c $(TEST_SRCS) date_SOURCES = date.c $(TEST_SRCS) @@ -63,6 +65,7 @@ XMLRPC_TESTS = xmlrpc-test xmlrpc-server-test endif TESTS = \ + chunk-test \ context-test \ continue-test \ date \ diff --git a/tests/chunk-test.c b/tests/chunk-test.c new file mode 100644 index 00000000..6bdd909c --- /dev/null +++ b/tests/chunk-test.c @@ -0,0 +1,314 @@ +/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- */ +/* + * Copyright (C) 2008 Red Hat, Inc. + */ + +#include "config.h" + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +#include <glib.h> +#include <libsoup/soup.h> + +#include "test-utils.h" + +typedef struct { + SoupSession *session; + SoupBuffer *chunks[3]; + int next, nwrote; +} PutTestData; + +static SoupBuffer * +error_chunk_allocator (SoupMessage *msg, gsize max_len, gpointer user_data) +{ + /* This should never be called, because there is no response body. */ + debug_printf (1, " error_chunk_allocator called!\n"); + errors++; + return soup_buffer_new (SOUP_MEMORY_TAKE, g_malloc (100), 100); +} + +static void +write_next_chunk (SoupMessage *msg, gpointer user_data) +{ + PutTestData *ptd = user_data; + + debug_printf (2, " writing chunk\n"); + +#ifdef IMPLEMENTED_OVERWRITE_CHUNKS_FOR_REQUESTS + if (ptd->next > 0 && ptd->chunks[ptd->next - 1]) { + debug_printf (1, " error: next chunk requested before last one freed!\n"); + errors++; + } +#endif + + if (ptd->next < G_N_ELEMENTS (ptd->chunks)) { + soup_message_body_append_buffer (msg->request_body, + ptd->chunks[ptd->next]); + soup_buffer_free (ptd->chunks[ptd->next]); + ptd->next++; + } else + soup_message_body_complete (msg->request_body); + soup_session_unpause_message (ptd->session, msg); +} + +static void +wrote_body_data (SoupMessage *msg, SoupBuffer *chunk, gpointer user_data) +{ + PutTestData *ptd = user_data; + + debug_printf (2, " wrote_body_data, %d bytes\n", + (int)chunk->length); + ptd->nwrote += chunk->length; +} + +static void +clear_buffer_ptr (gpointer data) +{ + SoupBuffer **buffer_ptr = data; + + debug_printf (2, " clearing chunk\n"); + if (*buffer_ptr) { + (*buffer_ptr)->length = 0; + *buffer_ptr = NULL; + } else { + debug_printf (2, " chunk is already clear!\n"); + errors++; + } +} + +/* Put a chunk containing @text into *@buffer, set up so that it will + * clear out *@buffer when the chunk is freed, allowing us to make sure + * the set_accumulate(FALSE) is working. + */ +static void +make_put_chunk (SoupBuffer **buffer, const char *text) +{ + *buffer = soup_buffer_new_with_owner (text, strlen (text), + buffer, clear_buffer_ptr); +} + +static void +do_request_test (SoupSession *session, SoupURI *base_uri) +{ + PutTestData ptd; + SoupMessage *msg; + const char *client_md5, *server_md5; + GChecksum *check; + int i, length; + + debug_printf (1, "PUT\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 = 0; + + check = g_checksum_new (G_CHECKSUM_MD5); + length = 0; + for (i = 0; i < 3; i++) { + g_checksum_update (check, (guchar *)ptd.chunks[i]->data, + ptd.chunks[i]->length); + length += ptd.chunks[i]->length; + } + client_md5 = g_checksum_get_string (check); + + msg = soup_message_new_from_uri ("PUT", base_uri); + soup_message_headers_set_encoding (msg->request_headers, SOUP_ENCODING_CHUNKED); + soup_message_set_chunk_allocator (msg, error_chunk_allocator, NULL, NULL); + g_signal_connect (msg, "wrote_headers", + G_CALLBACK (write_next_chunk), &ptd); + g_signal_connect (msg, "wrote_chunk", + G_CALLBACK (write_next_chunk), &ptd); + g_signal_connect (msg, "wrote_body_data", + G_CALLBACK (wrote_body_data), &ptd); + 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++; + } + + if (msg->request_body->data) { + debug_printf (1, " msg->request_body set!\n"); + errors++; + } + if (msg->request_body->length != length || length != ptd.nwrote) { + debug_printf (1, " sent length mismatch: %d vs %d vs %d\n", + msg->request_body->length, length, ptd.nwrote); + 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); + g_checksum_free (check); +} + +typedef struct { + SoupBuffer *current_chunk; + GChecksum *check; + int length; +} GetTestData; + +static SoupBuffer * +chunk_allocator (SoupMessage *msg, gsize max_len, gpointer user_data) +{ + GetTestData *gtd = user_data; + + debug_printf (2, " allocating chunk\n"); + + if (gtd->current_chunk) { + debug_printf (1, " error: next chunk allocated before last one freed!\n"); + errors++; + } + gtd->current_chunk = soup_buffer_new_with_owner (g_malloc (6), 6, + >d->current_chunk, + clear_buffer_ptr); + return gtd->current_chunk; +} + +static void +got_chunk (SoupMessage *msg, SoupBuffer *chunk, gpointer user_data) +{ + GetTestData *gtd = user_data; + + debug_printf (2, " got chunk, %d bytes\n", + (int)chunk->length); + if (chunk != gtd->current_chunk) { + debug_printf (1, "chunk mismatch! %p vs %p\n", + chunk, gtd->current_chunk); + } + + g_checksum_update (gtd->check, (guchar *)chunk->data, chunk->length); + gtd->length += chunk->length; +} + +static void +do_response_test (SoupSession *session, SoupURI *base_uri) +{ + GetTestData gtd; + SoupMessage *msg; + const char *client_md5, *server_md5; + + debug_printf (1, "GET\n"); + + gtd.current_chunk = NULL; + gtd.length = 0; + gtd.check = g_checksum_new (G_CHECKSUM_MD5); + + msg = soup_message_new_from_uri ("GET", base_uri); + soup_message_set_flags (msg, SOUP_MESSAGE_OVERWRITE_CHUNKS); + soup_message_set_chunk_allocator (msg, chunk_allocator, >d, NULL); + g_signal_connect (msg, "got_chunk", + G_CALLBACK (got_chunk), >d); + 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++; + } + + if (msg->response_body->data) { + debug_printf (1, " msg->response_body set!\n"); + errors++; + } + if (soup_message_headers_get_content_length (msg->response_headers) != gtd.length) { + debug_printf (1, " received length mismatch: %d vs %d\n", + (int)soup_message_headers_get_content_length (msg->response_headers), gtd.length); + errors++; + } + + client_md5 = g_checksum_get_string (gtd.check); + 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); + g_checksum_free (gtd.check); +} + +static void +do_chunk_tests (SoupURI *base_uri) +{ + SoupSession *session; + + session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL); + do_request_test (session, base_uri); + debug_printf (2, "\n\n"); + do_response_test (session, base_uri); + soup_session_abort (session); + g_object_unref (session); +} + +static void +server_callback (SoupServer *server, SoupMessage *msg, + const char *path, GHashTable *query, + SoupClientContext *context, gpointer data) +{ + SoupMessageBody *md5_body; + char *md5; + + if (msg->method == SOUP_METHOD_GET) { + soup_message_set_response (msg, "text/plain", + SOUP_MEMORY_STATIC, + "three\r\ntwo\r\none\r\n", + strlen ("three\r\ntwo\r\none\r\n")); + soup_buffer_free (soup_message_body_flatten (msg->response_body)); + md5_body = msg->response_body; + soup_message_set_status (msg, SOUP_STATUS_OK); + } else if (msg->method == SOUP_METHOD_PUT) { + soup_message_set_status (msg, SOUP_STATUS_CREATED); + md5_body = msg->request_body; + } else { + soup_message_set_status (msg, SOUP_STATUS_METHOD_NOT_ALLOWED); + return; + } + + md5 = g_compute_checksum_for_data (G_CHECKSUM_MD5, + (guchar *)md5_body->data, + md5_body->length); + soup_message_headers_append (msg->response_headers, + "Content-MD5", md5); + g_free (md5); +} + +int +main (int argc, char **argv) +{ + GMainLoop *loop; + SoupServer *server; + guint port; + SoupURI *base_uri; + + test_init (argc, argv, NULL); + + server = soup_test_server_new (TRUE); + soup_server_add_handler (server, NULL, + server_callback, NULL, NULL); + port = soup_server_get_port (server); + + loop = g_main_loop_new (NULL, TRUE); + + base_uri = soup_uri_new ("http://localhost"); + soup_uri_set_port (base_uri, port); + do_chunk_tests (base_uri); + soup_uri_free (base_uri); + + g_main_loop_unref (loop); + + test_cleanup (); + return errors != 0; +} |