On Sat, May 30, 2020 at 2:14 PM Yann Ylavic <[email protected]> wrote:
>
> Thoughts? (before I propose a patch..)
FWIW, the patch would look like the attached one.
Still with the aim of modernization, could we remove "proxy-sendextracrlf"?
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c (revision 1878280)
+++ modules/proxy/mod_proxy_http.c (working copy)
@@ -749,40 +749,24 @@ static int ap_proxy_http_prefetch(proxy_http_req_t
}
}
- /* Use chunked request body encoding or send a content-length body?
+ /* The request body can be streamed (using either content-length or
+ * chunked transfer-encoding), or spooled to memory/disk (MAX_MEM_SPOOL)
+ * then sent (thus using content-length with known length).
*
- * Prefer C-L when:
+ * We favor streaming when possible, that is:
*
- * We have no request body (handled by RB_STREAM_CL)
+ * The whole body (including empty/no body) was received on prefetch,
+ * i.e. input brigade contains EOS; handled by RB_STREAM_CL.
*
- * We have a request body length <= MAX_MEM_SPOOL
+ * There are only proto input filters, which means no filter should
+ * change the body length; handled by RB_STREAM_CL or RB_STREAM_CHUNKED
+ * according to respectively C-L or T-E received from the client.
*
- * The administrator has setenv force-proxy-request-1.0
+ * The administrator did not SetEnv proxy-no-chunked, which allows us
+ * to stream each input chunk using T-E; handled by RB_STREAM_CHUNKED.
*
- * The client sent a C-L body, and the administrator has
- * not setenv proxy-sendchunked or has set setenv proxy-sendcl
- *
- * The client sent a T-E body, and the administrator has
- * setenv proxy-sendcl, and not setenv proxy-sendchunked
- *
- * If both proxy-sendcl and proxy-sendchunked are set, the
- * behavior is the same as if neither were set, large bodies
- * that can't be read will be forwarded in their original
- * form of C-L, or T-E.
- *
- * To ensure maximum compatibility, setenv proxy-sendcl
- * To reduce server resource use, setenv proxy-sendchunked
- *
- * Then address specific servers with conditional setenv
- * options to restore the default behavior where desirable.
- *
- * We have to compute content length by reading the entire request
- * body; if request body is not small, we'll spool the remaining
- * input to a temporary file. Chunked is always preferable.
- *
- * We can only trust the client-provided C-L if the T-E header
- * is absent, and the filters are unchanged (the body won't
- * be resized by another content filter).
+ * Otherwise, we have to compute content length by spooling the entire
+ * request body; handled by RB_SPOOL_CL.
*/
if (!APR_BRIGADE_EMPTY(input_brigade)
&& APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) {
@@ -799,42 +783,23 @@ static int ap_proxy_http_prefetch(proxy_http_req_t
}
req->rb_method = RB_STREAM_CL;
}
- else if (req->old_te_val) {
- if (req->force10
- || (apr_table_get(r->subprocess_env, "proxy-sendcl")
- && !apr_table_get(r->subprocess_env, "proxy-sendchunks")
- && !apr_table_get(r->subprocess_env, "proxy-sendchunked"))) {
- req->rb_method = RB_SPOOL_CL;
- }
- else {
+ else if (r->input_filters == r->proto_input_filters
+ || (!req->force10 && !apr_table_get(r->subprocess_env,
+ "proxy-no-chunked"))) {
+ if (!req->old_cl_val || r->input_filters != r->proto_input_filters) {
req->rb_method = RB_STREAM_CHUNKED;
}
- }
- else if (req->old_cl_val) {
- if (r->input_filters == r->proto_input_filters) {
- if (!ap_parse_strict_length(&req->cl_val, req->old_cl_val)) {
- ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(01085)
- "could not parse request Content-Length (%s)",
- req->old_cl_val);
- return HTTP_BAD_REQUEST;
- }
+ else if (ap_parse_strict_length(&req->cl_val, req->old_cl_val)) {
req->rb_method = RB_STREAM_CL;
}
- else if (!req->force10
- && (apr_table_get(r->subprocess_env, "proxy-sendchunks")
- || apr_table_get(r->subprocess_env, "proxy-sendchunked"))
- && !apr_table_get(r->subprocess_env, "proxy-sendcl")) {
- req->rb_method = RB_STREAM_CHUNKED;
- }
else {
- req->rb_method = RB_SPOOL_CL;
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(01085)
+ "could not parse request Content-Length (%s)",
+ req->old_cl_val);
+ return HTTP_BAD_REQUEST;
}
}
else {
- /* This is an appropriate default; very efficient for no-body
- * requests, and has the behavior that it will not add any C-L
- * when the old_cl_val is NULL.
- */
req->rb_method = RB_SPOOL_CL;
}
