Re: [PATCH 1/2] http: add option to enable 100 Continue responses

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Oct 10, 2013, 01:35 UTC
Message-ID: <20131010013547.GA62549@vauxhall.crustytoothpaste.net>
In-Reply-To: <20131009213742.GA8362@sigill.intra.peff.net>

On Wed, Oct 09, 2013 at 05:37:42PM -0400, Jeff King wrote:

Show 10 quoted lines

> On Wed, Oct 09, 2013 at 02:19:36PM -0700, Shawn O. Pearce wrote:
> > 206b099 was written because the Google web servers for
> > android.googlesource.com and code.google.com do not support
> > 100-continue semantics. This caused the client to stall a full 1
> > second before each POST exchange. If ancestor negotiation required
> > O(128) have lines to be advertised I think this was 2 or 4 POSTs,
> > resulting in 2-4 second stalls above the other latency of the network
> > and the server.
> 
> Yuck.

Shame on Google. Of all people, they should be able to implement HTTP 1.1 properly.

Show 12 quoted lines

> Part of the problem is that curl is the one handling the negotiation.
> When we get a 401, I think we can ask curl_easy_getinfo to tell us which
> auth methods are available (via CURLINFO_HTTPAUTH_AVAIL). But I don't
> know how we decide that GSS is what's going to be used. I guess if it is
> the only option, and there is no basic-auth offered?
> 
> And then in that case turn on "Expect" (or more accurately, stop
> disabling it).
> 
> I don't have a GSS-enabled server to test on. Brian, can you try the
> patch at the end of this message on your non-working server and see what
> it outputs?

It doesn't trigger. My server only requires authentication for the actual push operation, and it looks like your code doesn't trigger in that case.

Show 7 quoted lines

> > >> +     headers = curl_slist_append(headers, http_use_100_continue ?
> > >> +             "Expect: 100-continue" : "Expect:");
> > >
> > > Is there any point in sending the Expect: header in cases where curl
> > > would not send it, though? It seems like we should assume curl does the
> > > right thing most of the time, and have our option only be to override
> > > curl in the negative direction.

I think curl expects that data in the POSTFIELDS option in order to trigger. I wasn't able to get it to trigger on its own.

Show 10 quoted lines

> > Adding a header of "Expect:" causes curl to disable the header and
> > never use it. Always supplying the header with no value prevents
> > libcurl from using 100-continue on its own, which is what I fixed in
> > 959dfcf.
> 
> Right. What I meant is that we do not want to unconditionally send the
> "Expect: 100-continue" in the other case. IOW, we would just want:
> 
>   if (!http_use_100_continue)
>           headers = curl_slist_append(headers, "Expect:");

I tried that. It doesn't work, since it never sends the Expect: 100-continue. I made libcurl dump all the headers it sends, and it doesn't send them in that case.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187