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.)
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.
Any suggestions for the best route to pursue?
Regards,
Jonathan Wolfe
On Jan 26, 2011, at 2:35 PM, Jonathan Wolfe wrote:
> Okay, I narrowed this down a bit more with some help from Alex Rousskov:
>
> When it works (doing a string replace from "asdf" to "fdsa" for example, so
> same total content length):
>
> 2011/01/26 16:07:21.312| storeEntryValidLength: Checking
> '1078B4E8EC1D17CFEBCD533EE19F7FD6'
> 2011/01/26 16:07:21.312| storeEntryValidLength: object_len = 20366
> 2011/01/26 16:07:21.312| storeEntryValidLength: hdr_sz = 360
> 2011/01/26 16:07:21.312| storeEntryValidLength: content_length = 20006
> 2011/01/26 16:07:21.317| StoreEntry::setMemStatus: inserted mem node
> http://www.example.com/squid-test
>
> When it doesn't work ("asdf" to just "a"):
>
> 2011/01/26 16:05:59.878| storeEntryValidLength: Checking
> '1078B4E8EC1D17CFEBCD533EE19F7FD6'
> 2011/01/26 16:05:59.878| storeEntryValidLength: object_len = 14843
> 2011/01/26 16:05:59.878| storeEntryValidLength: hdr_sz = 360
> 2011/01/26 16:05:59.878| storeEntryValidLength: content_length = 20006
> 2011/01/26 16:05:59.878| storeEntryValidLength: 5523 bytes too small;
> '1078B4E8EC1D17CFEBCD533EE19F7FD6'
> 2011/01/26 16:05:59.879| StoreEntry::checkCachable: NO: wrong content-length
>
> The headers returned in both cases don't actually include a Content-Length
> header, which is removed by the module using adapted->header().removeAny.
>
> It looks like squid is restoring the content length in the second case, and
> declaring it too small.
>
> See https://answers.launchpad.net/ecap/+question/142965 for my discussion
> with Alex on this. The diff he provided, which is repeated here, seems to
> have the effect of setting the message content length to -1 when removing the
> content length header from within the ecap module, and that results in this:
>
> 2011/01/26 17:21:46.539| storeEntryValidLength: Checking
> '1078B4E8EC1D17CFEBCD533EE19F7FD6'
> 2011/01/26 17:21:46.539| storeEntryValidLength: object_len = 16190
> 2011/01/26 17:21:46.539| storeEntryValidLength: hdr_sz = 360
> 2011/01/26 17:21:46.539| storeEntryValidLength: content_length = -1
> 2011/01/26 17:21:46.539| storeEntryValidLength: Unspecified content length:
> 1078B4E8EC1D17CFEBCD533EE19F7FD6
> 2011/01/26 17:21:46.544| StoreEntry::setMemStatus: inserted mem node
> http://www.example.com/squid-test
>
> Not the best behavior, but it does cache as expected now.
>
> Likely there's a better place to reset the content length, right? Perhaps in
> src/adaptation/ecap/XactionRep.cc, in moveAbContent() when we've received the
> full adapted body?
>
> Regards,
> -Jon
>
> On Jan 23, 2011, at 8:46 PM, Amos Jeffries wrote:
>
>> On 24/01/11 13:43, Henrik Nordström wrote:
>>> lör 2011-01-22 klockan 23:04 +1300 skrev Amos Jeffries:
>>>
>>>> Squid caches only one of N variants so the expected behviour is that
>>>> each new variant is a MISS but becomes a HIT on repeated duplicate
>>>> requests until a new variant pushes it out of cache.
>>>
>>> No it caches all N variants seen if the origin response has Vary:
>>>
>>> But not sure what happens with the gzip eCAP module in this regard.
>>
>> AFAIK, that proper variant handling was not yet ported to squid-3. Only in
>> squid-2 right now.
>> This identical behaviour is causing some problems with recent Chrome using
>> sdch encoding. Thus clashing with the gzip|deflate cached variant from other
>> browsers.
>>
>> Though yes the adapter output seems to be borked anyway.
>>
>> Amos
>> --
>> Please be using
>> Current Stable Squid 2.7.STABLE9 or 3.1.10
>> Beta testers wanted for 3.2.0.4
>
