The patch below changes the byterange filter to only do byteranging if
passed a complete EOS-terminated brigade containing only buckets with
non-negative length. This fixes the cases where the filter will
currently eat all your RAM (PR 29962).
It would be possible to also extend the code (adding significant
complexity) to be able to "streamily" byterange arbitrary responses for
certain byterange requests. I don't think it is necessary or even
desirable to do this in the stock byterange filter:
1) it's not particularly useful to be able to retrieve byteranges of the
output of some SSI, CGI or PHP script output, if the output may change
for each request anyway.
2) it's only good to byterange if it's clearly "cheap" to do so. For a
CGI script which does some expensive computation then generates a
response, byteranging the output is not desirable. Similarly for the
case of a reverse proxy to a dumb backend given in PR 29962: if the
backend doesn't support byteranges, handling a "last five bytes please"
byterange request *on the proxy* is dumb.
OK, let the flames begin...
http_protocol.c | 80 +++++++++++++++++++++-----------------------------------
1 files changed, 30 insertions(+), 50 deletions(-)
Index: modules/http/http_protocol.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
retrieving revision 1.484
diff -u -r1.484 http_protocol.c
--- modules/http/http_protocol.c 29 Sep 2004 14:38:42 -0000 1.484
+++ modules/http/http_protocol.c 12 Oct 2004 13:07:18 -0000
@@ -2839,13 +2839,6 @@
static int ap_set_byterange(request_rec *r);
-typedef struct byterange_ctx {
- apr_bucket_brigade *bb;
- int num_ranges;
- char *boundary;
- char *bound_head;
-} byterange_ctx;
-
/*
* Here we try to be compatible with clients that want multipart/x-byteranges
* instead of multipart/byteranges (also see above), as per HTTP/1.1. We
@@ -2871,19 +2864,35 @@
#define MIN_LENGTH(len1, len2) ((len1 > len2) ? len2 : len1)
request_rec *r = f->r;
conn_rec *c = r->connection;
- byterange_ctx *ctx = f->ctx;
apr_bucket *e;
apr_bucket_brigade *bsend;
apr_off_t range_start;
apr_off_t range_end;
char *current;
- apr_off_t bb_length;
apr_off_t clength = 0;
apr_status_t rv;
int found = 0;
+ int num_ranges;
+ char *boundary = NULL; /* init to avoid compiler warning */
+ char *bound_head = NULL;
+
+ for (e = APR_BRIGADE_FIRST(bb);
+ e != APR_BRIGADE_SENTINEL(bb) && !APR_BUCKET_IS_EOS(e)
+ && e->length != (apr_size_t)-1;
+ e = APR_BUCKET_NEXT(e)) {
+ clength += e->length;
+ }
+
+ /* Don't attempt to do byte range work if the brigade doesn't
+ * contain an EOS, or or if any of the buckets has length == -1
+ * (which implies the bucket will morph when ->read(). */
+
+ if (!APR_BUCKET_IS_EOS(e) || clength <= 0) {
+ ap_remove_output_filter(f);
+ return ap_pass_brigade(f->next, bb);
+ }
- if (!ctx) {
- int num_ranges = ap_set_byterange(r);
+ num_ranges = ap_set_byterange(r);
/* We have nothing to do, get out of the way. */
if (num_ranges == 0) {
@@ -2891,54 +2900,25 @@
return ap_pass_brigade(f->next, bb);
}
- ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
- ctx->num_ranges = num_ranges;
- /* create a brigade in case we never call ap_save_brigade() */
- ctx->bb = apr_brigade_create(r->pool, c->bucket_alloc);
-
- if (ctx->num_ranges > 1) {
+ if (num_ranges > 1) {
/* Is ap_make_content_type required here? */
const char *orig_ct = ap_make_content_type(r, r->content_type);
- ctx->boundary = apr_psprintf(r->pool, "%" APR_UINT64_T_HEX_FMT "%lx",
+ boundary = apr_psprintf(r->pool, "%" APR_UINT64_T_HEX_FMT "%lx",
(apr_uint64_t)r->request_time, (long)
getpid());
ap_set_content_type(r, apr_pstrcat(r->pool, "multipart",
use_range_x(r) ? "/x-" : "/",
"byteranges; boundary=",
- ctx->boundary, NULL));
+ boundary, NULL));
- ctx->bound_head = apr_pstrcat(r->pool,
- CRLF "--", ctx->boundary,
+ bound_head = apr_pstrcat(r->pool,
+ CRLF "--", boundary,
CRLF "Content-type: ",
orig_ct,
CRLF "Content-range: bytes ",
NULL);
- ap_xlate_proto_to_ascii(ctx->bound_head, strlen(ctx->bound_head));
+ ap_xlate_proto_to_ascii(bound_head, strlen(bound_head));
}
- }
-
- /* We can't actually deal with byte-ranges until we have the whole brigade
- * because the byte-ranges can be in any order, and according to the RFC,
- * we SHOULD return the data in the same order it was requested.
- *
- * XXX: We really need to dump all bytes prior to the start of the earliest
- * range, and only slurp up to the end of the latest range. By this we
- * mean that we should peek-ahead at the lowest first byte of any range,
- * and the highest last byte of any range.
- */
- if (!APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) {
- ap_save_brigade(f, &ctx->bb, &bb, r->pool);
- return APR_SUCCESS;
- }
-
- /* Prepend any earlier saved brigades. */
- APR_BRIGADE_PREPEND(bb, ctx->bb);
-
- /* It is possible that we won't have a content length yet, so we have to
- * compute the length before we can actually do the byterange work.
- */
- apr_brigade_length(bb, 1, &bb_length);
- clength = (apr_off_t)bb_length;
/* this brigade holds what we will be sending */
bsend = apr_brigade_create(r->pool, c->bucket_alloc);
@@ -2972,7 +2952,7 @@
/* For single range requests, we must produce Content-Range header.
* Otherwise, we need to produce the multipart boundaries.
*/
- if (ctx->num_ranges == 1) {
+ if (num_ranges == 1) {
apr_table_setn(r->headers_out, "Content-Range",
apr_psprintf(r->pool, "bytes " BYTERANGE_FMT,
range_start, range_end, clength));
@@ -2980,7 +2960,7 @@
else {
char *ts;
- e = apr_bucket_pool_create(ctx->bound_head, strlen(ctx->bound_head),
+ e = apr_bucket_pool_create(bound_head, strlen(bound_head),
r->pool, c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bsend, e);
@@ -3025,11 +3005,11 @@
return ap_pass_brigade(f->next, bsend);
}
- if (ctx->num_ranges > 1) {
+ if (num_ranges > 1) {
char *end;
/* add the final boundary */
- end = apr_pstrcat(r->pool, CRLF "--", ctx->boundary, "--" CRLF, NULL);
+ end = apr_pstrcat(r->pool, CRLF "--", boundary, "--" CRLF, NULL);
ap_xlate_proto_to_ascii(end, strlen(end));
e = apr_bucket_pool_create(end, strlen(end), r->pool, c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bsend, e);
