Created attachment 28492 [details]
patch for mod_ratelimit

To replay, use the following setting in httpd.conf:

<Location /downloads>
SetOutputFilter RATE_LIMIT
SetEnv rate-limit -400
</Location>

Start the httpd server; use your browser to access http://ipaddress/downloads/
Then, the access will be stuck and the server will print out the following message in error log: 

[Wed Mar 21 10:58:33.664383 2012] [ratelimit:error] [pid 2951:tid 140736908777216] [client 132.239.17.127:46106] AH01456: rl: partition failed.

The message is really bad for users. Who knows what is the "rl" and what is the "partition"?

Take a look at the code and find the root cause: 

------------------------

//modules/filters/mod_ratelimit.c

const char *rl = NULL;

rl = apr_table_get(f->r->subprocess_env, "rate-limit");

if (rl == NULL) {
    ap_remove_output_filter(f);
    return ap_pass_brigade(f->next, bb);
}

/* first run, init stuff */
ctx = apr_palloc(f->r->pool, sizeof(rl_ctx_t));
f->ctx = ctx;
ctx->speed = 0;
ctx->state = RATE_LIMIT;

/* rl is in kilo bytes / second  */
ctx->speed = atoi(rl) * 1024;

if (ctx->speed == 0) {
    /* remove ourselves */
    ap_remove_output_filter(f);
    return ap_pass_brigade(f->next, bb);
}

/* calculate how many bytes / interval we want to send */
/* speed is bytes / second, so, how many  (speed / 1000 % interval) */
ctx->chunk_size = (ctx->speed / (1000 / RATE_INTERVAL_MS));

......

rv = apr_brigade_partition(bb, ctx->chunk_size, &stop_point);
if (rv != APR_SUCCESS && rv != APR_INCOMPLETE) {
    ctx->state = RATE_ERROR;
    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, f->r, APLOGNO(01456)
                  "rl: partition failed.");
    break;
}

------------------------

Here is the thing: after get the ctx->speed, the code only checks whether it's 0 or not. If it's 0, then the cleaning job will be done. But a negative value will go much further and cause the error point.

A simple fix will be change the checking from "if (ctx->speed == 0) {"  to "if (ctx->speed <= 0) {" as the attachment.


Thanks!