On Wed, Jan 14, 2015 at 12:54 PM, Philip Martin
<philip.mar...@wandisco.com> wrote:
> Lieven Govaerts <l...@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);
> +}

Can't test now, but I expect it should be something like this:

    create_new_request(tb, &handler_ctx[0], "GET", "/", 1);
    run_client_and_mock_servers_loops_expect_ok(tc, tb, 1,
handler_ctx, tb->pool);

    create_new_request(tb, &handler_ctx[1], "GET", "/", 2);
    run_client_and_mock_servers_loops_expect_ok(tc, tb, 1,
handler_ctx, tb->pool);

I'll have more time this weekend to have a look.

Lieven

> +
>  
> /*****************************************************************************/
>  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*

Reply via email to