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

=== modified file 'src/adaptation/ecap/MessageRep.cc'
--- src/adaptation/ecap/MessageRep.cc	2014-04-27 07:59:17 +0000
+++ src/adaptation/ecap/MessageRep.cc	2014-05-02 17:20:05 +0000
@@ -118,41 +118,41 @@
 }
 
 libecap::Version
 Adaptation::Ecap::FirstLineRep::version() const
 {
     return libecap::Version(theMessage.http_ver.major,
                             theMessage.http_ver.minor);
 }
 
 void
 Adaptation::Ecap::FirstLineRep::version(const libecap::Version &aVersion)
 {
     theMessage.http_ver.major = aVersion.majr;
     theMessage.http_ver.minor = aVersion.minr;
 }
 
 libecap::Name
 Adaptation::Ecap::FirstLineRep::protocol() const
 {
     // TODO: optimize?
-    switch (theMessage.protocol) {
+    switch (theMessage.http_ver.protocol) {
     case AnyP::PROTO_HTTP:
         return libecap::protocolHttp;
     case AnyP::PROTO_HTTPS:
         return libecap::protocolHttps;
     case AnyP::PROTO_FTP:
         return libecap::protocolFtp;
     case AnyP::PROTO_GOPHER:
         return libecap::protocolGopher;
     case AnyP::PROTO_WAIS:
         return libecap::protocolWais;
     case AnyP::PROTO_WHOIS:
         return libecap::protocolWhois;
     case AnyP::PROTO_URN:
         return libecap::protocolUrn;
     case AnyP::PROTO_ICP:
         return protocolIcp;
 #if USE_HTCP
     case AnyP::PROTO_HTCP:
         return protocolHtcp;
 #endif
@@ -162,41 +162,41 @@
         return protocolIcy;
     case AnyP::PROTO_COAP:
     case AnyP::PROTO_COAPS: // use 'unknown' until libecap supports coap:// and coaps://
     case AnyP::PROTO_UNKNOWN:
         return protocolUnknown; // until we remember the protocol image
     case AnyP::PROTO_NONE:
         return Name();
 
     case AnyP::PROTO_MAX:
         break; // should not happen
         // no default to catch AnyP::PROTO_ additions
     }
     Must(false); // not reached
     return Name();
 }
 
 void
 Adaptation::Ecap::FirstLineRep::protocol(const Name &p)
 {
     // TODO: what happens if we fail to translate some protocol?
-    theMessage.protocol = TranslateProtocolId(p);
+    theMessage.http_ver.protocol = TranslateProtocolId(p);
 }
 
 AnyP::ProtocolType
 Adaptation::Ecap::FirstLineRep::TranslateProtocolId(const Name &name)
 {
     if (name.assignedHostId())
         return static_cast<AnyP::ProtocolType>(name.hostId());
     return AnyP::PROTO_UNKNOWN;
 }
 
 /* RequestHeaderRep */
 
 Adaptation::Ecap::RequestLineRep::RequestLineRep(HttpRequest &aMessage):
         FirstLineRep(aMessage), theMessage(aMessage)
 {
 }
 
 void
 Adaptation::Ecap::RequestLineRep::uri(const Area &aUri)
 {

Reply via email to