On 08/25/2011 07:38 PM, j...@apache.org wrote:
> Author: jim
> Date: Thu Aug 25 17:38:19 2011
> New Revision: 1161661
>
> URL: http://svn.apache.org/viewvc?rev=1161661&view=rev
> Log:
> Merge in byteranges
>
> Modified:
> httpd/httpd/trunk/modules/http/byterange_filter.c
>
> Modified: httpd/httpd/trunk/modules/http/byterange_filter.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http/byterange_filter.c?rev=1161661&r1=1161660&r2=1161661&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/http/byterange_filter.c (original)
> +++ httpd/httpd/trunk/modules/http/byterange_filter.c Thu Aug 25 17:38:19 2011
> @@ -469,11 +469,14 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_
>
> static int ap_set_byterange(request_rec *r)
> {
> - const char *range;
> + const char *range, *or;
> const char *if_range;
> const char *match;
> const char *ct;
> - int num_ranges;
> + char *merged, *cur;
> + int num_ranges = 0;
> + apr_off_t ostart, oend;
> + int in_merge = 0;
>
> if (r->assbackwards) {
> return 0;
> @@ -526,17 +529,81 @@ static int ap_set_byterange(request_rec
> }
> }
>
> - if (!ap_strchr_c(range, ',')) {
> - /* a single range */
> - num_ranges = 1;
> - }
> - else {
> - /* a multiple range */
> - num_ranges = 2;
> + range += 6;
> + or = apr_pstrdup(r->pool, range);
> + merged = apr_pstrdup(r->pool, "");
> + while ((cur = ap_getword(r->pool, &range, ','))) {
> + char *dash;
> + char *errp;
> + apr_off_t number, start, end;
> +
> + if (!(dash = strchr(cur, '-'))) {
> + break;
> + }
> +
> + if (dash == cur) {
> + /* In the form "-5"... leave as is */
> + merged = apr_pstrcat(r->pool, merged, (num_ranges++ ? "," : ""),
> cur, NULL);
> + continue;
> + }
> + *dash++ = '\0';
> + if (!*dash) {
> + /* form "5-" */
> + merged = apr_pstrcat(r->pool, merged, (num_ranges++ ? "," : ""),
> cur, "-", NULL);
> + continue;
> + }
> + /*
> + * we have #-#, so we need to grab them... we don't bother
> + * doing this for the #- or -# cases for speed reasons.
> + * After all, those will be fixed when the filter parses
> + * the merged range
> + */
> + if (apr_strtoff(&number, cur, &errp, 10) || *errp || number < 0) {
> + break;
> + }
> + start = number;
> + if (apr_strtoff(&number, dash, &errp, 10) || *errp || number <= 0) {
> + break;
> + }
> + end = number;
> + if (!in_merge) {
> + ostart = start;
> + oend = end;
> + }
> + in_merge = 0;
> + if (start <= ostart) {
> + ostart = start;
> + in_merge = 1;
> + }
> + if (start > ostart && start < oend) {
> + in_merge = 1;
> + }
> + if ((end-1) >= oend) {
> + oend = end;
> + in_merge = 1;
> + }
> + if (end > ostart && end < oend) {
> + in_merge = 1;
> + }
> + if (in_merge) {
> + continue;
I followed through the above algorithm with a range of "0-1,1000-1001" and the
result was a range of "0-1001" which feels wrong. Culprit seems to be the
((end-1) >= oend) check.
Shouldn't that be oend >= (start-1)?
> + } else {
> + char *nr = apr_psprintf(r->pool, "%" APR_OFF_T_FMT "-%"
> APR_OFF_T_FMT,
> + ostart, oend);
> + merged = apr_pstrcat(r->pool, merged, (num_ranges++ ? "," : ""),
> nr, NULL);
Doing this in a loop feels bad as it creates new buffers over and over.
Shouldn't we create a buffer of the size of strlen(range) and add to this
subsequently
(we know that the length of the merged range <= length of range)?
> + }
> }
>
> + if (in_merge) {
> + char *nr = apr_psprintf(r->pool, "%" APR_OFF_T_FMT "-%"
> APR_OFF_T_FMT,
> + ostart, oend);
> + merged = apr_pstrcat(r->pool, merged, (num_ranges++ ? "," : ""), nr,
> NULL);
> + }
> +
> r->status = HTTP_PARTIAL_CONTENT;
> - r->range = range + 6;
> + r->range = merged;
> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
> + "Range: %s | %s", or, merged);
>
> return num_ranges;
> }
>
Regards
Rüdiger
