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

On 05/01/2014 04:21 AM, Tsantilas Christos wrote:
> On 04/30/2014 09:33 PM, Alex Rousskov wrote:
>> 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.

Agreed.

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

There is also ICY IIRC. I do not recall whether we use FTP/1.0 in
ftp-gw, but FTP would become another possible value if we do.

> Maybe we need to modify:
> - Adaptation::Ecap::FirstLineRep::protocol to return the
> HttpMsg::http_ver protocol information

Yes.

> - Adaptation::Ecap::FirstLineRep::protocol(const Name &p) to set
> HttpMsg::http_ver

Yes.

> - Adaptation::Ecap::RequestLineRep::protocol to return the url protocol
> information.

No. The FirstLine protocol method is not about the URI, regardless of
the specific message kind.

> - Adaptation::Ecap::RequestLineRep::protocol(const Name &p) to set
> HttpRequest url protocol information (or something equivalent).

No, for the same reason. If an adapter wants to change the URI, they
should change the URI.

> The Adaptation::Ecap::RequestLineRep::protocol isn't it referring to the
> URL protocol (http://, https://, ftp://, etc)?

No. The meaning of the method is defined by FirstLine. Changing that
meaning in FirstLine kids would be wrong from API point of view.

> Else eCAP services need to re-parse URL to retrieve this information...

Yes, they should. Future libecap versions may add an API to manipulate
the URI. Until then, the adapters are on their own.

Cheers,

Alex.
Received on Fri May 02 2014 - 14:48:54 MDT