Here's the patch - one extra line necessary to make the comparison actually 
work.  Shall I file a squid bug for ecap modules removing Content-Length with 
this patch attached?

After patching, squid properly caches an adapted response with a different 
content length from the virgin response.  With Vary: Accept-Encoding coming 
from the underlying webserver, I get N variants cached, which is expected for 
just the basic Vary support in squid 3, I believe.


diff -urp squid-3.1.10/src/adaptation/ecap/Host.cc 
squid-3.1.10-ecap-patched/src/adaptation/ecap/Host.cc
--- squid-3.1.10/src/adaptation/ecap/Host.cc    2010-12-22 00:46:56.000000000 
-0500
+++ squid-3.1.10-ecap-patched/src/adaptation/ecap/Host.cc       2011-02-03 
13:27:41.000000000 -0500
@@ -25,6 +25,7 @@ Adaptation::Ecap::Host::Host()
     // this code can run only once
 
     libecap::headerReferer.assignHostId(HDR_REFERER);
+    libecap::headerContentLength.assignHostId(HDR_CONTENT_LENGTH);
 
     libecap::protocolHttp.assignHostId(PROTO_HTTP);
     libecap::protocolHttps.assignHostId(PROTO_HTTPS);
diff -urp squid-3.1.10/src/adaptation/ecap/MessageRep.cc 
squid-3.1.10-ecap-patched/src/adaptation/ecap/MessageRep.cc
--- squid-3.1.10/src/adaptation/ecap/MessageRep.cc      2010-12-22 
00:46:56.000000000 -0500
+++ squid-3.1.10-ecap-patched/src/adaptation/ecap/MessageRep.cc 2011-02-03 
13:29:48.000000000 -0500
@@ -48,6 +48,9 @@ Adaptation::Ecap::HeaderRep::add(const N
     HttpHeaderEntry *e = new HttpHeaderEntry(squidId, name.image().c_str(),
             value.toString().c_str());
     theHeader.addEntry(e);
+
+    if (squidId == HDR_CONTENT_LENGTH)
+        theMessage.content_length = theHeader.getInt64(HDR_CONTENT_LENGTH);
 }
 
 void
@@ -58,6 +61,9 @@ Adaptation::Ecap::HeaderRep::removeAny(c
         theHeader.delByName(name.image().c_str());
     else
         theHeader.delById(squidId);
+
+    if (squidId == HDR_CONTENT_LENGTH)
+        theMessage.content_length = theHeader.getInt64(HDR_CONTENT_LENGTH);
 }
 
 libecap::Area


Regards,
Jonathan Wolfe

On Feb 2, 2011, at 8:44 PM, Amos Jeffries wrote:

> On Wed, 2 Feb 2011 20:09:05 -0800, Jonathan Wolfe wrote:
>> Oops, here's the diff from Alex which I forgot to include below:
>> 
>> === modified file 'src/adaptation/ecap/MessageRep.cc'
>> --- src/adaptation/ecap/MessageRep.cc 2010-05-26 03:06:02 +0000
>> +++ src/adaptation/ecap/MessageRep.cc 2011-01-26 21:43:36 +0000
>> @@ -44,24 +44,30 @@
>> void
>> Adaptation::Ecap::HeaderRep::add(const Name &name, const Value &value)
>> {
>>     const http_hdr_type squidId = TranslateHeaderId(name); // HDR_OTHER
> OK
>>     HttpHeaderEntry *e = new HttpHeaderEntry(squidId,
>>     name.image().c_str(),
>>             value.toString().c_str());
>>     theHeader.addEntry(e);
>> +
>> + // XXX: this is too often and not enough, on many levels
>> + theMessage.content_length = theHeader.getInt64(HDR_CONTENT_LENGTH);
>> }
>> 
>> void
>> Adaptation::Ecap::HeaderRep::removeAny(const Name &name)
>> {
>>     const http_hdr_type squidId = TranslateHeaderId(name);
>>     if (squidId == HDR_OTHER)
>>         theHeader.delByName(name.image().c_str());
>>     else
>>         theHeader.delById(squidId);
>> +
>> + // XXX: this is too often and not enough, on many levels
>> + theMessage.content_length = theHeader.getInt64(HDR_CONTENT_LENGTH);
>> }
>> 
>> libecap::Area
>> Adaptation::Ecap::HeaderRep::image() const
>> {
>>     MemBuf mb;
>>     mb.init();
>> 
>> 
>> This only works, I think, because when removing Content-Length we end up
>> with a -1 in the length for the whole message, instead of the old (too
>> large) content length.  (There's a
>> Adaptation::Ecap::XactionRep::setAdaptedBodySize function that's
> commented
>> out in the eCAP support, presumably because it's common to not know the
>> length of the final adapted body, like in the gzip case.)
> 
> 
> I think that patch should be:
> 
> if (squidId == HDR_CONTENT_LENGTH)
>    theMessage.content_length = theHeader.getInt64(HDR_CONTENT_LENGTH);
> 
> in both places to avoid that "too often" case.
> 
>> 
>> There are a few options - assume the content-length for adapted messages
>> is just the total object_len - hdr_sz, leave it always -1 unless the
> eCAP
>> module tells you otherwise by setting the Content-Length header maybe,
> or
>> finally, store it by accumulating the size of returned chunks along the
> way
>> receiving the adapted body and set it accordingly when there's no more
>> adapted body to receive.
> 
> We almost always end up with problems caused by large bodies extending
> past any possible buffer window. So sending this headers after object
> completion is not an available choice.
> 
> AFAIK if the adaptor sends its reply with chunked encoding and no
> Content-Length header at all it would work out. Squid should handle the
> dechunking and connection termination requirements for old clients.
> 
> Amos
> 

Reply via email to