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
>
