Re: svn commit: r1482918 - in /httpd/httpd/trunk: modules/http/http_filters.c server/protocol.c

2013-07-25 Thread Rainer Jung
On 15.05.2013 17:46, minf...@apache.org wrote:
 Author: minfrin
 Date: Wed May 15 15:46:01 2013
 New Revision: 1482918
 
 URL: http://svn.apache.org/r1482918
 Log:
 core: Stop ap_finalize_request_protocol() and ap_get_client_block() from 
 silently
 swallowing errors from the filter stack, create error buckets and return them
 appropriately.
 
 Modified:
 httpd/httpd/trunk/modules/http/http_filters.c
 httpd/httpd/trunk/server/protocol.c
 
 Modified: httpd/httpd/trunk/modules/http/http_filters.c
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=1482918r1=1482917r2=1482918view=diff
 ==
 --- httpd/httpd/trunk/modules/http/http_filters.c (original)
 +++ httpd/httpd/trunk/modules/http/http_filters.c Wed May 15 15:46:01 2013

...

 @@ -1637,6 +1643,28 @@ AP_DECLARE(long) ap_get_client_block(req
   * not be used.
   */
  if (rv != APR_SUCCESS) {
 +apr_bucket *e;
 +
 +/* work around our silent swallowing of error messages by mapping
 + * error codes at this point, and sending an error bucket back
 + * upstream.
 + */
 +apr_brigade_cleanup(bb);
 +
 +e = ap_bucket_error_create(
 +ap_map_http_request_error(rv, HTTP_BAD_REQUEST), NULL, 
 r-pool,
 +r-connection-bucket_alloc);
 +APR_BRIGADE_INSERT_TAIL(bb, e);
 +
 +e = apr_bucket_eos_create(r-connection-bucket_alloc);
 +APR_BRIGADE_INSERT_TAIL(bb, e);
 +
 +rv = ap_pass_brigade(r-output_filters, bb);
 +if (APR_SUCCESS != rv) {
 +ap_log_rerror(APLOG_MARK, APLOG_INFO, rv, r, APLOGNO()
 +  Error while writing error response);
 +}
 +
  /* if we actually fail here, we want to just return and
   * stop trying to read data from the client.
   */
 

I have a question about this part which turned up when debugging a
mod_deflate test failure. Bear with me and my very incomplete
understanding of bucket brigades:

Is this error bucket insertion safe? In the mod_deflate test we added
spurious data to the end of a compressed request body. When processing
the request, mod_deflate inflated the request body and mod_echo_post
returned it. If the body was big enough the echoed data appeared at the
client. Now the spurious data was seen by mod_deflate and the above
error bucket inserted. This error did not show up on the client side.

Now the test case received a connection close header directly at the
beginning of the response, but I wonder what would have happened if this
had been a keep-alive connection. Would the error bucket have been
queued and returned in front of the next response? Or would the
connection have been aborted by the server in any case? I'm a little bit
nervous here because of the potential for response mixup.

If the posted body was small enough that the spurious data was found
before the first part of the response as sent out, then the server
correctly returns the 400 error page instead of the echo response.

Regards,

Rainer


Re: svn commit: r1482918 - in /httpd/httpd/trunk: modules/http/http_filters.c server/protocol.c

2013-05-27 Thread Guenter Knauf

Hi Graham,
seems you forgot to add a log number at line 1541:

On 15.05.2013 17:46, minf...@apache.org wrote:

Author: minfrin
Date: Wed May 15 15:46:01 2013
New Revision: 1482918

URL: http://svn.apache.org/r1482918
Log:
core: Stop ap_finalize_request_protocol() and ap_get_client_block() from 
silently
swallowing errors from the filter stack, create error buckets and return them
appropriately.

Modified:
 httpd/httpd/trunk/modules/http/http_filters.c
 httpd/httpd/trunk/server/protocol.c

Modified: httpd/httpd/trunk/modules/http/http_filters.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/http_filters.c?rev=1482918r1=1482917r2=1482918view=diff
==
--- httpd/httpd/trunk/modules/http/http_filters.c (original)
+++ httpd/httpd/trunk/modules/http/http_filters.c Wed May 15 15:46:01 2013

...

+if (APR_SUCCESS != rv) {
+ap_log_rerror(APLOG_MARK, APLOG_INFO, rv, r, APLOGNO()
+  Error while writing error response);
+}
+
  /* if we actually fail here, we want to just return and
   * stop trying to read data from the client.
   */



Gün.