Repository: thrift Updated Branches: refs/heads/master e1358ce8f -> 7fa9848b8
THRIFT-2680 c_glib: ThriftFramedTransport fails when peer unexpectedly closes connection Patch: Simon South Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/7fa9848b Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/7fa9848b Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/7fa9848b Branch: refs/heads/master Commit: 7fa9848b85479f81767a36a96e7e9805683015d4 Parents: e1358ce Author: Roger Meier <[email protected]> Authored: Mon Sep 1 20:21:33 2014 +0200 Committer: Roger Meier <[email protected]> Committed: Mon Sep 1 20:21:33 2014 +0200 ---------------------------------------------------------------------- .../c_glib/transport/thrift_framed_transport.c | 55 ++++++++----- lib/c_glib/test/testframedtransport.c | 87 ++++++++++++++++++++ 2 files changed, 120 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/7fa9848b/lib/c_glib/src/thrift/c_glib/transport/thrift_framed_transport.c ---------------------------------------------------------------------- diff --git a/lib/c_glib/src/thrift/c_glib/transport/thrift_framed_transport.c b/lib/c_glib/src/thrift/c_glib/transport/thrift_framed_transport.c index a58b875..9810aa6 100644 --- a/lib/c_glib/src/thrift/c_glib/transport/thrift_framed_transport.c +++ b/lib/c_glib/src/thrift/c_glib/transport/thrift_framed_transport.c @@ -71,25 +71,32 @@ thrift_framed_transport_read_frame (ThriftTransport *transport, GError **error) { ThriftFramedTransport *t = THRIFT_FRAMED_TRANSPORT (transport); - gint32 sz, bytes; + guint32 sz; + gint32 bytes; + gboolean result = FALSE; /* read the size */ - THRIFT_TRANSPORT_GET_CLASS (t->transport)->read (t->transport, - (guint32 *) &sz, - sizeof (sz), error); - sz = ntohl (sz); + if (thrift_transport_read (t->transport, + &sz, + sizeof (sz), + error) == sizeof (sz)) + { + sz = ntohl (sz); - /* create a buffer to hold the data and read that much data */ - guchar tmpdata[sz]; - bytes = THRIFT_TRANSPORT_GET_CLASS (t->transport)->read (t->transport, - tmpdata, - sz, - error); + /* create a buffer to hold the data and read that much data */ + guchar tmpdata[sz]; + bytes = thrift_transport_read (t->transport, tmpdata, sz, error); - /* add the data to the buffer */ - g_byte_array_append (t->r_buf, tmpdata, bytes); + if (bytes > 0 && (error == NULL || *error == NULL)) + { + /* add the data to the buffer */ + g_byte_array_append (t->r_buf, tmpdata, bytes); - return TRUE; + result = TRUE; + } + } + + return result; } /* the actual read is "slow" because it calls the underlying transport */ @@ -100,6 +107,7 @@ thrift_framed_transport_read_slow (ThriftTransport *transport, gpointer buf, ThriftFramedTransport *t = THRIFT_FRAMED_TRANSPORT (transport); guint32 want = len; guint32 have = t->r_buf->len; + gint32 result = -1; // we shouldn't hit this unless the buffer doesn't have enough to read assert (t->r_buf->len < want); @@ -113,17 +121,20 @@ thrift_framed_transport_read_slow (ThriftTransport *transport, gpointer buf, } // read a frame of input and buffer it - thrift_framed_transport_read_frame (transport, error); + if (thrift_framed_transport_read_frame (transport, error) == TRUE) + { + // hand over what we have up to what the caller wants + guint32 give = want < t->r_buf->len ? want : t->r_buf->len; - // hand over what we have up to what the caller wants - guint32 give = want < t->r_buf->len ? want : t->r_buf->len; + // copy the data into the buffer + memcpy (buf + len - want, t->r_buf->data, give); + t->r_buf = g_byte_array_remove_range (t->r_buf, 0, give); + want -= give; - // copy the data into the buffer - memcpy (buf + len - want, t->r_buf->data, give); - t->r_buf = g_byte_array_remove_range (t->r_buf, 0, give); - want -= give; + result = len - want; + } - return (len - want); + return result; } /* implements thrift_transport_read */ http://git-wip-us.apache.org/repos/asf/thrift/blob/7fa9848b/lib/c_glib/test/testframedtransport.c ---------------------------------------------------------------------- diff --git a/lib/c_glib/test/testframedtransport.c b/lib/c_glib/test/testframedtransport.c index 03c9f9b..843ad93 100755 --- a/lib/c_glib/test/testframedtransport.c +++ b/lib/c_glib/test/testframedtransport.c @@ -144,6 +144,92 @@ test_read_and_write(void) } } +/* test reading from the transport after the peer has unexpectedly + closed the connection */ +static void +test_read_after_peer_close(void) +{ + int status; + pid_t pid; + int port = 51199; + GError *err = NULL; + + pid = fork (); + g_assert (pid >= 0); + + if (pid == 0) + { + ThriftServerTransport *server_transport = NULL; + ThriftTransport *client_transport = NULL; + + /* child listens */ + server_transport = g_object_new (THRIFT_TYPE_SERVER_SOCKET, + "port", port, + NULL); + g_assert (server_transport != NULL); + + thrift_server_transport_listen (server_transport, &err); + g_assert (err == NULL); + + /* wrap the client transport in a ThriftFramedTransport */ + client_transport = g_object_new + (THRIFT_TYPE_FRAMED_TRANSPORT, + "transport", thrift_server_transport_accept (server_transport, &err), + "r_buf_size", 0, + NULL); + g_assert (err == NULL); + g_assert (client_transport != NULL); + + /* close the connection immediately after the client connects */ + thrift_transport_close (client_transport, NULL); + + g_object_unref (client_transport); + g_object_unref (server_transport); + + exit (0); + } else { + ThriftSocket *tsocket = NULL; + ThriftTransport *transport = NULL; + guchar buf[10]; /* a buffer */ + + /* parent connects, wait a bit for the socket to be created */ + sleep (1); + + tsocket = g_object_new (THRIFT_TYPE_SOCKET, + "hostname", "localhost", + "port", port, + NULL); + transport = g_object_new (THRIFT_TYPE_FRAMED_TRANSPORT, + "transport", THRIFT_TRANSPORT (tsocket), + "w_buf_size", 0, + NULL); + + g_assert (thrift_transport_open (transport, NULL) == TRUE); + g_assert (thrift_transport_is_open (transport)); + + /* attempting to read from the transport after the peer has closed + the connection fails gracefully without generating a critical + warning or segmentation fault */ + thrift_transport_read (transport, buf, 10, &err); + g_assert (err != NULL); + + g_error_free (err); + err = NULL; + + thrift_transport_read_end (transport, &err); + g_assert (err == NULL); + + thrift_transport_close (transport, &err); + g_assert (err == NULL); + + g_object_unref (transport); + g_object_unref (tsocket); + + g_assert (wait (&status) == pid); + g_assert (status == 0); + } +} + static void thrift_server (const int port) { @@ -191,6 +277,7 @@ main(int argc, char *argv[]) g_test_add_func ("/testframedtransport/CreateAndDestroy", test_create_and_destroy); g_test_add_func ("/testframedtransport/OpenAndClose", test_open_and_close); g_test_add_func ("/testframedtransport/ReadAndWrite", test_read_and_write); + g_test_add_func ("/testframedtransport/ReadAfterPeerClose", test_read_after_peer_close); return g_test_run (); }
