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

OK.
Looks that the attached patch should be enough to solve trunk with ecap
build.
If no problem I will apply to trunk.

Regards,
   Christos

On 05/02/2014 05:48 PM, Alex Rousskov wrote:
> 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.
>
>