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

On 04/30/2014 09:33 PM, 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.

OK, this is clarifies what we should do for ecap.

Before the patch r13384 for the libecap::FirstLine::protocol() returned
the protocol information from url, which now looks wrong.

>
> 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.

If I am not wrong, currently the protocol in HttpMsg::http_ver is
always AnyP::PROTO_HTTP.
I think this is normal because we are always handling HTTP messages.

>
> 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.

Maybe we need to modify:
 - Adaptation::Ecap::FirstLineRep::protocol to return the
HttpMsg::http_ver protocol information
 - Adaptation::Ecap::FirstLineRep::protocol(const Name &p) to set
HttpMsg::http_ver
 - Adaptation::Ecap::RequestLineRep::protocol to return the url protocol
information.
 - Adaptation::Ecap::RequestLineRep::protocol(const Name &p) to set
HttpRequest url protocol information (or something equivalent).

The Adaptation::Ecap::RequestLineRep::protocol isn't it referring to the
URL protocol (http://, https://, ftp://, etc)?
Else eCAP services need to re-parse URL to retrieve this information...

>
> 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?
>
>
> Thank you,
>
> Alex.
>
>
Received on Thu May 01 2014 - 10:21:16 MDT