On 12/01/2014 02:15 PM, Yann Ylavic wrote:
On Wed, Nov 19, 2014 at 8:19 AM, <jkal...@apache.org> wrote:
Author: jkaluza
Date: Wed Nov 19 07:19:13 2014
New Revision: 1640495
URL: http://svn.apache.org/r1640495
Log:
* mod_proxy_fcgi: Ignore body data from backend for 304 responses. PR 57198.
Modified:
httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
[]
--- httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c Wed Nov 19 07:19:13 2014
@@ -369,7 +369,7 @@ static apr_status_t dispatch(proxy_conn_
const char **err)
{
apr_bucket_brigade *ib, *ob;
- int seen_end_of_headers = 0, done = 0;
+ int seen_end_of_headers = 0, done = 0, ignore_body = 0;
apr_status_t rv = APR_SUCCESS;
int script_error_status = HTTP_OK;
conn_rec *c = r->connection;
@@ -579,9 +579,16 @@ recv_again:
APR_BRIGADE_INSERT_TAIL(ob, tmp_b);
r->status = status;
ap_pass_brigade(r->output_filters, ob);
- ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
APLOGNO(01070)
- "Error parsing script headers");
- rv = APR_EINVAL;
+ if (status == HTTP_NOT_MODIFIED) {
+ /* The 304 response MUST NOT contain
+ * a message-body, ignore it. */
+ ignore_body = 1;
+ }
+ else {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
APLOGNO(01070)
+ "Error parsing script
headers");
+ rv = APR_EINVAL;
+ }
break;
}
There seems to be another case below this point where we "Send the
part of the body that we read while reading the headers" (as the
comment says), with no !ignore_body check.
I agree with this part and I will fix it in my next commit.
Also, the backend may use a "Status: 304 Not Modified" header, which
would not result in ap_scan_script_header_err_brigade_ex() returning
that status but setting it to r->status.
In this case, it's up to backend to not send the body and if it sends
some body together with 304 status, we don't have to care about that,
right? I can implement also check for r->status and set ignore_body=1,
but I thought it's not needed on httpd side.
So, more generally, shouldn't we check both
ap_scan_script_header_err_brigade_ex()'s returned status AND r->status
against 304 to ignore the body (ie. bypass ap_pass_brigade() for
anything but EOS)?
Regards,
Yann.
Regards,
Jan Kaluza
