[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [serf-dev] Re: [PATCH] Serf crash on spurious data between responses

From: Philip Martin <philip.martin_at_wandisco.com>
Date: Wed, 14 Jan 2015 11:54:39 +0000

Lieven Govaerts <lgo_at_mobsol.be> writes:

> The easiest way is to use existing tests as a starting point.:
> - Test test_serf_connection_request_create sends 2 requests and responses.
> This tests sends two pipelined requests, but you can modifiy it to
> send the 1st request, then run the serf loop, then send the 2nd
> request.
> - To add the extra data to the response of the first request, you'll
> have to create the raw response body.
> Have a look at test test_request_timeout, line:
> Respond(WithRawData(RESPONSE_408, strlen(RESPONSE_408)))

I tried to do this but I cannot use the testsuite to trigger the
problem. Using the earlier script to trigger the problem in svn I see
the following order of calls:

Breakpoint 4, serf_connection_request_create (conn=0x465590,
    setup=0x7ffff75ec67a <setup_request_cb>, setup_baton=0x469be0)
    at outgoing.c:1728
1728 request = create_request(conn, setup, setup_baton,
(gdb) c
Continuing.

Breakpoint 1, setup_request (request=0x46a308) at outgoing.c:776
776 serf_connection_t *conn = request->conn;
(gdb)
Continuing.

Breakpoint 2, read_from_connection (conn=0x465590) at outgoing.c:1142
1142 int close_connection = FALSE;
(gdb)
Continuing.

Breakpoint 3, destroy_request (request=0x46a308) at outgoing.c:544
544 serf_connection_t *conn = request->conn;
(gdb)
Continuing.

Breakpoint 4, serf_connection_request_create (conn=0x465590,
    setup=0x7ffff75ec67a <setup_request_cb>, setup_baton=0x469be0)
    at outgoing.c:1728
1728 request = create_request(conn, setup, setup_baton,
(gdb)
Continuing.

Breakpoint 2, read_from_connection (conn=0x465590) at outgoing.c:1142
1142 int close_connection = FALSE;

That is: create, setup, read, destroy for the first request leaving
spurious data on the socket, followed by create, read for the second
request. It's the call to read before setup that is the problem and
that doesn't happen with the test below. I think the reason is that in
the test both requests are created at the start, rather than creating
the second request after handling the first. The test needs to separate
the two requests so that one gets handled before the second is created.
I'm not sure how best to do that.

Index: test/test_context.c
===================================================================
--- test/test_context.c (revision 2472)
+++ test/test_context.c (working copy)
@@ -968,6 +968,43 @@ static void test_max_keepalive_requests(CuTest *tc
     CuAssertIntEquals(tc, num_requests, tb->handled_requests->nelts);
 }
 
+#define RESPONSE_200_SPURIOUS "HTTP/1.1 200 OK" CRLF\
+"Date: Tue, 13 Jan 2015 22:06:51 GMT" CRLF\
+"Server: Apache/2.2.22" CRLF\
+"Content-Length: 27" CRLF\
+"Content-Type: text/html; charset=UTF-8" CRLF \
+CRLF\
+"<html><body></body></html> "
+
+static void test_spurious_data(CuTest *tc)
+{
+ test_baton_t *tb = tc->testBaton;
+ apr_status_t status;
+ handler_baton_t handler_ctx[2];
+ const int num_requests = sizeof(handler_ctx)/sizeof(handler_ctx[0]);
+
+ setup_test_mock_server(tb);
+ status = setup_test_client_context(tb, NULL, tb->pool);
+ CuAssertIntEquals(tc, APR_SUCCESS, status);
+
+ Given(tb->mh)
+ GETRequest(URLEqualTo("/"), ChunkedBodyEqualTo("1"),
+ HeaderEqualTo("Host", tb->serv_host))
+ Respond(WithRawData(RESPONSE_200_SPURIOUS,
+ strlen(RESPONSE_200_SPURIOUS)))
+ GETRequest(URLEqualTo("/"), ChunkedBodyEqualTo("2"),
+ HeaderEqualTo("Host", tb->serv_host))
+ Respond(WithCode(200), WithChunkedBody(""))
+ EndGiven
+
+ create_new_request(tb, &handler_ctx[0], "GET", "/", 1);
+ create_new_request(tb, &handler_ctx[1], "GET", "/", 2);
+ serf_connection_set_max_outstanding_requests(tb->connection, 1);
+
+ run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
+ handler_ctx, tb->pool);
+}
+
 /*****************************************************************************/
 CuSuite *test_context(void)
 {
@@ -975,6 +1012,7 @@ CuSuite *test_context(void)
 
     CuSuiteSetSetupTeardownCallbacks(suite, test_setup, test_teardown);
 
+#if 0
     SUITE_ADD_TEST(suite, test_serf_connection_request_create);
     SUITE_ADD_TEST(suite, test_serf_connection_priority_request_create);
     SUITE_ADD_TEST(suite, test_closed_connection);
@@ -993,6 +1031,9 @@ CuSuite *test_context(void)
     SUITE_ADD_TEST(suite, test_connection_large_response);
     SUITE_ADD_TEST(suite, test_connection_large_request);
     SUITE_ADD_TEST(suite, test_max_keepalive_requests);
+#else
+ SUITE_ADD_TEST(suite, test_spurious_data);
+#endif
 
     return suite;
 }

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*
Received on 2015-01-14 12:57:22 CET

This is an archived mail posted to the Subversion Dev mailing list.