Re: [PATCH] ecap canonical url invalidation during reqmod

On 17/01/11 21:14, Leonid Evdokimov wrote:
>> Can you explain why this can't simply always do
>> safe_free(theMessage.canonical) before re-parsing?
>
> I've done this patch to be as non-invasive regarding current logic as
> possible, so I replicated the logic from ICAP code to eCap code.
>
> I think, that the really proper place to
> safe_free(theMessage.canonical) is marked with FIXME line in my patch
> - but I'm not 100% sure about that point .

I think its a no for there. The HttpRequest object there should be empty
anyway at that point.

>
> Anyway, the patch is required to fix a bug when precache ecap service
> can't properly rewrite URL (url is rewritten but `canonical` value is
> stale so request to disk-cache is done with incorrect URL) and it
> fixes it.

Oh, thanks for pointing ICAP out. The usage in there and elsewhere seems
a bit wrong. The canonical field should not be filled or copied
directly, but always generated by urlCanonical(request_object).

The way urlCanonical() works its safe to erase the cached version at any
point. Just slows Squid down re-calculating it on next use. There may be
code not using it right though.

I think we should actually do it in url.cc. Removing request->canonical
at the start of urlParse() and (maybe) setting it in urlParseFinish().
This will resolve both eCAP and ICAP.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.10
   Beta testers wanted for 3.2.0.4