serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Zhakov <i...@visualsvn.com>
Subject Re: svn commit: r1715431 - in /serf/trunk: buckets/response_buckets.c serf_bucket_types.h test/test_buckets.c
Date Sat, 21 Nov 2015 17:40:20 GMT
On 21 November 2015 at 19:55, Bert Huijben <bert@qqmail.nl> wrote:
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> Sent: zaterdag 21 november 2015 17:00
>> To: rhuijben@apache.org
>> Cc: dev@serf.apache.org
>> Subject: Re: svn commit: r1715431 - in /serf/trunk:
>> buckets/response_buckets.c serf_bucket_types.h test/test_buckets.c
>>
>> On 21 November 2015 at 01:09,  <rhuijben@apache.org> wrote:
>> > Author: rhuijben
>> > Date: Fri Nov 20 22:09:46 2015
>> > New Revision: 1715431
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1715431&view=rev
>> > Log:
>> > Allow a response to handle multiple sets of headers for a single request.
>> > This enables support for http statee like 100 'Continue'.
>> >
>> Hi Bert,
>>
>> Thanks for fixing this long-standing problem!
>>
>> >
>> > * buckets/response_buckets.c
>> >   (response_context_t): Hold two sets of headers.
>> >   (serf_bucket_response_create): Tweak init.
>> >   (serf_bucket_response_get_headers): Get the fetch headers.
>> >   (serf_response_destroy_and_data): Update cleanup.
>> >   (parse_status_line): Allow parsing another statusline.
>> >   (fetch_headers): Store new headers in incoming headers.
>> >   (run_machine): Handle new states.
>> >
>> >   (serf_bucket_response_wait_for_some_headers): New function.
>> > +apr_status_t serf_bucket_response_wait_for_some_headers(
>> > +    serf_bucket_t *bucket,
>> > +     int wait_for_next)
>>
>> I've two questions about this function:
>> - What is the purpose of wait_for_next parameter?
>> - May be better API would be
>> serf_bucket_response_for_headers2(serf_bucket_t *bucket, int
>> ignore_interim_status) ? I
>
> I intend to change that a bit later (probably tomorrow or Monday). I think we
> need to split this in two functions.
>
OK, I'll wait for further commits. But currently I think it would be
better to have serf_bucket_response_for_headers2() and deprecate
serf_bucket_response_for_headers() to encourage API users to revise
they usage of this function in terms whether they want to handle
interim responses or not.

> Currently with FALSE it waits until there is some set of headers available. If there
is
> one it returns directly.
>
> While with TRUE it stops looking at the current set of headers if it is not the set of
> final headers... and then waits for the next set of headers. (final or not)
>


>
> I intend to replace the current auth code that currently uses +- wait_for_headers() and
> only then starts forwarding to the normal response code, to use a similar approach...
But
> then replace the stream below the response bucket with that of the new response after
> sending the request with the right headers.
>
Yes, current auth code will mask interim responses since it waits for
final headers.

> This code already assumes that the bucket is really a response bucket and can do the
> right thing. (I think we should check this condition though... and don't apply auth
> functions (and 99% chance segfault) if it is not a normal response bucket)
>
>
> And when we do that... we can also use 100% this same approach for sending a
> HTTP/1 request, upgrade to http/2 (first 101 header-set) ... and handle the final set
> of headers+body as HTTP/2.
>
I'm not sure that I'm fully understand you plan, but I'm very
interested to see this code :)

-- 
Ivan Zhakov

Mime
View raw message