trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a..@apache.org
Subject git commit: TS-1265 Fix range handling for open end ranges.
Date Mon, 21 May 2012 22:48:22 GMT
Updated Branches:
  refs/heads/master 0fd1b94b1 -> b57523bdb


TS-1265 Fix range handling for open end ranges.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/b57523bd
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/b57523bd
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/b57523bd

Branch: refs/heads/master
Commit: b57523bdbb1e45a88d2e4ee78ba4f1649a116039
Parents: 0fd1b94
Author: Alan M. Carroll <amc@network-geographics.com>
Authored: Mon May 21 07:59:01 2012 -0500
Committer: Alan M. Carroll <amc@network-geographics.com>
Committed: Mon May 21 17:44:13 2012 -0500

----------------------------------------------------------------------
 proxy/http/HttpSM.cc |  106 ++++++++++++++++++++++++++-------------------
 1 files changed, 61 insertions(+), 45 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/b57523bd/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 18b2aab..2622fa4 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -3889,9 +3889,11 @@ void
 HttpSM::parse_range_and_compare(MIMEField *field, int64_t content_length)
 {
   // note: unsatisfiable_range is initialized to true in constructor
-  int prev_good_range, i;
+  int prev_good_range = -1;
   const char *value;
   int value_len;
+  int n_values;
+  int nr = 0; // number of valid ranges, also index to range array.
   HdrCsvIter csv;
   const char *s, *e;
 
@@ -3900,26 +3902,26 @@ HttpSM::parse_range_and_compare(MIMEField *field, int64_t content_length)
 
   ink_assert(field != NULL);
 
-  t_state.num_range_fields = 0;
+  n_values = 0;
   value = csv.get_first(field, &value_len);
-
   while (value) {
-    t_state.num_range_fields++;
+    ++n_values;
     value = csv.get_next(&value_len);
   }
 
-  if (t_state.num_range_fields <= 0)
+  if (n_values <= 0)
     return;
 
-  t_state.ranges = NEW(new RangeRecord[t_state.num_range_fields]);
-
   value = csv.get_first(field, &value_len);
 
-  i = 0;
-  prev_good_range = -1;
   // Currently HTTP/1.1 only defines bytes Range
   if (ptr_len_ncmp(value, value_len, "bytes=", 6) == 0) {
+    t_state.ranges = NEW(new RangeRecord[n_values]);
+    value += 6; // skip leading 'bytes='
+    value_len -= 6;
+
     while (value) {
+      bool valid = true; // found valid range.
       // If delimiter '-' is missing
       if (!(e = (const char *) memchr(value, '-', value_len))) {
         t_state.range_setup = HttpTransact::RANGE_NOT_SATISFIABLE;
@@ -3927,71 +3929,85 @@ HttpSM::parse_range_and_compare(MIMEField *field, int64_t content_length)
         return;
       }
 
-      if ( memcmp(value,"bytes=",6) == 0 ) {
-        s = value + 6;
-      }
-      else {
-        s = value;
-      }
-      t_state.ranges[i]._start = ((s==e)?-1:mime_parse_int64(s, e));
+      /* [amc] We should do a much better job of checking the values
+         from mime_parse_int64 to detect invalid range values (e.g.
+         non-numeric). Those need to be handled differently than
+         missing values. My reading of the spec is that ATS should go to
+         RANGE_NONE in such a case.
+      */
+      s = value;
+      t_state.ranges[nr]._start = ((s==e)?-1:mime_parse_int64(s, e));
 
-      e++;
+      ++e;
       s = e;
       e = value + value_len;
       if ( e && *(e-1) == '-') { //open-ended Range: bytes=10-\r\n\r\n should be
supported
-        t_state.ranges[i]._end = -1;
+        t_state.ranges[nr]._end = -1;
       }
       else {
-        t_state.ranges[i]._end = mime_parse_int64(s, e);
+        t_state.ranges[nr]._end = mime_parse_int64(s, e);
       }
 
       // check and change if necessary whether this is a right entry
-      // the last _end bytes are required
-      if (t_state.ranges[i]._start == -1 && t_state.ranges[i]._end > 0) {
-        if (t_state.ranges[i]._end > content_length)
-          t_state.ranges[i]._end = content_length;
+      if (t_state.ranges[nr]._start >= content_length) {
+          valid = false;
+      } 
+      // open start
+      else if (t_state.ranges[nr]._start == -1 && t_state.ranges[nr]._end > 0)
{
+        if (t_state.ranges[nr]._end > content_length)
+          t_state.ranges[nr]._end = content_length;
 
-        t_state.ranges[i]._start = content_length - t_state.ranges[i]._end;
-        t_state.ranges[i]._end = content_length - 1;
+        t_state.ranges[nr]._start = content_length - t_state.ranges[nr]._end;
+        t_state.ranges[nr]._end = content_length - 1;
       }
-      // open start
-      else if (t_state.ranges[i]._start >= 0 && t_state.ranges[i]._end == -1)
{
-        if (t_state.ranges[i]._start >= content_length)
-          t_state.ranges[i]._start = -1;
-        else
-          t_state.ranges[i]._end = content_length - 1;
+      // open end
+      else if (t_state.ranges[nr]._start >= 0 && t_state.ranges[nr]._end == -1)
{
+          t_state.ranges[nr]._end = content_length - 1;
       }
       // "normal" Range - could be wrong if _end<_start
-      else if (t_state.ranges[i]._start >= 0 && t_state.ranges[i]._end >= 0)
{
-        if (t_state.ranges[i]._start > t_state.ranges[i]._end || t_state.ranges[i]._start
>= content_length)
-          t_state.ranges[i]._start = t_state.ranges[i]._end = -1;
-        else if (t_state.ranges[i]._end >= content_length)
-          t_state.ranges[i]._end = content_length - 1;
+      else if (t_state.ranges[nr]._start >= 0 && t_state.ranges[nr]._end >=
0) {
+        if (t_state.ranges[nr]._start > t_state.ranges[nr]._end)
+          // [amc] My reading of the spec is that this should cause a change
+          // to RANGE_NONE because it is syntatically invalid.
+          valid = false;
+        else if (t_state.ranges[nr]._end >= content_length)
+          t_state.ranges[nr]._end = content_length - 1;
+      }
+      // Syntactically invalid range, fail.
+      else {
+        // [amc] My reading of the spec is that this should cause a change
+        // to RANGE_NONE because it is syntatically invalid.
+        t_state.range_setup = HttpTransact::RANGE_NOT_SATISFIABLE;
+        t_state.num_range_fields = -1;
+        return;
       }
-
-      else
-        t_state.ranges[i]._start = t_state.ranges[i]._end = -1;
 
       // this is a good Range entry
-      if (t_state.ranges[i]._start != -1) {
+      if (valid) {
         if (t_state.unsatisfiable_range) {
           t_state.unsatisfiable_range = false;
           // initialize t_state.current_range to the first good Range
-          t_state.current_range = i;
+          t_state.current_range = nr;
         }
         // currently we don't handle out-of-order Range entry
-        else if (prev_good_range >= 0 && t_state.ranges[i]._start <= t_state.ranges[prev_good_range]._end)
{
+        else if (prev_good_range >= 0 && t_state.ranges[nr]._start <= t_state.ranges[prev_good_range]._end)
{
           t_state.not_handle_range = true;
           break;
         }
 
-        prev_good_range = i;
+        prev_good_range = nr;
+        ++nr;
       }
-
       value = csv.get_next(&value_len);
-      i++;
     }
   }
+  // Fail if we didn't find any valid ranges.
+  if (nr < 1) {
+    t_state.range_setup = HttpTransact::RANGE_NOT_SATISFIABLE;
+    t_state.num_range_fields = -1;
+  } else {
+    t_state.num_range_fields = nr;
+  }
 }
 
 void


Mime
View raw message