On Thu, Oct 22, 2015 at 3:42 PM, Andy Wang <[email protected]> wrote:
>
> Tested with the patch and looks good.
Not that much actually, the patch fails to consume the CRLFs, and
hence can end up in an infinite loop.
So I'm attaching a new one here (committed in trunk with a larger
scope, this version is for 2.4.x and limited to your use case).
Could you give it a (new) try please (I have already done some testing
but it's probably worth passing your tests, before I can propose its
backport to 2.4.x)?
While the previous patch could not handle more than a single
(trailing) [CR]LF, this new one should (up to ten, which is the new
limit for tolerated blank lines in between requests).
Thanks,
Yann.
Index: include/httpd.h
===================================================================
--- include/httpd.h (revision 1710105)
+++ include/httpd.h (working copy)
@@ -200,6 +200,10 @@ extern "C" {
#ifndef DEFAULT_LIMIT_REQUEST_FIELDS
#define DEFAULT_LIMIT_REQUEST_FIELDS 100
#endif
+/** default/hard limit on number of leading/trailing empty lines */
+#ifndef DEFAULT_LIMIT_BLANK_LINES
+#define DEFAULT_LIMIT_BLANK_LINES 10
+#endif
/**
* The default default character set name to add if AddDefaultCharset is
Index: modules/http/http_request.c
===================================================================
--- modules/http/http_request.c (revision 1710105)
+++ modules/http/http_request.c (working copy)
@@ -230,22 +230,91 @@ AP_DECLARE(void) ap_die(int type, request_rec *r)
static void check_pipeline(conn_rec *c, apr_bucket_brigade *bb)
{
+ c->data_in_input_filters = 0;
if (c->keepalive != AP_CONN_CLOSE && !c->aborted) {
apr_status_t rv;
+ int num_blank_lines = DEFAULT_LIMIT_BLANK_LINES;
+ ap_input_mode_t mode = AP_MODE_SPECULATIVE;
+ apr_size_t len, cr = 0;
+ char buf[2];
- AP_DEBUG_ASSERT(APR_BRIGADE_EMPTY(bb));
- rv = ap_get_brigade(c->input_filters, bb, AP_MODE_SPECULATIVE,
- APR_NONBLOCK_READ, 1);
- if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(bb)) {
- /*
- * Error or empty brigade: There is no data present in the input
- * filter
+ do {
+ apr_brigade_cleanup(bb);
+ rv = ap_get_brigade(c->input_filters, bb, mode,
+ APR_NONBLOCK_READ, cr + 1);
+ if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(bb)) {
+ /*
+ * Error or empty brigade: There is no data present in the input
+ * filter
+ */
+ if (mode == AP_MODE_READBYTES) {
+ /* Unexpected error, stop with this connection */
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, rv, c, APLOGNO(02967)
+ "Can't consume pipelined empty lines");
+ c->keepalive = AP_CONN_CLOSE;
+ }
+ return;
+ }
+
+ /* Ignore trailing blank lines (which must not be interpreted as
+ * pipelined requests) up to the limit, otherwise we would block
+ * on the next read without flushing data, and hence possibly delay
+ * pending response(s) until the next/real request comes in or the
+ * keepalive timeout expires.
*/
- c->data_in_input_filters = 0;
- }
- else {
- c->data_in_input_filters = 1;
- }
+ len = cr + 1;
+ rv = apr_brigade_flatten(bb, buf, &len);
+ if (rv != APR_SUCCESS || len != cr + 1) {
+ int level;
+ if (mode == AP_MODE_READBYTES) {
+ /* Unexpected error, stop with this connection */
+ c->keepalive = AP_CONN_CLOSE;
+ level = APLOG_ERR;
+ }
+ else {
+ /* Let outside (non-speculative/blocking) read determine
+ * where this possible failure comes from (metadata,
+ * morphed EOF socket => empty bucket? debug only here).
+ */
+ c->data_in_input_filters = 1;
+ level = APLOG_DEBUG;
+ }
+ ap_log_cerror(APLOG_MARK, level, rv, c, APLOGNO(02968)
+ "Can't check pipelined data");
+ return;
+ }
+
+ if (mode == AP_MODE_READBYTES) {
+ mode = AP_MODE_SPECULATIVE;
+ cr = 0;
+ continue;
+ }
+
+ if (cr) {
+ AP_DEBUG_ASSERT(len == 2 && buf[0] == APR_ASCII_CR);
+ if (buf[1] != APR_ASCII_LF) {
+ return;
+ }
+ mode = AP_MODE_READBYTES;
+ num_blank_lines--;
+ }
+ else {
+ if (buf[0] == APR_ASCII_CR) {
+ cr = 1;
+ }
+ else if (buf[0] == APR_ASCII_LF) {
+ mode = AP_MODE_READBYTES;
+ num_blank_lines--;
+ }
+ else {
+ c->data_in_input_filters = 1;
+ return;
+ }
+ }
+ } while (num_blank_lines >= 0);
+
+ /* Don't recycle this (abused) connection */
+ c->keepalive = AP_CONN_CLOSE;
}
}
