Compliance: Improved HTTP Range header field validation.

1) Improve HttpHdrRangeSpec::parseInit() to parse syntactically valid range
specs:

* Suffix ranges with 0 length (i.e. -0), are syntactically valid.

* Check that last-byte-pos is greater than or equal to first-byte-pos.

After the change, HttpHdrRangeSpec::parseInit() successfully parses suffix
ranges with 0 length. They were rejected before. RFC 2616 section 14.35.1 says
such range specs are syntactically valid but unsatisfiable. Thus, we should
ignore the range spec itself, but not the whole range header. These range
specs will be rejected later, during canonization.


2) In HttpHdrRangeSpec::parseInit(), ignore the whole range header if one of
range specs is syntactically invalid (i.e. range spec parsing fails).


Co-Advisor test case: test_clause/rfc2616/invalidRange

=== modified file 'src/HttpHdrRange.cc'
--- src/HttpHdrRange.cc	2010-04-07 09:49:57 +0000
+++ src/HttpHdrRange.cc	2010-08-01 18:39:28 +0000
@@ -81,67 +81,71 @@ HttpHdrRangeSpec::Create(const char *fie
         return NULL;
 
     return new HttpHdrRangeSpec(spec);
 }
 
 bool
 HttpHdrRangeSpec::parseInit(const char *field, int flen)
 {
     const char *p;
 
     if (flen < 2)
         return false;
 
     /* is it a suffix-byte-range-spec ? */
     if (*field == '-') {
         if (!httpHeaderParseOffset(field + 1, &length))
             return false;
     } else
         /* must have a '-' somewhere in _this_ field */
         if (!((p = strchr(field, '-')) || (p - field >= flen))) {
-            debugs(64, 2, "ignoring invalid (missing '-') range-spec near: '" << field << "'");
+            debugs(64, 2, "invalid (missing '-') range-spec near: '" << field << "'");
             return false;
         } else {
             if (!httpHeaderParseOffset(field, &offset))
                 return false;
 
             p++;
 
             /* do we have last-pos ? */
             if (p - field < flen) {
                 int64_t last_pos;
 
                 if (!httpHeaderParseOffset(p, &last_pos))
                     return false;
 
+                /*
+                 * RFC 2616 section 14.35.1 says that the
+                 * last-byte-pos MUST be greater than or equal to the
+                 * first-byte-pos.
+                 */
+                if (last_pos < offset) {
+                    debugs(64, 2, "invalid (last-byte-pos < first-byte-pos) range-spec near: " << field);
+                    return false;
+                }
+
                 HttpHdrRangeSpec::HttpRange aSpec (offset, last_pos + 1);
 
                 length = aSpec.size();
             }
         }
 
-    /* we managed to parse, check if the result makes sence */
-    if (length == 0) {
-        debugs(64, 2, "ignoring invalid (zero length) range-spec near: '" << field << "'");
-        return false;
-    }
-
     return true;
 }
 
 void
 HttpHdrRangeSpec::packInto(Packer * packer) const
 {
     if (!known_spec(offset))	/* suffix */
         packerPrintf(packer, "-%"PRId64,  length);
     else if (!known_spec(length))		/* trailer */
         packerPrintf(packer, "%"PRId64"-", offset);
     else			/* range */
         packerPrintf(packer, "%"PRId64"-%"PRId64,
                      offset, offset + length - 1);
 }
 
 void
 HttpHdrRangeSpec::outputInfo( char const *note) const
 {
     debugs(64, 5, "HttpHdrRangeSpec::canonize: " << note << ": [" <<
            offset << ", " << offset + length <<
@@ -231,69 +235,70 @@ HttpHdrRange::HttpHdrRange () : clen (Ht
 HttpHdrRange *
 HttpHdrRange::ParseCreate(const String * range_spec)
 {
     HttpHdrRange *r = new HttpHdrRange;
 
     if (!r->parseInit(range_spec)) {
         delete r;
         r = NULL;
     }
 
     return r;
 }
 
 /* returns true if ranges are valid; inits HttpHdrRange */
 bool
 HttpHdrRange::parseInit(const String * range_spec)
 {
     const char *item;
     const char *pos = NULL;
     int ilen;
-    int count = 0;
     assert(this && range_spec);
     ++ParsedCount;
     debugs(64, 8, "parsing range field: '" << range_spec << "'");
     /* check range type */
 
     if (range_spec->caseCmp("bytes=", 6))
         return 0;
 
     /* skip "bytes="; hack! */
     pos = range_spec->termedBuf() + 6;
 
     /* iterate through comma separated list */
     while (strListGetItem(range_spec, ',', &item, &ilen, &pos)) {
         HttpHdrRangeSpec *spec = HttpHdrRangeSpec::Create(item, ilen);
         /*
-         * HTTP/1.1 draft says we must ignore the whole header field if one spec
-         * is invalid. However, RFC 2068 just says that we must ignore that spec.
+         * RFC 2616 section 14.35.1: MUST ignore Range with
+         * at least one syntactically invalid byte-range-specs.
          */
+        if (!spec) {
+            while (!specs.empty())
+                delete specs.pop_back();
+            debugs(64, 2, "ignoring invalid range field: '" << range_spec << "'");
+            break;
+        }
 
-        if (spec)
-            specs.push_back(spec);
-
-        ++count;
+        specs.push_back(spec);
     }
 
-    debugs(64, 8, "parsed range range count: " << count << ", kept " <<
-           specs.size());
-    return specs.count != 0;
+    debugs(64, 8, "got range specs: " << specs.size());
+    return !specs.empty();
 }
 
 HttpHdrRange::~HttpHdrRange()
 {
     while (specs.size())
         delete specs.pop_back();
 }
 
 HttpHdrRange::HttpHdrRange(HttpHdrRange const &old) : specs()
 {
     specs.reserve(old.specs.size());
 
     for (const_iterator i = old.begin(); i != old.end(); ++i)
         specs.push_back(new HttpHdrRangeSpec ( **i));
 
     assert(old.specs.size() == specs.size());
 }
 
 HttpHdrRange::iterator
 HttpHdrRange::begin()