Bug #624: Various assertion errors on Range requests

use reply range spec if does not match request range spec (forward as-is)

This assertion is triggered if request range is not satisified by reply.  In
this case, we simply forward the reply as-is.
To forward as-is, we override client_side reply range spec with server_side
reply range spec.

if reply has ranges but request is not: we forward as-is.

Another situation where this assertion is triggered:  if reply is partial
content (status 206) but request is not.  This patch forward as-is the
offending response received on server_side.

diff --git a/src/HttpHdrRange.cc b/src/HttpHdrRange.cc
index 1c52ae8a49466af07a0ec5217884d17ecd7fd8e4..7c44a51844ddf55b49748bf1daf77a725a061983 100644 (file)
--- a/src/HttpHdrRange.cc
+++ b/src/HttpHdrRange.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: HttpHdrRange.cc,v 1.39 2004/08/30 05:12:31 robertc Exp $
+ * $Id: HttpHdrRange.cc,v 1.40 2006/06/06 19:22:13 hno Exp $
  *
  * DEBUG: section 64    HTTP Range Header
  * AUTHOR: Alex Rousskov
@@ -552,6 +552,23 @@ HttpHdrRange::offsetLimitExceeded() const
     return true;
 }
 
+bool
+HttpHdrRange::contains(HttpHdrRangeSpec& r) const
+{
+    assert(r.length >= 0);
+    HttpHdrRangeSpec::HttpRange rrange(r.offset, r.offset + r.length);
+
+    for (const_iterator i = begin(); i != end(); ++i) {
+        HttpHdrRangeSpec::HttpRange irange((*i)->offset, (*i)->offset + (*i)->length);
+        HttpHdrRangeSpec::HttpRange intersection = rrange.intersection(irange);
+
+        if (intersection.start == irange.start && intersection.size() == irange.size())
+            return true;
+    }
+
+    return false;
+}
+
 const HttpHdrRangeSpec *
 HttpHdrRangeIter::currentSpec() const
 {

diff --git a/src/HttpHeaderRange.h b/src/HttpHeaderRange.h
index 65a8e4a63582b4558ecb76534457e39256905bf3..6e6a3cff3b9bbbb88c9d2f9724bf7a8d3715fe3d 100644 (file)
--- a/src/HttpHeaderRange.h
+++ b/src/HttpHeaderRange.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: HttpHeaderRange.h,v 1.7 2006/04/22 05:29:18 robertc Exp $
+ * $Id: HttpHeaderRange.h,v 1.8 2006/06/06 19:22:13 hno Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -104,6 +104,7 @@ public:
     ssize_t firstOffset() const;
     ssize_t lowestOffset(ssize_t) const;
     bool offsetLimitExceeded() const;
+    bool contains(HttpHdrRangeSpec& r) const;
     Vector<HttpHdrRangeSpec *> specs;
 
 private:

diff --git a/src/client_side.cc b/src/client_side.cc
index 8cd22df968fd8c6e1d9b3c3a3fd9b4466bd1a716..f1eb5dcaa1f67e5e00bebf4c692352b75f174225 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -1,6 +1,6 @@
 
 /*
- * $Id: client_side.cc,v 1.726 2006/05/26 19:58:37 wessels Exp $
+ * $Id: client_side.cc,v 1.727 2006/06/06 19:22:13 hno Exp $
  *
  * DEBUG: section 33    Client-side Routines
  * AUTHOR: Duane Wessels
@@ -290,7 +290,7 @@ ClientSocketContext::connIsFinished()
     clientStreamDetach(getTail(), http);
 }
 
-ClientSocketContext::ClientSocketContext() : http(NULL), next(NULL),
+ClientSocketContext::ClientSocketContext() : http(NULL), reply(NULL), next(NULL),
         writtenToSocket(0),
         mayUseConnection_ (false),
         connRegistered_ (false)
@@ -1114,6 +1114,12 @@ ClientSocketContext::buildRangeHeader(HttpReply * rep)
         /* XXX: TODO: Review, this unconditional set may be wrong. - TODO: review. */
         httpStatusLineSet(&rep->sline, rep->sline.version,
                           HTTP_PARTIAL_CONTENT, NULL);
+        // web server responded with a valid, but unexpected range.
+        // will (try-to) forward as-is.
+        //TODO: we should cope with multirange request/responses
+        bool replyMatchRequest = rep->content_range != NULL ?
+                                 request->range->contains(rep->content_range->spec) :
+                                 true;
         const int spec_count = http->request->range->specs.count;
         int actual_clen = -1;
 
@@ -1125,20 +1131,30 @@ ClientSocketContext::buildRangeHeader(HttpReply * rep)
         /* append appropriate header(s) */
 
         if (spec_count == 1) {
-            HttpHdrRange::iterator pos = http->request->range->begin();
-            assert(*pos);
-            /* append Content-Range */
+            if (!replyMatchRequest) {
+                hdr->delById(HDR_CONTENT_RANGE);
+                hdr->putContRange(rep->content_range);
+                actual_clen = rep->content_length;
+                //http->range_iter.pos = rep->content_range->spec.begin();
+                (*http->range_iter.pos)->offset = rep->content_range->spec.offset;
+                (*http->range_iter.pos)->length = rep->content_range->spec.length;
 
-            if (!hdr->has(HDR_CONTENT_RANGE)) {
-                /* No content range, so this was a full object we are
-                 * sending parts of.
-                 */
-                httpHeaderAddContRange(hdr, **pos, rep->content_length);
+            } else {
+                HttpHdrRange::iterator pos = http->request->range->begin();
+                assert(*pos);
+                /* append Content-Range */
+
+                if (!hdr->has(HDR_CONTENT_RANGE)) {
+                    /* No content range, so this was a full object we are
+                     * sending parts of.
+                     */
+                    httpHeaderAddContRange(hdr, **pos, rep->content_length);
+                }
+
+                /* set new Content-Length to the actual number of bytes
+                 * transmitted in the message-body */
+                actual_clen = (*pos)->length;
             }
-
-            /* set new Content-Length to the actual number of bytes
-             * transmitted in the message-body */
-            actual_clen = (*pos)->length;
         } else {
             /* multipart! */
             /* generate boundary string */
@@ -1172,6 +1188,8 @@ ClientSocketContext::buildRangeHeader(HttpReply * rep)
 void
 ClientSocketContext::prepareReply(HttpReply * rep)
 {
+    reply = rep;
+
     if (http->request->range)
         buildRangeHeader(rep);
 }
@@ -1434,6 +1452,11 @@ ClientSocketContext::getNextRangeOffset() const
             return start;
         }
 
+    } else if (reply && reply->content_range) {
+        /* request does not have ranges, but reply does */
+        //FIXME: should use range_iter_pos on reply, as soon as reply->content_range
+        //       becomes HttpHdrRange rather than HttpHdrRangeSpec.
+        return http->out.offset + reply->content_range->spec.offset;
     }
 
     return http->out.offset;
@@ -1475,6 +1498,16 @@ ClientSocketContext::socketState()
                 else
                     return STREAM_UNPLANNED_COMPLETE;
             }
+        } else if (reply && reply->content_range) {
+            /* reply has content-range, but request did not */
+
+            if (http->memObject()->endOffset() <=
+                    reply->content_range->spec.offset + reply->content_range->spec.length) {
+                if (http->request->flags.proxy_keepalive)
+                    return STREAM_COMPLETE;
+                else
+                    return STREAM_UNPLANNED_COMPLETE;
+            }
         }
 
         return STREAM_NONE;

diff --git a/src/client_side.h b/src/client_side.h
index 11316e302121e5e50adf728f090cee042ee57cbc..60db5e7260d9c3cbb16b042347ee550b9f0e8ad7 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -1,6 +1,6 @@
 
 /*
- * $Id: client_side.h,v 1.15 2006/04/27 19:27:37 wessels Exp $
+ * $Id: client_side.h,v 1.16 2006/06/06 19:22:13 hno Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -61,6 +61,7 @@ public:
     void writeComplete(int fd, char *bufnotused, size_t size, comm_err_t errflag);
     void keepaliveNextRequest();
     ClientHttpRequest *http;   /* we own this */
+    HttpReply *reply;
     char reqbuf[HTTP_REQBUF_SZ];
     Pointer next;