Re: /bzr/squid3/trunk/ r13384: Bug 1961: pt1: URL handling redesign

On 04/30/2014 01:14 PM, Amos Jeffries wrote:
> On 1/05/2014 6:33 a.m., Alex Rousskov wrote:
>> On 28/04/2014 5:35 a.m., Tsantilas Christos wrote:
>>> Unfortunately this is not build with ecap.
>>
>>
>> On 04/27/2014 07:57 PM, Amos Jeffries wrote:
>>> What the eCAP field needs to be set to depends on its definition:
>>
>>
>> libecap::FirstLine::protocol() is meant for things like
>>
>> * the HTTP-Version part of HTTP messages (in RFC 2616 terminology) or
>> * the ICAP-Version part of ICAP messages (in RFC 3507 terminology).
>>
>> It is not related to the URI. In fact, libecap::FirstLine does not know
>> about the message URI! Only its libecap::RequestLine child knows that
>> requests have URIs.
>>
>> HttpMsg::http_ver in recent trunk is described as being meant for the
>> same thing. Thus, there is a perfect match between the two concepts.
>>
>>
>> Now, we need to understand how the actual code maps to the above design,
>> and what code changes are needed to fix the trunk build broken by
>> r13384. My understanding is that
>>
>> 1. If Squid trunk does not set HttpMsg::http_ver [correctly] when
>> parsing any requests or responses, we should fix the corresponding Squid
>> code. This will ensure that eCAP adapters are getting the right
>> information about virgin messages. This item requires an investigation
>> and may not require any code changes.
>>
>> 2. If Squid trunk does not use HttpMsg::http_ver [correctly] when
>> formatting any requests or responses, we should fix the corresponding
>> Squid code. This will ensure that eCAP adapters can adapt virgin
>> messages as needed. Note that Squid does not support post-cache
>> vectoring points so Squid should still set its own http_ver in any
>> outgoing messages. This item requires an investigation and may not
>> require any code changes.
>>
>> 3. Squid eCAP implementation should be changed to read and write
>> HttpMsg::http_ver. This affects two
>> Adaptation::Ecap::FirstLineRep::protocol methods and possibly other
>> code. This item requires code changes.
>>
>> There is no need to change eCAP APIs. Some existing adapters might be
>> inconvenienced by this Squid change, but those adapters are probably
>> using the wrong interface anyway, and no such cases are known to exist.
>>
>> If the above is accurate, Amos, would you like to work on any of the
>> above three items?
>
>
> There will also need to be review of internally generated requests,
> including the URL rewriter generated clone request. They use Squid
> default version on output but what the state of it is up to then I don't
> know.
>
>
> I have kind of being doing #1 already in my parser-ng work and happy to
> take that on and then #2 unless someone else gets it done first.

Sounds good. Please feel free to revert r13384 until this is fixed.

Meanwhile, either Christos or I will change the Squid eCAP code to use
http_ver (#3) unless Christos find a flaw in that plan.

Thank you,

Alex.
Received on Wed Apr 30 2014 - 19:36:56 MDT