You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by we...@apache.org on 2013/01/15 04:38:00 UTC

git commit: TS-1574, TS-1577 not do range acceleration when read_from_writer, and range request can invalidate cached copy when server response is 304.

Updated Branches:
  refs/heads/master 02d5a927f -> 1199e9914


TS-1574, TS-1577 not do range acceleration when read_from_writer, and
range request can invalidate cached copy when server response is 304.


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

Branch: refs/heads/master
Commit: 1199e991424d8495a80075718d8ec95a64f3a712
Parents: 02d5a92
Author: weijin <ta...@taobao.com>
Authored: Tue Jan 15 10:27:28 2013 +0800
Committer: weijin <ta...@taobao.com>
Committed: Tue Jan 15 10:27:28 2013 +0800

----------------------------------------------------------------------
 CHANGES                    |    3 +
 iocore/cache/Cache.cc      |    2 +-
 proxy/Transform.cc         |   34 +++--
 proxy/Transform.h          |    2 +-
 proxy/TransformInternal.h  |    4 +-
 proxy/http/HttpSM.cc       |  269 ++++++++++++++++++++-------------------
 proxy/http/HttpTransact.cc |   48 ++-----
 proxy/http/HttpTransact.h  |   11 +-
 proxy/http/HttpTunnel.cc   |    3 +-
 9 files changed, 183 insertions(+), 193 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1199e991/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 8091b47..1c442e0 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,9 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 3.3.1
 
+  *) [TS-1574] [TS-1577] when read_from_writer, we should not do range acceleration.
+   Range request can invalidate cached copy if the server reponse is 304.
+
   *) [TS-1609] Traffic Cop doesn't wait() for its children
 
   *) [TS-1601] HttpServerSession::release don't close ServerSession if ServerSessionPool locking contention

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1199e991/iocore/cache/Cache.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc
index 019d8d0..da3c7e1 100644
--- a/iocore/cache/Cache.cc
+++ b/iocore/cache/Cache.cc
@@ -1847,7 +1847,7 @@ CacheVC::dead(int event, Event *e) {
 bool
 CacheVC::is_pread_capable()
 {
-  return alternate.get_frag_offset_count() > 0;
+  return !f.read_from_writer_called;
 }
 
 #define STORE_COLLISION 1

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1199e991/proxy/Transform.cc
----------------------------------------------------------------------
diff --git a/proxy/Transform.cc b/proxy/Transform.cc
index 8674c33..80f3dff 100644
--- a/proxy/Transform.cc
+++ b/proxy/Transform.cc
@@ -114,9 +114,9 @@ TransformProcessor::null_transform(ProxyMutex *mutex)
   -------------------------------------------------------------------------*/
 
 INKVConnInternal *
-TransformProcessor::range_transform(ProxyMutex *mut, RangeRecord *ranges, bool unsatisfiable, int num_fields, HTTPHdr *transform_resp, const char * content_type, int content_type_len, int64_t content_length)
+TransformProcessor::range_transform(ProxyMutex *mut, RangeRecord *ranges, int num_fields, HTTPHdr *transform_resp, const char * content_type, int content_type_len, int64_t content_length)
 {
-  RangeTransform *range_transform = NEW(new RangeTransform(mut, ranges, unsatisfiable, num_fields, transform_resp, content_type, content_type_len, content_length));
+  RangeTransform *range_transform = NEW(new RangeTransform(mut, ranges, num_fields, transform_resp, content_type, content_type_len, content_length));
   return range_transform;
 }
 
@@ -729,13 +729,12 @@ TransformTest::run()
 /*-------------------------------------------------------------------------
   -------------------------------------------------------------------------*/
 
-RangeTransform::RangeTransform(ProxyMutex *mut, RangeRecord *ranges,bool unsatisfiable, int num_fields, HTTPHdr * transform_resp, const char * content_type, int content_type_len, int64_t content_length)
+RangeTransform::RangeTransform(ProxyMutex *mut, RangeRecord *ranges, int num_fields, HTTPHdr * transform_resp, const char * content_type, int content_type_len, int64_t content_length)
   : INKVConnInternal(NULL, reinterpret_cast<TSMutex>(mut)),
   m_output_buf(NULL),
   m_output_reader(NULL),
   m_transform_resp(transform_resp),
   m_output_vio(NULL),
-  m_unsatisfiable_range(unsatisfiable),
   m_range_content_length(0),
   m_num_range_fields(num_fields),
   m_current_range(0), m_content_type(content_type), m_content_type_len(content_type_len), m_ranges(ranges), m_output_cl(content_length), m_done(0)
@@ -969,7 +968,7 @@ RangeTransform::add_sub_header(int index)
   m_done += m_output_buf->write("\r\n", 2);
   m_done += m_output_buf->write(cont_range, sizeof(cont_range) - 1);
 
-  snprintf(numbers, sizeof(numbers), "%" PRId64 "d-%" PRId64 "d/%" PRId64 "d", m_ranges[index]._start, m_ranges[index]._end, m_output_cl);
+  snprintf(numbers, sizeof(numbers), "%" PRId64 "-%" PRId64 "/%" PRId64 "", m_ranges[index]._start, m_ranges[index]._end, m_output_cl);
   len = strlen(numbers);
   if (len < RANGE_NUMBERS_LENGTH)
     m_done += m_output_buf->write(numbers, len);
@@ -992,25 +991,32 @@ RangeTransform::change_response_header()
   HTTPStatus status_code;
   
   ink_release_assert(m_transform_resp);
-  ink_release_assert(m_transform_resp->field_find(MIME_FIELD_CONTENT_RANGE, MIME_LEN_CONTENT_RANGE) == NULL);
 
   status_code = HTTP_STATUS_PARTIAL_CONTENT;
   m_transform_resp->status_set(status_code);
   reason_phrase = (char *) (http_hdr_reason_lookup(status_code));
   m_transform_resp->reason_set(reason_phrase, strlen(reason_phrase));
 
-  // set the right Content-Type for multiple entry Range
-  field = m_transform_resp->field_find(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE);
+  if (m_num_range_fields > 1) {
+    // set the right Content-Type for multiple entry Range
+    field = m_transform_resp->field_find(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE);
 
-  if (field != NULL)
-    m_transform_resp->field_delete(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE);
+    if (field != NULL)
+      m_transform_resp->field_delete(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE);
 
 
-  field = m_transform_resp->field_create(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE);
-  field->value_append(m_transform_resp->m_heap, m_transform_resp->m_mime, range_type, sizeof(range_type) - 1);
-
-  m_transform_resp->field_attach(field);
+    field = m_transform_resp->field_create(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE);
+    field->value_append(m_transform_resp->m_heap, m_transform_resp->m_mime, range_type, sizeof(range_type) - 1);
 
+    m_transform_resp->field_attach(field);
+  } else {
+    char numbers[RANGE_NUMBERS_LENGTH];
+    m_transform_resp->field_delete(MIME_FIELD_CONTENT_RANGE, MIME_LEN_CONTENT_RANGE);
+    field = m_transform_resp->field_create(MIME_FIELD_CONTENT_RANGE, MIME_LEN_CONTENT_RANGE);
+    snprintf(numbers, sizeof(numbers), "bytes %" PRId64"-%" PRId64"/%" PRId64, m_ranges[0]._start, m_ranges[0]._end, m_output_cl);
+    field->value_set(m_transform_resp->m_heap, m_transform_resp->m_mime, numbers, strlen(numbers));
+    m_transform_resp->field_attach(field);
+  }
 }
 
 #undef RANGE_NUMBERS_LENGTH

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1199e991/proxy/Transform.h
----------------------------------------------------------------------
diff --git a/proxy/Transform.h b/proxy/Transform.h
index af27870..bbe6bab 100644
--- a/proxy/Transform.h
+++ b/proxy/Transform.h
@@ -49,7 +49,7 @@ public:
 public:
   VConnection * open(Continuation * cont, APIHook * hooks);
   INKVConnInternal *null_transform(ProxyMutex * mutex);
-  INKVConnInternal *range_transform(ProxyMutex * mutex, RangeRecord * ranges, bool, int, HTTPHdr *, const char * content_type, int content_type_len, int64_t content_length);
+  INKVConnInternal *range_transform(ProxyMutex * mutex, RangeRecord * ranges, int, HTTPHdr *, const char * content_type, int content_type_len, int64_t content_length);
 };
 
 #ifdef TS_HAS_TESTS

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1199e991/proxy/TransformInternal.h
----------------------------------------------------------------------
diff --git a/proxy/TransformInternal.h b/proxy/TransformInternal.h
index 45781a7..8b03529 100644
--- a/proxy/TransformInternal.h
+++ b/proxy/TransformInternal.h
@@ -115,7 +115,7 @@ public:
 class RangeTransform:public INKVConnInternal
 {
 public:
-  RangeTransform(ProxyMutex * mutex, RangeRecord * ranges, bool, int, HTTPHdr *transform_resp, const char * content_type, int content_type_len, int64_t content_length);
+  RangeTransform(ProxyMutex * mutex, RangeRecord * ranges, int num_fields, HTTPHdr *transform_resp, const char * content_type, int content_type_len, int64_t content_length);
   ~RangeTransform();
 
   // void parse_range_and_compare();
@@ -135,8 +135,6 @@ public:
   // MIMEField *m_range_field;
   HTTPHdr *m_transform_resp;
   VIO *m_output_vio;
-  bool m_unsatisfiable_range;
-  // bool m_not_handle_range;
   int64_t m_range_content_length;
   int m_num_chars_for_cl;
   int m_num_range_fields;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1199e991/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 0c7a02f..0752500 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -3906,22 +3906,39 @@ HttpSM::do_hostdb_update_if_necessary()
   return;
 }
 
+/*
+ * range entry vaild [a,b] (a >= 0 and b >= 0 and a <= b)
+ * HttpTransact::RANGE_NONE if the content length of cached copy is zero or
+ * no range entry
+ * HttpTransact::RANGE_NOT_SATISFIABLE iff all range entrys are valid but
+ * none overlap the current extent of the cached copy
+ * HttpTransact::RANGE_NOT_HANDLED if out-of-order Range entrys or
+ * the cached copy`s content_length is INT64_MAX (e.g. read_from_writer and trunked)
+ * HttpTransact::RANGE_REQUESTED if all sub range entrys are valid and
+ * in order (remove the entrys that not overlap the extent of cache copy)
+ */
 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 = -1;
   const char *value;
   int value_len;
   int n_values;
   int nr = 0; // number of valid ranges, also index to range array.
+  int not_satisfy = 0;
   HdrCsvIter csv;
-  const char *s, *e;
+  const char *s, *e, *tmp;
+  RangeRecord *ranges = NULL;
+  int64_t start, end;
+
+  ink_debug_assert(field != NULL && t_state.range_setup == HttpTransact::RANGE_NONE && t_state.ranges == NULL);
 
   if (content_length <= 0)
     return;
-
-  ink_assert(field != NULL);
+  if (content_length == INT64_MAX) {
+    t_state.range_setup = HttpTransact::RANGE_NOT_HANDLED;
+    return;
+  }
 
   n_values = 0;
   value = csv.get_first(field, &value_len);
@@ -3930,105 +3947,112 @@ HttpSM::parse_range_and_compare(MIMEField *field, int64_t content_length)
     value = csv.get_next(&value_len);
   }
 
-  if (n_values <= 0)
+  value = csv.get_first(field, &value_len);
+  if (n_values <= 0 || ptr_len_ncmp(value, value_len, "bytes=", 6))
     return;
 
-  value = csv.get_first(field, &value_len);
+  ranges = NEW(new RangeRecord[n_values]);
+  value += 6; // skip leading 'bytes='
+  value_len -= 6;
 
-  // 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;
-        t_state.num_range_fields = -1;
-        return;
-      }
+  for (; value; value = csv.get_next(&value_len)) {
+    if (!(tmp = (const char *) memchr(value, '-', value_len))) {
+      t_state.range_setup = HttpTransact::RANGE_NONE;
+      goto Lfaild;
+    }
 
-      /* [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;
-      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[nr]._end = -1;
-      }
-      else {
-        t_state.ranges[nr]._end = mime_parse_int64(s, e);
-      }
+    // process start value
+    s = value;
+    e = tmp;
+    // skip leading white spaces
+    for (; s < e && ParseRules::is_ws(*s); ++s) ;
 
-      // check and change if necessary whether this is a right entry
-      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[nr]._start = content_length - t_state.ranges[nr]._end;
-        t_state.ranges[nr]._end = content_length - 1;
+    if (s >= e)
+      start = -1;
+    else {
+      for (start = 0; s < e && *s >= '0' && *s <= '9'; ++s)
+        start = start * 10 + (*s - '0');
+      // skip last white spaces
+      for (; s < e && ParseRules::is_ws(*s); ++s) ;
+
+      if (s < e || start < 0) {
+        t_state.range_setup = HttpTransact::RANGE_NONE;
+        goto Lfaild;
       }
-      // open end
-      else if (t_state.ranges[nr]._start >= 0 && t_state.ranges[nr]._end == -1) {
-          t_state.ranges[nr]._end = content_length - 1;
+    }
+
+    // process end value
+    s = tmp + 1;
+    e = value + value_len;
+    // skip leading white spaces
+    for (; s < e && ParseRules::is_ws(*s); ++s) ;
+
+    if (s >= e) {
+      if (start < 0) {
+        t_state.range_setup = HttpTransact::RANGE_NONE;
+        goto Lfaild;
+      } else if (start >= content_length) {
+        not_satisfy++;
+        continue;
       }
-      // "normal" Range - could be wrong if _end<_start
-      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;
+      end = content_length - 1;
+    } else {
+      for (end = 0; s < e && *s >= '0' && *s <= '9'; ++s)
+        end = end * 10 + (*s - '0');
+      // skip last white spaces
+      for (; s < e && ParseRules::is_ws(*s); ++s) ;
+
+      if (s < e || end < 0) {
+        t_state.range_setup = HttpTransact::RANGE_NONE;
+        goto Lfaild;
       }
-      // 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;
+
+      if (start < 0) {
+        if (end >= content_length)
+          end = content_length;
+        start = content_length - end;
+        end = content_length - 1;
+      } else if (start >= content_length && start <= end) {
+        not_satisfy++;
+        continue;
       }
 
-      // this is a good Range entry
-      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 = nr;
-        }
-        // currently we don't handle out-of-order Range entry
-        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;
-        }
+      if (end >= content_length)
+        end = content_length - 1;
+    }
 
-        prev_good_range = nr;
-        ++nr;
-      }
-      value = csv.get_next(&value_len);
+    if (start > end) {
+      t_state.range_setup = HttpTransact::RANGE_NONE;
+      goto Lfaild;
     }
+
+    if (prev_good_range >= 0 && start <= ranges[prev_good_range]._end) {
+      t_state.range_setup = HttpTransact::RANGE_NOT_HANDLED;
+      goto Lfaild;
+    }
+
+    ink_debug_assert(start >= 0 && end >= 0 && start < content_length && end < content_length);
+
+    prev_good_range = nr;
+    ranges[nr]._start = start;
+    ranges[nr]._end = end;
+    ++nr;
   }
-  // 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 {
+
+  if (nr > 0) {
+    t_state.range_setup = HttpTransact::RANGE_REQUESTED;
+    t_state.ranges = ranges;
     t_state.num_range_fields = nr;
+    return;
   }
+
+  if (not_satisfy)
+    t_state.range_setup = HttpTransact::RANGE_NOT_SATISFIABLE;
+
+Lfaild:
+  t_state.num_range_fields = -1;
+  delete []ranges;
+  return;
 }
 
 void
@@ -4036,9 +4060,12 @@ HttpSM::calculate_output_cl(int64_t content_length, int64_t num_chars)
 {
   int i;
 
-  if (t_state.unsatisfiable_range)
+  if (t_state.range_setup != HttpTransact::RANGE_REQUESTED &&
+      t_state.range_setup != HttpTransact::RANGE_NOT_TRANSFORM_REQUESTED)
     return;
 
+  ink_debug_assert(t_state.ranges);
+
   if (t_state.num_range_fields == 1) {
     t_state.range_output_cl = t_state.ranges[0]._end - t_state.ranges[0]._start + 1;
   }
@@ -4071,18 +4098,6 @@ HttpSM::do_range_parse(MIMEField *range_field)
   
   parse_range_and_compare(range_field, content_length);
   calculate_output_cl(content_length, num_chars_for_cl);
-  
-  if (t_state.unsatisfiable_range) {
-    t_state.range_setup = HttpTransact::RANGE_NOT_SATISFIABLE;
-    return;
-  }
-
-  if (t_state.not_handle_range) {
-    t_state.range_setup = HttpTransact::RANGE_NOT_HANDLED;
-    return;
-  }
-
-  t_state.range_setup = HttpTransact::RANGE_REQUESTED;
 }
 
 // this function looks for any Range: headers, parses them and either
@@ -4102,40 +4117,30 @@ HttpSM::do_range_setup_if_necessary()
   ink_assert(field != NULL);
   
   t_state.range_setup = HttpTransact::RANGE_NONE;
+
   if (t_state.method == HTTP_WKSIDX_GET && t_state.hdr_info.client_request.version_get() == HTTPVersion(1, 1)) {
     do_range_parse(field);
-    
+
+    // if only one range entry and pread is capable, no need transform range
+    if (t_state.range_setup == HttpTransact::RANGE_REQUESTED &&
+        t_state.num_range_fields == 1 &&
+        cache_sm.cache_read_vc->is_pread_capable())
+      t_state.range_setup = HttpTransact::RANGE_NOT_TRANSFORM_REQUESTED;
+
     if (t_state.range_setup == HttpTransact::RANGE_REQUESTED && 
-        (t_state.num_range_fields > 1 || !cache_sm.cache_read_vc->is_pread_capable()) &&
-        api_hooks.get(TS_HTTP_RESPONSE_TRANSFORM_HOOK) == NULL
-       ) 
-    {
-          Debug("http_trans", "Unable to accelerate range request, fallback to transform");
-          content_type = t_state.cache_info.object_read->response_get()->value_get(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE, &field_content_type_len);
-          //create a Range: transform processor for requests of type Range: bytes=1-2,4-5,10-100 (eg. multiple ranges)
-          range_trans = transformProcessor.range_transform(mutex, 
-                  t_state.ranges,
-                  t_state.unsatisfiable_range, 
-                  t_state.num_range_fields, 
-                  &t_state.hdr_info.transform_response,
-                  content_type,
-                  field_content_type_len,
-                  t_state.cache_info.object_read->object_size_get()
-                  );
-          if (range_trans != NULL) {
-            api_hooks.append(TS_HTTP_RESPONSE_TRANSFORM_HOOK, range_trans);
-            t_state.range_setup = HttpTransact::RANGE_REQUESTED;
-          }
-          else { //we couldnt append the transform to our API hooks so bailing
-            t_state.range_setup = HttpTransact::RANGE_NOT_SATISFIABLE;
-          } 
-    }
-    else if (t_state.range_setup == HttpTransact::RANGE_REQUESTED && t_state.num_range_fields == 1) {
-      Debug("http_trans", "Handling single Range: request");
-      //no op, we will handle this later in the HttpTunnel
-    }
-    else {
-      t_state.range_setup = HttpTransact::RANGE_NOT_SATISFIABLE;
+        api_hooks.get(TS_HTTP_RESPONSE_TRANSFORM_HOOK) == NULL) {
+      Debug("http_trans", "Unable to accelerate range request, fallback to transform");
+      content_type = t_state.cache_info.object_read->response_get()->value_get(MIME_FIELD_CONTENT_TYPE, MIME_LEN_CONTENT_TYPE, &field_content_type_len);
+      //create a Range: transform processor for requests of type Range: bytes=1-2,4-5,10-100 (eg. multiple ranges)
+      range_trans = transformProcessor.range_transform(mutex,
+          t_state.ranges,
+          t_state.num_range_fields,
+          &t_state.hdr_info.transform_response,
+          content_type,
+          field_content_type_len,
+          t_state.cache_info.object_read->object_size_get()
+          );
+      api_hooks.append(TS_HTTP_RESPONSE_TRANSFORM_HOOK, range_trans);
     }
   }
 }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1199e991/proxy/http/HttpTransact.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 38ed2e7..24948fa 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -2597,26 +2597,7 @@ HttpTransact::HandleCacheOpenReadHit(State* s)
 
       build_request(s, &s->hdr_info.client_request, &s->hdr_info.server_request, s->current.server->http_version);
 
-      /* We can't handle revalidate requests with ranges. If it comes
-         back unmodified that's fine but if not the OS will either
-         provide the entire object which is difficult to handle and
-         potentially very inefficient, or we'll get back just the
-         range and we can't cache that anyway. So just forward the
-         request.
-
-         Note: We're here only if the cache is stale so we don't want
-         to serve it.
-      */
-      if (s->hdr_info.client_request.presence(MIME_PRESENCE_RANGE)) {
-        Debug("http_trans", "[CacheOpenReadHit] Forwarding range request for stale cache object.");
-        s->cache_info.action = CACHE_DO_NO_ACTION;
-        s->cache_info.object_read = NULL;
-      } else {
-        // If it's not a range tweak the request to be a revalidate.
-        // Note this doesn't actually issue the request, it simply
-        // adjusts the headers for later.
-        issue_revalidate(s);
-      }
+      issue_revalidate(s);
 
       // this can not be anything but a simple origin server connection.
       // in other words, we would not have looked up the cache for a
@@ -6243,8 +6224,8 @@ HttpTransact::is_response_cacheable(State* s, HTTPHdr* request, HTTPHdr* respons
     }
   }
   // do not cache partial content - Range response
-  if (response_code == HTTP_STATUS_PARTIAL_CONTENT) {
-    DebugTxn("http_trans", "[is_response_cacheable] " "Partial content response - don't cache");
+  if (response_code == HTTP_STATUS_PARTIAL_CONTENT || response_code == HTTP_STATUS_RANGE_NOT_SATISFIABLE) {
+    DebugTxn("http_trans", "[is_response_cacheable] " "response code %d - don't cache", response_code);
     return false;
   }
 
@@ -6746,11 +6727,16 @@ HttpTransact::handle_content_length_header(State* s, HTTPHdr* header, HTTPHdr* b
         ink_debug_assert(header->get_content_length() == cl);
 
         switch (s->source) {
+        case SOURCE_HTTP_ORIGIN_SERVER:
+          // We made our decision about whether to trust the
+          //   response content length in init_state_vars_from_response()
+          if (s->range_setup != HttpTransact::RANGE_NOT_TRANSFORM_REQUESTED)
+            break;
         case SOURCE_CACHE:
           
           // if we are doing a single Range: request, calculate the new
           // C-L: header
-          if (s->range_setup == HttpTransact::RANGE_REQUESTED && s->num_range_fields == 1) {
+          if (s->range_setup == HttpTransact::RANGE_NOT_TRANSFORM_REQUESTED) {
             Debug("http_trans", "Partial content requested, re-calculating content-length");
             change_response_header_because_of_range_request(s,header);
             s->hdr_info.trust_response_cl = true;
@@ -6768,10 +6754,6 @@ HttpTransact::handle_content_length_header(State* s, HTTPHdr* header, HTTPHdr* b
             s->hdr_info.trust_response_cl = false;
           }
           break;
-        case SOURCE_HTTP_ORIGIN_SERVER:
-          // We made our decision about whether to trust the
-          //   response content length in init_state_vars_from_response()
-          break;
         case SOURCE_TRANSFORM:
           if (s->hdr_info.transform_response_cl == HTTP_UNDEFINED_CL) {
             s->hdr_info.trust_response_cl = false;
@@ -6801,8 +6783,9 @@ HttpTransact::handle_content_length_header(State* s, HTTPHdr* header, HTTPHdr* b
           header->field_delete(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH);
           s->hdr_info.trust_response_cl = false;
           s->hdr_info.request_content_length = HTTP_UNDEFINED_CL;
+          ink_assert(s->range_setup == RANGE_NONE);
         } 
-        else if (s->range_setup == HttpTransact::RANGE_REQUESTED && s->num_range_fields == 1) {
+        else if (s->range_setup == RANGE_NOT_TRANSFORM_REQUESTED) {
           // if we are doing a single Range: request, calculate the new
           // C-L: header
           Debug("http_trans", "Partial content requested, re-calculating content-length");
@@ -6826,7 +6809,7 @@ HttpTransact::handle_content_length_header(State* s, HTTPHdr* header, HTTPHdr* b
           s->hdr_info.trust_response_cl = false;
         }
         header->field_delete(MIME_FIELD_CONTENT_LENGTH, MIME_LEN_CONTENT_LENGTH);
-
+        ink_assert(s->range_setup != RANGE_NOT_TRANSFORM_REQUESTED);
       }
     }
   } else if (header->type_get() == HTTP_TYPE_REQUEST) {
@@ -8955,13 +8938,12 @@ HttpTransact::change_response_header_because_of_range_request(State *s, HTTPHdr
 
     header->field_attach(field);
     header->set_content_length(s->range_output_cl);
-  }
-  else {
+  } else {
     char numbers[RANGE_NUMBERS_LENGTH];
-
+    header->field_delete(MIME_FIELD_CONTENT_RANGE, MIME_LEN_CONTENT_RANGE);
     field = header->field_create(MIME_FIELD_CONTENT_RANGE, MIME_LEN_CONTENT_RANGE);
     snprintf(numbers, sizeof(numbers), "bytes %" PRId64"-%" PRId64"/%" PRId64, s->ranges[0]._start, s->ranges[0]._end, s->cache_info.object_read->object_size_get());
-    field->value_append(header->m_heap, header->m_mime, numbers, strlen(numbers));
+    field->value_set(header->m_heap, header->m_mime, numbers, strlen(numbers));
     header->field_attach(field);
     header->set_content_length(s->range_output_cl);
   }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1199e991/proxy/http/HttpTransact.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 688b1f1..adb06e7 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -655,6 +655,7 @@ public:
     RANGE_REQUESTED,
     RANGE_NOT_SATISFIABLE,
     RANGE_NOT_HANDLED,
+    RANGE_NOT_TRANSFORM_REQUESTED
   };
   
   enum CacheAuth_t
@@ -1070,11 +1071,8 @@ public:
     
     // Http Range: related variables
     RangeSetup_t range_setup;
-    bool unsatisfiable_range;
-    bool not_handle_range;
     int64_t num_range_fields;
     int64_t range_output_cl;
-    int64_t current_range;
     RangeRecord *ranges;
     
     OverridableHttpConfigParams *txn_conf;
@@ -1170,11 +1168,8 @@ public:
         pristine_url(),
         api_skip_all_remapping(false),
         range_setup(RANGE_NONE),
-        unsatisfiable_range(false),
-        not_handle_range(false),
         num_range_fields(0),
         range_output_cl(0),
-        current_range(-1),
         ranges(NULL),
         txn_conf(NULL),
         transparent_passthrough(false)
@@ -1266,9 +1261,9 @@ public:
       arena.reset();
       pristine_url.clear();
 
-      delete[] ranges; ranges = NULL;
+      delete[] ranges;
+      ranges = NULL;
       range_setup = RANGE_NONE;
-      unsatisfiable_range = true;
       return;
     }
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1199e991/proxy/http/HttpTunnel.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
index 73a89ef..9db3df2 100644
--- a/proxy/http/HttpTunnel.cc
+++ b/proxy/http/HttpTunnel.cc
@@ -798,7 +798,8 @@ HttpTunnel::producer_run(HttpTunnelProducer * p)
   }
 
   int64_t read_start_pos = 0;
-  if (p->vc_type == HT_CACHE_READ && sm->t_state.range_setup == HttpTransact::RANGE_REQUESTED && sm->t_state.num_range_fields == 1) {
+  if (p->vc_type == HT_CACHE_READ && sm->t_state.range_setup == HttpTransact::RANGE_NOT_TRANSFORM_REQUESTED) {
+    ink_debug_assert(sm->t_state.num_range_fields == 1); // we current just support only one range entry
     read_start_pos = sm->t_state.ranges[0]._start;
     producer_n = (sm->t_state.ranges[0]._end - sm->t_state.ranges[0]._start)+1;
     consumer_n = (producer_n + sm->client_response_hdr_bytes);