On Mon, Dec 1, 2014 at 2:15 PM, Yann Ylavic <[email protected]> wrote: > On Wed, Nov 19, 2014 at 8:19 AM, <[email protected]> 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. > > 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. > 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)?
And maybe status 204 the same way too (no possible body with 204 either). > > Regards, > Yann.
