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

Re: svn commit: r1495419 - in /subversion/trunk/subversion/libsvn_ra_serf: options.c ra_serf.h serf.c util.c

From: Greg Stein <gstein_at_gmail.com>
Date: Fri, 21 Jun 2013 09:19:38 -0400

Please revert this. You *cannot* rely on a 411 response.

You are also not allowed to send a 1.1 request to a server for which
you don't know if they support 1.0 vs 1.1.

Sigh. Seriously, man. This is completely counter to the work that was
done. We *start* with 1.0, and then we upgrade to 1.1 only when the
server identifies that it can handle it. Not to mention the
complications of intervening proxies. You're making a mess of that
work.

-g

On Fri, Jun 21, 2013 at 8:47 AM, <ivan_at_apache.org> wrote:
> Author: ivan
> Date: Fri Jun 21 12:47:46 2013
> New Revision: 1495419
>
> URL: http://svn.apache.org/r1495419
> Log:
> Rework server chunked transfer encoding support detection in ra_serf. Use
> HTTP/1.1 chunked transfer encoding for first OPTIONS request and then
> fallback to HTTP/1.0 requests if HTTP 411 status code or HTTP/1.0 response
> received.
>
> Discussion and report: http://svn.haxx.se/dev/archive-2013-06/0408.shtml
>
> * subversion/libsvn_ra_serf/ra_serf.h
> (svn_ra_serf__session_t): Remove HTTP10 and add USE_CHUNKED_ENCODING.
> * subversion/libsvn_ra_serf/options.c
> (svn_ra_serf__exchange_capabilities): Retry OPTIONS request without using
> chunked transfer encoding if HTTP 411 status code or HTTP/1.0 response
> received for chunked request.
> * subversion/libsvn_ra_serf/serf.c
> (svn_ra_serf__open): Initialize USE_CHUNKED_ENCODING to TRUE.
> * subversion/libsvn_ra_serf/util.c
> (setup_serf_req): Use USE_CHUNKED_ENCODING member instead of removed
> HTTP10. Always send 'Connection: keep-alive' header.
>
> Modified:
> subversion/trunk/subversion/libsvn_ra_serf/options.c
> subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
> subversion/trunk/subversion/libsvn_ra_serf/serf.c
> subversion/trunk/subversion/libsvn_ra_serf/util.c
>
> Modified: subversion/trunk/subversion/libsvn_ra_serf/options.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/options.c?rev=1495419&r1=1495418&r2=1495419&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/options.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/options.c Fri Jun 21 12:47:46 2013
> @@ -488,6 +488,22 @@ svn_ra_serf__exchange_capabilities(svn_r
>
> err = svn_ra_serf__context_run_one(opt_ctx->handler, pool);
>
> + /* Retry request if HTTP/1.1 411 Length Required or we got HTTP/1.0 response. */
> + if (serf_sess->use_chunked_encoding &&
> + (opt_ctx->handler->sline.code == 411 ||
> + opt_ctx->handler->sline.version == SERF_HTTP_10))
> + {
> + /* Ignore any errors and retry request using HTTP/1.0 with
> + Content-Length.*/
> + svn_error_clear(err);
> +
> + serf_sess->use_chunked_encoding = FALSE;
> +
> + SVN_ERR(create_options_req(&opt_ctx, serf_sess, serf_sess->conns[0], pool));
> +
> + err = svn_ra_serf__context_run_one(opt_ctx->handler, pool);
> + }
> +
> /* If our caller cares about server redirections, and our response
> carries such a thing, report as much. We'll disregard ERR --
> it's most likely just a complaint about the response body not
>
> Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1495419&r1=1495418&r2=1495419&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Fri Jun 21 12:47:46 2013
> @@ -140,9 +140,8 @@ struct svn_ra_serf__session_t {
> apr_uri_t repos_root;
> const char *repos_root_str;
>
> - /* The server is not Apache/mod_dav_svn (directly) and only supports
> - HTTP/1.0. Thus, we cannot send chunked requests. */
> - svn_boolean_t http10;
> + /* The server supports chunked request bodies. */
> + svn_boolean_t use_chunked_encoding;
>
> /* Our Version-Controlled-Configuration; may be NULL until we know it. */
> const char *vcc_url;
>
> Modified: subversion/trunk/subversion/libsvn_ra_serf/serf.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/serf.c?rev=1495419&r1=1495418&r2=1495419&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/serf.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/serf.c Fri Jun 21 12:47:46 2013
> @@ -437,9 +437,10 @@ svn_ra_serf__open(svn_ra_session_t *sess
>
> serf_sess->capabilities = apr_hash_make(serf_sess->pool);
>
> - /* We have to assume that the server only supports HTTP/1.0. Once it's clear
> - HTTP/1.1 is supported, we can upgrade. */
> - serf_sess->http10 = TRUE;
> + /* Assume HTTP/1.1 server and use chunked transfer encoding. We fallback
> + * to HTTP/1.0 requests in svn_ra_serf__exchange_capabilities() if server
> + * doesn't support chunked encoding. */
> + serf_sess->use_chunked_encoding = FALSE;
>
> SVN_ERR(load_config(serf_sess, config, serf_sess->pool));
>
>
> Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1495419&r1=1495418&r2=1495419&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/util.c Fri Jun 21 12:47:46 2013
> @@ -642,7 +642,7 @@ setup_serf_req(serf_request_t *request,
>
> svn_spillbuf_t *buf;
>
> - if (session->http10 && body_bkt != NULL)
> + if (!session->use_chunked_encoding && body_bkt != NULL)
> {
> /* 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
> @@ -670,7 +670,7 @@ setup_serf_req(serf_request_t *request,
>
> /* Set the Content-Length value. This will also trigger an HTTP/1.0
> request (rather than the default chunked request). */
> - if (session->http10)
> + if (!session->use_chunked_encoding)
> {
> if (body_bkt == NULL)
> serf_bucket_request_set_CL(*req_bkt, 0);
> @@ -690,10 +690,9 @@ setup_serf_req(serf_request_t *request,
> serf_bucket_headers_setn(*hdrs_bkt, "Content-Type", content_type);
> }
>
> - if (session->http10)
> - {
> - serf_bucket_headers_setn(*hdrs_bkt, "Connection", "keep-alive");
> - }
> + /* Always set Connection: keep-alive because we don't know if server
> + * is HTTP/1.1 aware. */
> + serf_bucket_headers_setn(*hdrs_bkt, "Connection", "keep-alive");
>
> if (accept_encoding)
> {
> @@ -1861,10 +1860,6 @@ handle_response(serf_request_t *request,
>
> handler->sline = sl;
> handler->sline.reason = apr_pstrdup(handler->handler_pool, sl.reason);
> -
> - /* HTTP/1.1? (or later) */
> - if (sl.version != SERF_HTTP_10)
> - handler->session->http10 = FALSE;
> }
>
> /* Keep reading from the network until we've read all the headers. */
>
>
Received on 2013-06-21 15:20:15 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.