You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2013/07/10 07:59:52 UTC
svn commit: r1501654 - in
/subversion/branches/tristate-chunked-request/subversion:
include/svn_config.h libsvn_ra_serf/options.c libsvn_ra_serf/ra_serf.h
libsvn_ra_serf/serf.c libsvn_ra_serf/util.c libsvn_subr/config_file.c
Author: breser
Date: Wed Jul 10 05:59:51 2013
New Revision: 1501654
URL: http://svn.apache.org/r1501654
Log:
Replace busted-proxy config knob with a tristate http-chunked-requests.
* subversion/include/svn_config.h
(SVN_CONFIG_OPTION_BUSTED_PROXY): Replaced by ...
(SVN_CONFIG_OPTION_HTTP_CHUNKED_REQUESTS): New.
* subversion/libsvn_ra_serf/options.c
(svn_ra_serf__probe_proxy): Sets a boolean specifying if the server
supports chunked requests, rather than modifying the session directly.
* subversion/libsvn_ra_serf/ra_serf.h
(svn_ra_serf__session_t): Replace using_chunked_requests and busted_proxy
members with set_CL and using_chunked_requests members respectively.
The new using_chunked_requests member is now a tristate instead of boolean.
(svn_ra_serf__probe_proxy): Update definition to match implementation.
* subversion/libsvn_ra_serf/serf.c
(load_config): Update to use httpd-chunked-requests instaed of busted-proxy.
(svn_ra_serf__open): Switch to using the tristate config.
* subversion/libsvn_ra_serf/util.c
(setup_serf_req): Just use set_CL value on the session.
(svn_ra_serf__error_on_status): Adjust 411 error message.
* subversion/libsvn_subr/config_file.c
(svn_config_ensure): Update default servers with new config option.
Patch by: ivan
(tweaked by me)
Modified:
subversion/branches/tristate-chunked-request/subversion/include/svn_config.h
subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/options.c
subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/ra_serf.h
subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/serf.c
subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/util.c
subversion/branches/tristate-chunked-request/subversion/libsvn_subr/config_file.c
Modified: subversion/branches/tristate-chunked-request/subversion/include/svn_config.h
URL: http://svn.apache.org/viewvc/subversion/branches/tristate-chunked-request/subversion/include/svn_config.h?rev=1501654&r1=1501653&r2=1501654&view=diff
==============================================================================
--- subversion/branches/tristate-chunked-request/subversion/include/svn_config.h (original)
+++ subversion/branches/tristate-chunked-request/subversion/include/svn_config.h Wed Jul 10 05:59:51 2013
@@ -96,7 +96,7 @@ typedef struct svn_config_t svn_config_t
/** @since New in 1.8. */
#define SVN_CONFIG_OPTION_HTTP_MAX_CONNECTIONS "http-max-connections"
/** @since New in 1.9. */
-#define SVN_CONFIG_OPTION_BUSTED_PROXY "busted-proxy"
+#define SVN_CONFIG_OPTION_HTTP_CHUNKED_REQUESTS "http-chunked-requests"
#define SVN_CONFIG_CATEGORY_CONFIG "config"
#define SVN_CONFIG_SECTION_AUTH "auth"
Modified: subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/options.c
URL: http://svn.apache.org/viewvc/subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/options.c?rev=1501654&r1=1501653&r2=1501654&view=diff
==============================================================================
--- subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/options.c (original)
+++ subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/options.c Wed Jul 10 05:59:51 2013
@@ -541,10 +541,13 @@ create_simple_options_body(serf_bucket_t
svn_error_t *
-svn_ra_serf__probe_proxy(svn_ra_serf__session_t *serf_sess,
+svn_ra_serf__probe_proxy(svn_boolean_t *supports_chunked,
+ svn_ra_serf__session_t *serf_sess,
apr_pool_t *scratch_pool)
{
svn_ra_serf__handler_t *handler;
+ svn_error_t *err;
+ svn_boolean_t set_CL;
handler = apr_pcalloc(scratch_pool, sizeof(*handler));
handler->handler_pool = scratch_pool;
@@ -561,13 +564,23 @@ svn_ra_serf__probe_proxy(svn_ra_serf__se
/* No special headers. */
- SVN_ERR(svn_ra_serf__context_run_one(handler, scratch_pool));
+ /* Disable sending Content-Length for this specific request to check server
+ support for chunked Transfer-Encoding. */
+ set_CL = serf_sess->set_CL;
+ serf_sess->set_CL = FALSE;
+ err = svn_ra_serf__context_run_one(handler, scratch_pool);
+
+ /* Revert original value back. */
+ serf_sess->set_CL = set_CL;
+
+ SVN_ERR(err);
+
/* Some versions of nginx in reverse proxy mode will return 411. They want
a Content-Length header, rather than chunked requests. We can keep other
HTTP/1.1 features, but will disable the chunking. */
if (handler->sline.code == 411)
{
- serf_sess->using_chunked_requests = FALSE;
+ *supports_chunked = FALSE;
return SVN_NO_ERROR;
}
@@ -575,6 +588,7 @@ svn_ra_serf__probe_proxy(svn_ra_serf__se
handler->path,
handler->location));
+ *supports_chunked = TRUE;
return SVN_NO_ERROR;
}
Modified: subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/ra_serf.h?rev=1501654&r1=1501653&r2=1501654&view=diff
==============================================================================
--- subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/ra_serf.h Wed Jul 10 05:59:51 2013
@@ -144,8 +144,8 @@ struct svn_ra_serf__session_t {
HTTP/1.0. Thus, we cannot send chunked requests. */
svn_boolean_t http10;
- /* Should we use Transfer-Encoding: chunked for HTTP/1.1 servers. */
- svn_boolean_t using_chunked_requests;
+ /* Should we use send Content-Length for HTTP requests. */
+ svn_boolean_t set_CL;
/* Our Version-Controlled-Configuration; may be NULL until we know it. */
const char *vcc_url;
@@ -191,9 +191,8 @@ struct svn_ra_serf__session_t {
/* Are we using a proxy? */
svn_boolean_t using_proxy;
- /* Should we be careful with this proxy? (some have insufficient support that
- we need to work around). */
- svn_boolean_t busted_proxy;
+ /* Should we use Transfer-Encoding: chunked for HTTP/1.1 servers. */
+ svn_tristate_t using_chunked_requests;
const char *proxy_username;
const char *proxy_password;
@@ -1366,9 +1365,11 @@ svn_ra_serf__run_merge(const svn_commit_
/* When running with a proxy, we may need to detect and correct for problems.
This probing function will send a simple OPTIONS request to detect problems
- with the connection. */
+ with the connection. Sets *SUPPORTS_CHUNKED to non-zero if server supports
+ chunked Transfer-Encoding and zero otherwise. */
svn_error_t *
-svn_ra_serf__probe_proxy(svn_ra_serf__session_t *serf_sess,
+svn_ra_serf__probe_proxy(svn_boolean_t *supports_chunked,
+ svn_ra_serf__session_t *serf_sess,
apr_pool_t *scratch_pool);
Modified: subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/serf.c
URL: http://svn.apache.org/viewvc/subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/serf.c?rev=1501654&r1=1501653&r2=1501654&view=diff
==============================================================================
--- subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/serf.c (original)
+++ subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/serf.c Wed Jul 10 05:59:51 2013
@@ -225,11 +225,12 @@ load_config(svn_ra_serf__session_t *sess
SVN_CONFIG_OPTION_HTTP_MAX_CONNECTIONS,
SVN_CONFIG_DEFAULT_OPTION_HTTP_MAX_CONNECTIONS));
- /* Is this proxy potentially busted? Do we need to take special care? */
- SVN_ERR(svn_config_get_bool(config, &session->busted_proxy,
- SVN_CONFIG_SECTION_GLOBAL,
- SVN_CONFIG_OPTION_BUSTED_PROXY,
- FALSE));
+ /* Should we use chunked transfer encopding. */
+ SVN_ERR(svn_config_get_tristate(config, &session->using_chunked_requests,
+ SVN_CONFIG_SECTION_GLOBAL,
+ SVN_CONFIG_OPTION_HTTP_CHUNKED_REQUESTS,
+ "auto",
+ svn_tristate_unknown));
if (config)
server_group = svn_config_find_group(config,
@@ -288,12 +289,13 @@ load_config(svn_ra_serf__session_t *sess
SVN_CONFIG_OPTION_HTTP_MAX_CONNECTIONS,
session->max_connections));
- /* Do we need to take care with this proxy? */
- SVN_ERR(svn_config_get_bool(
- config, &session->busted_proxy,
+ /* Should we use chunked transfer encopding. */
+ SVN_ERR(svn_config_get_tristate(
+ config, &session->using_chunked_requests,
server_group,
- SVN_CONFIG_OPTION_BUSTED_PROXY,
- session->busted_proxy));
+ SVN_CONFIG_OPTION_HTTP_CHUNKED_REQUESTS,
+ "auto",
+ session->using_chunked_requests));
}
/* Don't allow the http-max-connections value to be larger than our
@@ -455,9 +457,10 @@ svn_ra_serf__open(svn_ra_session_t *sess
HTTP/1.1 is supported, we can upgrade. */
serf_sess->http10 = TRUE;
- /* If we switch to HTTP/1.1, then we will use chunked requests. We may disable
- this, if we find an intervening proxy does not support chunked requests. */
- serf_sess->using_chunked_requests = TRUE;
+ /* If we switch to HTTP/1.1, then we will use chunked requests instead of
+ * Content-Length. We may switch to Content-Length, if we find an intervening
+ * proxy does not support chunked requests. */
+ serf_sess->set_CL = TRUE;
SVN_ERR(load_config(serf_sess, config, serf_sess->pool));
@@ -505,13 +508,39 @@ svn_ra_serf__open(svn_ra_session_t *sess
_("Connection to '%s' failed"), session_URL);
SVN_ERR(err);
- /* We have set up a useful connection (that doesn't indication a redirect).
- If we've been told there is possibly a busted proxy in our path to the
- server AND we switched to HTTP/1.1 (chunked requests), then probe for
- problems in any proxy. */
- if ((corrected_url == NULL || *corrected_url == NULL)
- && serf_sess->busted_proxy && !serf_sess->http10)
- SVN_ERR(svn_ra_serf__probe_proxy(serf_sess, pool));
+ /* Early exit if we got the redirection. */
+ if (corrected_url && *corrected_url)
+ return SVN_NO_ERROR;
+
+ /* Determine if we're goign to use Content-Length or chunked
+ * Transfer-Encoding with a HTTP/1.1 server. HTTP/1.0 servers
+ * always use Content-Length. */
+ if (!serf_sess->http10)
+ {
+ if (serf_sess->using_chunked_requests == svn_tristate_true)
+ {
+ /* Upgrade to chunked Transfer-Encoding. */
+ serf_sess->set_CL = FALSE;
+ }
+ else if (serf_sess->using_chunked_requests == svn_tristate_false)
+ {
+ /* Disable chunked Transfer-Encoding without probing. */
+ serf_sess->set_CL = TRUE;
+ }
+ else if (serf_sess->using_chunked_requests == svn_tristate_unknown)
+ {
+ svn_boolean_t supports_chunked;
+ /* We have set up a useful connection (that doesn't indication a
+ redirect). And user didn't specify preference whether to use
+ chunked requests or not. Then probe for chunked request encoding
+ support in any proxy/server. */
+ SVN_ERR(svn_ra_serf__probe_proxy(&supports_chunked, serf_sess, pool));
+
+ /* Upgrade to chunked Transfer-Encoding after probing. */
+ if (supports_chunked)
+ serf_sess->set_CL = FALSE;
+ }
+ }
return SVN_NO_ERROR;
}
Modified: subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/util.c?rev=1501654&r1=1501653&r2=1501654&view=diff
==============================================================================
--- subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/branches/tristate-chunked-request/subversion/libsvn_ra_serf/util.c Wed Jul 10 05:59:51 2013
@@ -640,11 +640,9 @@ setup_serf_req(serf_request_t *request,
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;
- if (set_CL && body_bkt != NULL)
+ if (session->set_CL && 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
@@ -672,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 (set_CL)
+ if (session->set_CL)
{
if (body_bkt == NULL)
serf_bucket_request_set_CL(*req_bkt, 0);
@@ -2444,8 +2442,9 @@ svn_ra_serf__error_on_status(serf_status
return svn_error_createf(SVN_ERR_RA_DAV_REQUEST_FAILED, NULL,
_("DAV request failed: 411 Content length required. The "
"server or an intermediate proxy does not accept "
- "chunked encoding. Try setting 'busted-proxy=yes' "
- "in your client configuration."));
+ "chunked encoding. Try setting "
+ "http-chunked-encoding=auto or no in your client "
+ "configuration."));
}
if (sline.code >= 300)
Modified: subversion/branches/tristate-chunked-request/subversion/libsvn_subr/config_file.c
URL: http://svn.apache.org/viewvc/subversion/branches/tristate-chunked-request/subversion/libsvn_subr/config_file.c?rev=1501654&r1=1501653&r2=1501654&view=diff
==============================================================================
--- subversion/branches/tristate-chunked-request/subversion/libsvn_subr/config_file.c (original)
+++ subversion/branches/tristate-chunked-request/subversion/libsvn_subr/config_file.c Wed Jul 10 05:59:51 2013
@@ -838,9 +838,8 @@ svn_config_ensure(const char *config_dir
"### http-max-connections Maximum number of parallel server" NL
"### connections to use for any given" NL
"### HTTP operation." NL
- "### busted-proxy The proxy may have some protocol" NL
- "### issues that Subversion needs to" NL
- "### detect and work around." NL
+ "### http-chunked-requests Whether to use chunked transfer" NL
+ "### encoding for HTTP requests body." NL
"### neon-debug-mask Debug mask for Neon HTTP library" NL
"### ssl-authority-files List of files, each of a trusted CA"
NL
Re: svn commit: r1501654 - in /subversion/branches/tristate-chunked-request/subversion:
include/svn_config.h libsvn_ra_serf/options.c libsvn_ra_serf/ra_serf.h
libsvn_ra_serf/serf.c libsvn_ra_serf/util.c libsvn_subr/config_file.c
Posted by Ben Reser <be...@reser.org>.
On Wed, Jul 10, 2013 at 9:33 PM, Greg Stein <gs...@gmail.com> wrote:
> I don't think you want to do it this way.
>
> * keep the (now-renamed) "detect_chunking" flag separate, and in the
> session. it may be "local" to open() right now, but in the future it
> may be used as part of an operation's "first request"
>
> * detect_chunking remains a boolean, and it can be set to FALSE when
> the first probe is done (whether that is OPTIONS or a REPORT or...)
>
> * don't alter the probe or the request logic. it is fine. consider:
> - "auto": set detect_chunking, set using_chunked_requests
> - "yes": clear detect_chunking, set using_chunked_requests
> - "no": clear detect_chunking, clear using_chunked_requests
> In other words, you can get away with a simple change to serf.c for
> the "auto" work
Deleted the branch and redid it as described above. I do like this
implementation better.
See r1502401.
Re: svn commit: r1501654 - in /subversion/branches/tristate-chunked-request/subversion:
include/svn_config.h libsvn_ra_serf/options.c libsvn_ra_serf/ra_serf.h
libsvn_ra_serf/serf.c libsvn_ra_serf/util.c libsvn_subr/config_file.c
Posted by Greg Stein <gs...@gmail.com>.
On Wed, Jul 10, 2013 at 1:59 AM, <br...@apache.org> wrote:
>...
> * subversion/libsvn_ra_serf/ra_serf.h
> (svn_ra_serf__session_t): Replace using_chunked_requests and busted_proxy
> members with set_CL and using_chunked_requests members respectively.
> The new using_chunked_requests member is now a tristate instead of boolean.
I don't think you want to do it this way.
* keep the (now-renamed) "detect_chunking" flag separate, and in the
session. it may be "local" to open() right now, but in the future it
may be used as part of an operation's "first request"
* detect_chunking remains a boolean, and it can be set to FALSE when
the first probe is done (whether that is OPTIONS or a REPORT or...)
* don't alter the probe or the request logic. it is fine. consider:
- "auto": set detect_chunking, set using_chunked_requests
- "yes": clear detect_chunking, set using_chunked_requests
- "no": clear detect_chunking, clear using_chunked_requests
In other words, you can get away with a simple change to serf.c for
the "auto" work
And sorry: my r1502097 change conflicts with your branch. But
considering how simple this change *should* be, I think you can just
toss the branch and suggest a one-file patch.
(to be clear: I'm still feeling about -0.7 on backporting an "auto"
patch to 1.8.x; my current feeling is that a knob/upgrade
recommendation is sufficient, and we improve the situation in the
1.9.x series)
Cheers,
-g