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) {