On Sun, Jun 30, 2013 at 7:37 PM, <[email protected]> wrote:
>> On Tue, Jun 25, 2013 at 3:11 PM, <[email protected]> wrote:
>> > > I agree that force-http10 is not good name and semantic. Actually
>> > > these proxies is not busted: it's allowed to HTTP/1.1 proxies to
>> > > require content-length if they want. And strictly speaking proxies may
>> > > have different behavior for different requests.
>> >
>> > From *our* standpoint, they are busted. Subversion wants to use
>> > chunked requests. If they don't support it, then they are busted.
>> > Simple as that.
>> >
>> > And we want to use a provocative name so that people understand
>> > something needs to be *fixed*. Fixed for us because we view them as
>> > *busted*.
>
>> From the *users* standpoint subversion is busted. Something that
>>
>> I'm not seeing the point. Subversion will (now) work, but we still
>> view the proxy as busted. It doesn't provide the performance
>> characteristics that Subversion wants and expects. Note that
>> Subversion is built to work against mod_dav_svn which is HTTP/1.1
>> with chunked requests. The interposition of a proxy that disables
>> chunked requests... busted.
>
> Yes you missed my point. Users don't care why something is
> broken, just that it is and that they now have to perform some
> manual operation to get it to work again. Score a -1 for svn user
> happiness. No reason to punish an user for something that is
> most likely outside of their control.
>
>> worked fine in 1.7 does not work in 1.8 without modifying potentially
>> unrelated things that neither the server admin or the client
>> user may have control over. (Think proxy at a hotel, etc.)
>>
>> Of course. But we can fix this. (and I believe, have fixed it)
>>
>> The spec states that 411 is an allowed response and is it also states
>> the client should prefer to not use chunked requests if possible.
>> Serf is being overly optimistic here.
>>
>> "Prefer" is not the same as "must" :-)
>
> True, but it is there for the exact reason we are arguing about :)
> (That clients which ignore this advice will not work correctly
> in a lot of real-world situations.)
>
>> In our current model, we prefer chunked. But there is a pretty
>> straightforward extension to serf's bucket model that should allow
>> us to use C-L in many situations. We *might* be able to do that in a
>> serf 1.x, but I'm not sure. Worst case, we'll add the feature in serf 2.x.
>
> I completely agree this is the preferred solution.
>
Serf-trunk now has serf_bucket_get_remaining() to get length of
request body since r2008. Attached patch enables this functionality in
ra_serf. Within this patch Content-Length will be send for every
request for no cost and should make proxies (and servers) happy.
--
Ivan Zhakov
Index: subversion/libsvn_ra_serf/util.c
===================================================================
--- subversion/libsvn_ra_serf/util.c (revision 1500021)
+++ subversion/libsvn_ra_serf/util.c (working copy)
@@ -640,12 +640,17 @@
apr_pool_t *scratch_pool)
{
serf_bucket_alloc_t *allocator = serf_request_get_alloc(request);
-
- svn_spillbuf_t *buf;
svn_boolean_t set_CL = session->http10 || !session->using_chunked_requests;
+ apr_uint64_t request_body_len;
- if (set_CL && body_bkt != NULL)
+ if (body_bkt)
+ request_body_len = serf_bucket_get_remaining(body_bkt);
+ else
+ request_body_len = 0;
+
+ if (set_CL && request_body_len == SERF_LENGTH_UNKNOWN)
{
+ svn_spillbuf_t *buf;
/* Ugh. Use HTTP/1.0 to talk to the server because we don't know if
it speaks HTTP/1.1 (and thus, chunked requests), or because the
server actually responded as only supporting HTTP/1.0.
@@ -663,6 +668,7 @@
body_bkt = svn_ra_serf__create_sb_bucket(buf, allocator,
request_pool,
scratch_pool);
+ request_body_len = svn_spillbuf__get_size(buf);
}
/* Create a request bucket. Note that this sucker is kind enough to
@@ -672,12 +678,9 @@
/* Set the Content-Length value. This will also trigger an HTTP/1.0
request (rather than the default chunked request). */
- if (set_CL)
+ if (request_body_len != SERF_LENGTH_UNKNOWN)
{
- if (body_bkt == NULL)
- serf_bucket_request_set_CL(*req_bkt, 0);
- else
- serf_bucket_request_set_CL(*req_bkt, svn_spillbuf__get_size(buf));
+ serf_bucket_request_set_CL(*req_bkt, request_body_len);
}
*hdrs_bkt = serf_bucket_request_get_headers(*req_bkt);
