From: Amos Jeffries Date: Tue, 7 Jul 2015 11:53:08 +0000 (-0700) Subject: Cleanup: refactor HttpRequest::urlpath member into class URL X-Git-Tag: merge-candidate-3-v1~52^2~7 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=51b5dcf;p=thirdparty%2Fsquid.git Cleanup: refactor HttpRequest::urlpath member into class URL --- diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index 3308e789eb..3a2b0f3d38 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -61,7 +61,7 @@ HttpRequest::initHTTP(const HttpRequestMethod& aMethod, AnyP::ProtocolType aProt { method = aMethod; url.setScheme(aProtocol); - urlpath = aUrlpath; + url.path(aUrlpath); } void @@ -69,7 +69,6 @@ HttpRequest::init() { method = Http::METHOD_NONE; url.clear(); - urlpath = NULL; #if USE_AUTH auth_user_request = NULL; #endif @@ -126,7 +125,6 @@ HttpRequest::clean() safe_free(vary_headers); url.clear(); - urlpath.clean(); header.clean(); @@ -173,7 +171,8 @@ HttpRequest::reset() HttpRequest * HttpRequest::clone() const { - HttpRequest *copy = new HttpRequest(method, url.getScheme(), urlpath.termedBuf()); + HttpRequest *copy = new HttpRequest(); + copy->method = method; // TODO: move common cloning clone to Msg::copyTo() or copy ctor copy->header.append(&header); copy->hdrCacheInit(); @@ -182,9 +181,11 @@ HttpRequest::clone() const copy->pstate = pstate; // TODO: should we assert a specific state here? copy->body_pipe = body_pipe; + copy->url.setScheme(url.getScheme()); copy->url.userInfo(url.userInfo()); copy->url.host(url.host()); copy->url.port(url.port()); + copy->url.path(url.path()); // urlPath handled in ctor copy->canonical = canonical ? xstrdup(canonical) : NULL; @@ -372,8 +373,8 @@ HttpRequest::pack(Packable * p) { assert(p); /* pack request-line */ - p->appendf(SQUIDSBUFPH " " SQUIDSTRINGPH " HTTP/%d.%d\r\n", - SQUIDSBUFPRINT(method.image()), SQUIDSTRINGPRINT(urlpath), + p->appendf(SQUIDSBUFPH " " SQUIDSBUFPH " HTTP/%d.%d\r\n", + SQUIDSBUFPRINT(method.image()), SQUIDSBUFPRINT(url.path()), http_ver.major, http_ver.minor); /* headers */ header.packInto(p); @@ -393,10 +394,10 @@ httpRequestPack(void *obj, Packable *p) /* returns the length of request line + headers + crlf */ int -HttpRequest::prefixLen() +HttpRequest::prefixLen() const { return method.image().length() + 1 + - urlpath.size() + 1 + + url.path().length() + 1 + 4 + 1 + 3 + 2 + header.len + 2; } @@ -491,23 +492,19 @@ HttpRequest::clearError() errDetail = ERR_DETAIL_NONE; } -const char *HttpRequest::packableURI(bool full_uri) const +void +HttpRequest::packFirstLineInto(Packable * p, bool full_uri) const { + SBuf tmp; if (full_uri) - return urlCanonical((HttpRequest*)this); - - if (urlpath.size()) - return urlpath.termedBuf(); + tmp = urlCanonical((HttpRequest*)this); + else + tmp = url.path(); - return "/"; -} - -void HttpRequest::packFirstLineInto(Packable * p, bool full_uri) const -{ // form HTTP request-line - p->appendf(SQUIDSBUFPH " %s HTTP/%d.%d\r\n", + p->appendf(SQUIDSBUFPH " " SQUIDSBUFPH " HTTP/%d.%d\r\n", SQUIDSBUFPRINT(method.image()), - packableURI(full_uri), + SQUIDSBUFPRINT(tmp), http_ver.major, http_ver.minor); } diff --git a/src/HttpRequest.h b/src/HttpRequest.h index dd885f162b..a0f11d6855 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -116,8 +116,6 @@ public: Auth::UserRequest::Pointer auth_user_request; #endif - String urlpath; - char *canonical; /** @@ -196,7 +194,7 @@ public: bool bodyNibbled() const; // the request has a [partially] consumed body - int prefixLen(); + int prefixLen() const; void swapOut(StoreEntry * e); @@ -230,8 +228,6 @@ public: int64_t getRangeOffsetLimit(); /* the result of this function gets cached in rangeOffsetLimit */ private: - const char *packableURI(bool full_uri) const; - mutable int64_t rangeOffsetLimit; /* caches the result of getRangeOffsetLimit */ protected: diff --git a/src/URL.h b/src/URL.h index 8411887e84..c33dc53e19 100644 --- a/src/URL.h +++ b/src/URL.h @@ -14,6 +14,8 @@ #include "rfc2181.h" #include "SBuf.h" +#include + /** * The URL class represents a Uniform Resource Location * @@ -53,6 +55,13 @@ public: void port(unsigned short p) {port_=p; touch();} unsigned short port() const {return port_;} + void path(const char *p) {path_=p; touch();} + void path(const SBuf &p) {path_=p; touch();} + const SBuf &path() const; + + /// the static '/' default URL-path + static const SBuf &SlashPath(); + /// the static '*' pseudo-URL static const SBuf &Asterisk(); @@ -101,11 +110,24 @@ private: unsigned short port_; ///< URL port + // XXX: for now includes query-string. + SBuf path_; ///< URL path segment + // pre-assembled URL forms mutable SBuf authorityHttp_; ///< RFC 7230 section 5.3.3 authority, maybe without default-port mutable SBuf authorityWithPort_; ///< RFC 7230 section 5.3.3 authority with explicit port + mutable SBuf canonical_; ///< full absolute-URI }; +inline std::ostream & +operator <<(std::ostream &os, const URL &url) +{ + if (const char *sc = url.getScheme().c_str()) + os << sc << ":"; + os << "//" << url.authority() << url.path(); + return os; +} + class HttpRequest; class HttpRequestMethod; diff --git a/src/acl/UrlPath.cc b/src/acl/UrlPath.cc index e3911d671c..eb4e04007f 100644 --- a/src/acl/UrlPath.cc +++ b/src/acl/UrlPath.cc @@ -18,13 +18,14 @@ int ACLUrlPathStrategy::match (ACLData * &data, ACLFilledChecklist *checklist, ACLFlags &) { - if (!checklist->request->urlpath.size()) + if (checklist->request->url.path().isEmpty()) return -1; - char *esc_buf = xstrdup(checklist->request->urlpath.termedBuf()); + SBuf tmp = checklist->request->url.path(); + char *esc_buf = xstrndup(tmp.rawContent(), tmp.length()); rfc1738_unescape(esc_buf); int result = data->match(esc_buf); - safe_free(esc_buf); + xfree(esc_buf); return result; } diff --git a/src/adaptation/Service.cc b/src/adaptation/Service.cc index 36e7f46f9e..b36246d2d4 100644 --- a/src/adaptation/Service.cc +++ b/src/adaptation/Service.cc @@ -49,7 +49,7 @@ Adaptation::Service::wants(const ServiceFilter &filter) const // Sending a message to a service that does not want it is useless. // note that we cannot check wantsUrl for service that is not "up" // note that even essential services are skipped on unwanted URLs! - return wantsUrl(filter.request->urlpath); + return wantsUrl(filter.request->url.path()); } // The service is down and is either not bypassable or not probed due diff --git a/src/adaptation/Service.h b/src/adaptation/Service.h index e1d028329b..52a4f9ebb6 100644 --- a/src/adaptation/Service.h +++ b/src/adaptation/Service.h @@ -45,7 +45,7 @@ public: bool wants(const ServiceFilter &filter) const; // the methods below can only be called on an up() service - virtual bool wantsUrl(const String &urlPath) const = 0; + virtual bool wantsUrl(const SBuf &urlPath) const = 0; // called by transactions to report service failure virtual void noteFailure() = 0; diff --git a/src/adaptation/ecap/ServiceRep.cc b/src/adaptation/ecap/ServiceRep.cc index 489300cc1e..95dda0992e 100644 --- a/src/adaptation/ecap/ServiceRep.cc +++ b/src/adaptation/ecap/ServiceRep.cc @@ -237,10 +237,12 @@ bool Adaptation::Ecap::ServiceRep::up() const return theService != NULL; } -bool Adaptation::Ecap::ServiceRep::wantsUrl(const String &urlPath) const +bool Adaptation::Ecap::ServiceRep::wantsUrl(const SBuf &urlPath) const { Must(up()); - return theService->wantsUrl(urlPath.termedBuf()); + SBuf nonConstUrlPath = urlPath; + // c_str() reallocates and terminates for libecap API + return theService->wantsUrl(nonConstUrlPath.c_str()); } Adaptation::Initiate * diff --git a/src/adaptation/ecap/ServiceRep.h b/src/adaptation/ecap/ServiceRep.h index 28cc0aab52..248d1d9214 100644 --- a/src/adaptation/ecap/ServiceRep.h +++ b/src/adaptation/ecap/ServiceRep.h @@ -38,7 +38,7 @@ public: virtual bool probed() const; virtual bool up() const; virtual Adaptation::Initiate *makeXactLauncher(HttpMsg *virginHeader, HttpRequest *virginCause, AccessLogEntry::Pointer &alp); - virtual bool wantsUrl(const String &urlPath) const; + virtual bool wantsUrl(const SBuf &urlPath) const; virtual void noteFailure(); virtual const char *status() const; virtual void detach(); diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index 5c51e37622..310227048b 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -1389,9 +1389,7 @@ void Adaptation::Icap::ModXact::makeRequestHeaders(MemBuf &buf) // to simplify, we could assume that request is always available - String urlPath; if (request) { - urlPath = request->urlpath; if (ICAP::methodRespmod == m) encapsulateHead(buf, "req-hdr", httpBuf, request); else if (ICAP::methodReqmod == m) @@ -1583,10 +1581,10 @@ void Adaptation::Icap::ModXact::decideOnPreview() return; } - const String urlPath = virginRequest().urlpath; + const SBuf urlPath(virginRequest().url.path()); size_t wantedSize; if (!service().wantsPreview(urlPath, wantedSize)) { - debugs(93, 5, HERE << "should not offer preview for " << urlPath); + debugs(93, 5, "should not offer preview for " << urlPath); return; } diff --git a/src/adaptation/icap/Options.cc b/src/adaptation/icap/Options.cc index 3e0593ca7f..d3d4db69a5 100644 --- a/src/adaptation/icap/Options.cc +++ b/src/adaptation/icap/Options.cc @@ -43,7 +43,8 @@ Adaptation::Icap::Options::~Options() // future optimization note: this method is called by ICAP ACL code at least // twice for each HTTP message to see if the message should be ignored. For any // non-ignored HTTP message, ICAP calls to check whether a preview is needed. -Adaptation::Icap::Options::TransferKind Adaptation::Icap::Options::transferKind(const String &urlPath) const +Adaptation::Icap::Options::TransferKind +Adaptation::Icap::Options::transferKind(const SBuf &urlPath) const { if (theTransfers.preview.matches(urlPath)) return xferPreview; @@ -54,7 +55,7 @@ Adaptation::Icap::Options::TransferKind Adaptation::Icap::Options::transferKind( if (theTransfers.ignore.matches(urlPath)) return xferIgnore; - debugs(93,7, HERE << "url " << urlPath << " matches no extensions; " << + debugs(93,7, "url " << urlPath << " matches no extensions; " << "using default: " << theTransfers.byDefault->name); return theTransfers.byDefault->kind; } @@ -184,26 +185,24 @@ void Adaptation::Icap::Options::TransferList::add(const char *extension) wordlistAdd(&extensions, extension); }; -bool Adaptation::Icap::Options::TransferList::matches(const String &urlPath) const +bool Adaptation::Icap::Options::TransferList::matches(const SBuf &urlPath) const { - const int urlLen = urlPath.size(); + const SBuf::size_type urlLen = urlPath.length(); for (wordlist *e = extensions; e; e = e->next) { // optimize: store extension lengths - const int eLen = strlen(e->key); + const size_t eLen = strlen(e->key); // assume URL contains at least '/' before the extension if (eLen < urlLen) { - const int eOff = urlLen - eLen; + const size_t eOff = urlLen - eLen; // RFC 3507 examples imply that extensions come without leading '.' - if (urlPath[eOff-1] == '.' && - strcmp(urlPath.termedBuf() + eOff, e->key) == 0) { - debugs(93,7, HERE << "url " << urlPath << " matches " << - name << " extension " << e->key); + if (urlPath[eOff-1] == '.' && urlPath.substr(eOff).cmp(e->key, eLen) == 0) { + debugs(93,7, "url " << urlPath << " matches " << name << " extension " << e->key); return true; } } } - debugs(93,8, HERE << "url " << urlPath << " matches no " << name << " extensions"); + debugs(93,8, "url " << urlPath << " matches no " << name << " extensions"); return false; } diff --git a/src/adaptation/icap/Options.h b/src/adaptation/icap/Options.h index f0451c778b..c4a23f6c63 100644 --- a/src/adaptation/icap/Options.h +++ b/src/adaptation/icap/Options.h @@ -42,7 +42,7 @@ public: time_t timestamp() const { return theTimestamp; }; typedef enum { xferNone, xferPreview, xferIgnore, xferComplete } TransferKind; - TransferKind transferKind(const String &urlPath) const; + TransferKind transferKind(const SBuf &urlPath) const; public: const char *error; // human-readable information; set iff !valid() @@ -68,7 +68,7 @@ protected: TransferList(); ~TransferList(); - bool matches(const String &urlPath) const; + bool matches(const SBuf &urlPath) const; void parse(const String &buf, bool &foundStar); void add(const char *extension); diff --git a/src/adaptation/icap/ServiceRep.cc b/src/adaptation/icap/ServiceRep.cc index b7f5ea6acd..c25bf2072c 100644 --- a/src/adaptation/icap/ServiceRep.cc +++ b/src/adaptation/icap/ServiceRep.cc @@ -321,13 +321,13 @@ bool Adaptation::Icap::ServiceRep::availableForOld() const return (available != 0); // it is -1 (no limit) or has available slots } -bool Adaptation::Icap::ServiceRep::wantsUrl(const String &urlPath) const +bool Adaptation::Icap::ServiceRep::wantsUrl(const SBuf &urlPath) const { Must(hasOptions()); return theOptions->transferKind(urlPath) != Adaptation::Icap::Options::xferIgnore; } -bool Adaptation::Icap::ServiceRep::wantsPreview(const String &urlPath, size_t &wantedSize) const +bool Adaptation::Icap::ServiceRep::wantsPreview(const SBuf &urlPath, size_t &wantedSize) const { Must(hasOptions()); diff --git a/src/adaptation/icap/ServiceRep.h b/src/adaptation/icap/ServiceRep.h index a03b9ad8d0..e1eef552b5 100644 --- a/src/adaptation/icap/ServiceRep.h +++ b/src/adaptation/icap/ServiceRep.h @@ -81,8 +81,8 @@ public: void callWhenReady(AsyncCall::Pointer &cb); // the methods below can only be called on an up() service - bool wantsUrl(const String &urlPath) const; - bool wantsPreview(const String &urlPath, size_t &wantedSize) const; + bool wantsUrl(const SBuf &urlPath) const; + bool wantsPreview(const SBuf &urlPath, size_t &wantedSize) const; bool allows204() const; bool allows206() const; Comm::ConnectionPointer getConnection(bool isRetriable, bool &isReused); diff --git a/src/carp.cc b/src/carp.cc index 5d3c4455d1..dcc75c3e76 100644 --- a/src/carp.cc +++ b/src/carp.cc @@ -178,16 +178,14 @@ carpSelectParent(HttpRequest * request) key.appendf(":%u", request->url.port()); } if (tp->options.carp_key.path) { - String::size_type pos; - if ((pos=request->urlpath.find('?'))!=String::npos) - key.append(SBuf(request->urlpath.substr(0,pos))); - else - key.append(SBuf(request->urlpath)); + // XXX: fix when path and query are separate + key.append(request->url.path().substr(0,request->url.path().find('?'))); // 0..N } if (tp->options.carp_key.params) { - String::size_type pos; - if ((pos=request->urlpath.find('?'))!=String::npos) - key.append(SBuf(request->urlpath.substr(pos,request->urlpath.size()))); + // XXX: fix when path and query are separate + SBuf::size_type pos; + if ((pos=request->url.path().find('?')) != SBuf::npos) + key.append(request->url.path().substr(pos)); // N..npos } } // if the url-based key is empty, e.g. because the user is diff --git a/src/client_side.cc b/src/client_side.cc index c3477fb8df..739cffae53 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2203,11 +2203,6 @@ parseHttpRequest(ConnStateData *csd, const Http1::RequestParserPointer &hp) clientSocketDetach, newClient, tempBuffer); /* set url */ - // XXX: c_str() does re-allocate but here replaces explicit malloc/free. - // when internalCheck() accepts SBuf removing this will be a net gain for performance. - SBuf tmp(hp->requestUri()); - const char *url = tmp.c_str(); - debugs(33,5, "Prepare absolute URL from " << (csd->transparent()?"intercept":(csd->port->flags.accelSurrogate ? "accel":""))); /* Rewrite the URL in transparent or accelerator mode */ @@ -2218,7 +2213,7 @@ parseHttpRequest(ConnStateData *csd, const Http1::RequestParserPointer &hp) * - intercept mode (NAT) * - intercept mode with failures * - accelerator mode (reverse proxy) - * - internal URL + * - internal relative-URL * - mixed combos of the above with internal URL * - remote interception with PROXY protocol * - remote reverse-proxy with PROXY protocol @@ -2227,10 +2222,10 @@ parseHttpRequest(ConnStateData *csd, const Http1::RequestParserPointer &hp) /* intercept or transparent mode, properly working with no failures */ prepareTransparentURL(csd, http, hp); - } else if (internalCheck(url)) { + } else if (internalCheck(hp->requestUri())) { // NP: only matches relative-URI /* internal URL mode */ /* prepend our name & port */ - http->uri = xstrdup(internalLocalUri(NULL, url)); + http->uri = xstrdup(internalLocalUri(NULL, hp->requestUri())); // We just re-wrote the URL. Must replace the Host: header. // But have not parsed there yet!! flag for local-only handling. http->flags.internal = true; @@ -2245,7 +2240,7 @@ parseHttpRequest(ConnStateData *csd, const Http1::RequestParserPointer &hp) * requested url. may be rewritten later, so make extra room */ int url_sz = hp->requestUri().length() + Config.appendDomainLen + 5; http->uri = (char *)xcalloc(url_sz, 1); - strcpy(http->uri, url); + xstrncpy(http->uri, hp->requestUri().rawContent(), hp->requestUri().length()); } result->flags.parsed_ok = 1; @@ -2563,11 +2558,11 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp, request->flags.spoofClientIp = false; } - if (internalCheck(request->urlpath.termedBuf())) { + if (internalCheck(request->url.path())) { if (internalHostnameIs(request->url.host()) && request->url.port() == getMyPort()) { debugs(33, 2, "internal URL found: " << request->url.getScheme() << "://" << request->url.authority(true)); http->flags.internal = true; - } else if (Config.onoff.global_internal_static && internalStaticCheck(request->urlpath.termedBuf())) { + } else if (Config.onoff.global_internal_static && internalStaticCheck(request->url.path())) { debugs(33, 2, "internal URL found: " << request->url.getScheme() << "://" << request->url.authority(true) << " (global_internal_static on)"); request->url.setScheme(AnyP::PROTO_HTTP); request->SetHost(internalHostname()); diff --git a/src/clients/FtpGateway.cc b/src/clients/FtpGateway.cc index b43205b655..8b6dbfbdcb 100644 --- a/src/clients/FtpGateway.cc +++ b/src/clients/FtpGateway.cc @@ -1081,35 +1081,33 @@ Ftp::Gateway::checkAuth(const HttpHeader * req_hdr) return 0; /* different username */ } -static String str_type_eq; void Ftp::Gateway::checkUrlpath() { - int l; - size_t t; + static SBuf str_type_eq("type="); + auto t = request->url.path().rfind(';'); - if (str_type_eq.size()==0) //hack. String doesn't support global-static - str_type_eq="type="; - - if ((t = request->urlpath.rfind(';')) != String::npos) { - if (request->urlpath.substr(t+1,t+1+str_type_eq.size())==str_type_eq) { - typecode = (char)xtoupper(request->urlpath[t+str_type_eq.size()+1]); - request->urlpath.cut(t); + if (t != SBuf::npos) { + auto filenameEnd = t-1; + if (request->url.path().substr(++t).cmp(str_type_eq, str_type_eq.length()) == 0) { + t += str_type_eq.length(); + typecode = (char)xtoupper(request->url.path()[t]); + request->url.path(request->url.path().substr(0,filenameEnd)); } } - l = request->urlpath.size(); + int l = request->url.path().length(); /* check for null path */ if (!l) { flags.isdir = 1; flags.root_dir = 1; flags.need_base_href = 1; /* Work around broken browsers */ - } else if (!request->urlpath.cmp("/%2f/")) { + } else if (!request->url.path().cmp("/%2f/")) { /* UNIX root directory */ flags.isdir = 1; flags.root_dir = 1; - } else if ((l >= 1) && (request->urlpath[l - 1] == '/')) { + } else if ((l >= 1) && (request->url.path()[l-1] == '/')) { /* Directory URL, ending in / */ flags.isdir = 1; @@ -1133,7 +1131,7 @@ Ftp::Gateway::buildTitleUrl() SBuf authority = request->url.authority(request->url.getScheme() != AnyP::PROTO_FTP); title_url.append(authority.rawContent(), authority.length()); - title_url.append(request->urlpath); + title_url.append(request->url.path().rawContent(), request->url.path().length()); base_href = "ftp://"; @@ -1149,7 +1147,7 @@ Ftp::Gateway::buildTitleUrl() } base_href.append(authority.rawContent(), authority.length()); - base_href.append(request->urlpath); + base_href.append(request->url.path().rawContent(), request->url.path().length()); base_href.append("/"); } @@ -1166,8 +1164,8 @@ Ftp::Gateway::start() checkUrlpath(); buildTitleUrl(); - debugs(9, 5, HERE << "FD " << ctrl.conn->fd << " : host=" << request->url.host() << - ", path=" << request->urlpath << ", user=" << user << ", passwd=" << password); + debugs(9, 5, "FD " << ctrl.conn->fd << " : host=" << request->url.host() << + ", path=" << request->url.path() << ", user=" << user << ", passwd=" << password); state = BEGIN; Ftp::Client::start(); } @@ -1365,10 +1363,6 @@ ftpReadPass(Ftp::Gateway * ftpState) static void ftpSendType(Ftp::Gateway * ftpState) { - const char *t; - const char *filename; - char mode; - /* check the server control channel is still available */ if (!ftpState || !ftpState->haveControlChannel("ftpSendType")) return; @@ -1376,7 +1370,7 @@ ftpSendType(Ftp::Gateway * ftpState) /* * Ref section 3.2.2 of RFC 1738 */ - mode = ftpState->typecode; + char mode = ftpState->typecode; switch (mode) { @@ -1394,9 +1388,10 @@ ftpSendType(Ftp::Gateway * ftpState) if (ftpState->flags.isdir) { mode = 'A'; } else { - t = ftpState->request->urlpath.rpos('/'); - filename = t ? t + 1 : ftpState->request->urlpath.termedBuf(); - mode = mimeGetTransferMode(filename); + auto t = ftpState->request->url.path().rfind('/'); + // XXX: performance regression, c_str() may reallocate + SBuf filename = ftpState->request->url.path().substr(t != SBuf::npos ? t + 1 : 0); + mode = mimeGetTransferMode(filename.c_str()); } break; @@ -1423,7 +1418,8 @@ ftpReadType(Ftp::Gateway * ftpState) debugs(9, 3, HERE << "code=" << code); if (code == 200) { - p = path = xstrdup(ftpState->request->urlpath.termedBuf()); + const SBuf tmp = ftpState->request->url.path(); + p = path = xstrndup(tmp.rawContent(),tmp.length()); if (*p == '/') ++p; @@ -2368,7 +2364,9 @@ ftpTrySlashHack(Ftp::Gateway * ftpState) safe_free(ftpState->filepath); /* Build the new path (urlpath begins with /) */ - path = xstrdup(ftpState->request->urlpath.termedBuf()); + const SBuf tmp = ftpState->request->url.path(); + path = xstrndup(tmp.rawContent(), tmp.length()); + path[tmp.length()] = '\0'; rfc1738_unescape(path); @@ -2419,17 +2417,17 @@ Ftp::Gateway::hackShortcut(FTPSM * nextState) static void ftpFail(Ftp::Gateway *ftpState) { - debugs(9, 6, HERE << "flags(" << + const bool slashHack = ftpState->request->url.path().caseCmp("/%2f", 4)==0; + debugs(9, 6, "flags(" << (ftpState->flags.isdir?"IS_DIR,":"") << (ftpState->flags.try_slash_hack?"TRY_SLASH_HACK":"") << "), " << "mdtm=" << ftpState->mdtm << ", size=" << ftpState->theSize << - "slashhack=" << (ftpState->request->urlpath.caseCmp("/%2f", 4)==0? "T":"F") ); + "slashhack=" << (slashHack? "T":"F")); /* Try the / hack to support "Netscape" FTP URL's for retreiving files */ if (!ftpState->flags.isdir && /* Not a directory */ - !ftpState->flags.try_slash_hack && /* Not in slash hack */ - ftpState->mdtm <= 0 && ftpState->theSize < 0 && /* Not known as a file */ - ftpState->request->urlpath.caseCmp("/%2f", 4) != 0) { /* No slash encoded */ + !ftpState->flags.try_slash_hack && !slashHack && /* Not doing slash hack */ + ftpState->mdtm <= 0 && ftpState->theSize < 0) { /* Not known as a file */ switch (ftpState->state) { @@ -2532,12 +2530,6 @@ ftpSendReply(Ftp::Gateway * ftpState) void Ftp::Gateway::appendSuccessHeader() { - const char *mime_type = NULL; - const char *mime_enc = NULL; - String urlpath = request->urlpath; - const char *filename = NULL; - const char *t = NULL; - debugs(9, 3, HERE); if (flags.http_header_sent) @@ -2553,7 +2545,12 @@ Ftp::Gateway::appendSuccessHeader() entry->buffer(); /* released when done processing current data payload */ - filename = (t = urlpath.rpos('/')) ? t + 1 : urlpath.termedBuf(); + SBuf urlPath = request->url.path(); + auto t = urlPath.rfind('/'); + SBuf filename = urlPath.substr(t != SBuf::npos ? t : 0); + + const char *mime_type = NULL; + const char *mime_enc = NULL; if (flags.isdir) { mime_type = "text/html"; @@ -2562,7 +2559,8 @@ Ftp::Gateway::appendSuccessHeader() case 'I': mime_type = "application/octet-stream"; - mime_enc = mimeGetContentEncoding(filename); + // XXX: performance regression, c_str() may reallocate + mime_enc = mimeGetContentEncoding(filename.c_str()); break; case 'A': @@ -2570,8 +2568,9 @@ Ftp::Gateway::appendSuccessHeader() break; default: - mime_type = mimeGetContentType(filename); - mime_enc = mimeGetContentEncoding(filename); + // XXX: performance regression, c_str() may reallocate + mime_type = mimeGetContentType(filename.c_str()); + mime_enc = mimeGetContentEncoding(filename.c_str()); break; } } @@ -2646,18 +2645,18 @@ Ftp::Gateway::ftpAuthRequired(HttpRequest * request, const char *realm) const char * Ftp::UrlWith2f(HttpRequest * request) { - String newbuf = "%2f"; + SBuf newbuf("%2f"); if (request->url.getScheme() != AnyP::PROTO_FTP) return NULL; - if ( request->urlpath[0]=='/' ) { - newbuf.append(request->urlpath); - request->urlpath.absorb(newbuf); + if (request->url.path()[0] == '/') { + newbuf.append(request->url.path()); + request->url.path(newbuf); safe_free(request->canonical); - } else if ( !strncmp(request->urlpath.termedBuf(), "%2f", 3) ) { - newbuf.append(request->urlpath.substr(1,request->urlpath.size())); - request->urlpath.absorb(newbuf); + } else if (!request->url.path().startsWith(newbuf)) { + newbuf.append(request->url.path().substr(1)); + request->url.path(newbuf); safe_free(request->canonical); } diff --git a/src/errorpage.cc b/src/errorpage.cc index 1e7169608d..18b9c30778 100644 --- a/src/errorpage.cc +++ b/src/errorpage.cc @@ -735,18 +735,10 @@ ErrorState::Dump(MemBuf * mb) str.append("\r\n", 2); /* - HTTP stuff */ str.append("HTTP Request:\r\n", 15); - - if (NULL != request) { - String urlpath_or_slash; - - if (request->urlpath.size() != 0) - urlpath_or_slash = request->urlpath; - else - urlpath_or_slash = "/"; - - str.appendf(SQUIDSBUFPH " " SQUIDSTRINGPH " %s/%d.%d\n", + if (request) { + str.appendf(SQUIDSBUFPH " " SQUIDSBUFPH " %s/%d.%d\n", SQUIDSBUFPRINT(request->method.image()), - SQUIDSTRINGPRINT(urlpath_or_slash), + SQUIDSBUFPRINT(request->url.path()), AnyP::ProtocolType_str[request->http_ver.protocol], request->http_ver.major, request->http_ver.minor); request->header.packInto(&str); @@ -953,23 +945,17 @@ ErrorState::Convert(char token, bool building_deny_info_url, bool allowRecursion case 'R': if (building_deny_info_url) { if (request != NULL) { - p = (request->urlpath.size() != 0 ? request->urlpath.termedBuf() : "/"); + SBuf tmp = request->url.path(); + p = tmp.c_str(); no_urlescape = 1; } else p = "[no request]"; break; } - if (NULL != request) { - String urlpath_or_slash; - - if (request->urlpath.size() != 0) - urlpath_or_slash = request->urlpath; - else - urlpath_or_slash = "/"; - - mb.appendf(SQUIDSBUFPH " " SQUIDSTRINGPH " %s/%d.%d\n", + if (request != NULL) { + mb.appendf(SQUIDSBUFPH " " SQUIDSBUFPH " %s/%d.%d\n", SQUIDSBUFPRINT(request->method.image()), - SQUIDSTRINGPRINT(urlpath_or_slash), + SQUIDSBUFPRINT(request->url.path()), AnyP::ProtocolType_str[request->http_ver.protocol], request->http_ver.major, request->http_ver.minor); request->header.packInto(&mb, true); //hide authorization data diff --git a/src/external_acl.cc b/src/external_acl.cc index 62e12fc409..f4542ac70a 100644 --- a/src/external_acl.cc +++ b/src/external_acl.cc @@ -978,8 +978,10 @@ makeExternalAclKey(ACLFilledChecklist * ch, external_acl_data * acl_data) str = buf; break; - case Format::LFT_CLIENT_REQ_URLPATH: - str = request->urlpath.termedBuf(); + case Format::LFT_CLIENT_REQ_URLPATH: { + SBuf tmp = request->url.path(); + str = tmp.c_str(); + } break; case Format::LFT_CLIENT_REQ_METHOD: { diff --git a/src/format/Format.cc b/src/format/Format.cc index 2ef5ef7bf9..67a52d7a07 100644 --- a/src/format/Format.cc +++ b/src/format/Format.cc @@ -971,7 +971,8 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS case LFT_REQUEST_URLPATH_OLD_31: case LFT_CLIENT_REQ_URLPATH: if (al->request) { - out = al->request->urlpath.termedBuf(); + SBuf s = al->request->url.path(); + out = s.c_str(); quote = 1; } break; @@ -1044,7 +1045,8 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS case LFT_SERVER_REQ_URLPATH: if (al->adapted_request) { - out = al->adapted_request->urlpath.termedBuf(); + SBuf s = al->adapted_request->url.path(); + out = s.c_str(); quote = 1; } break; diff --git a/src/gopher.cc b/src/gopher.cc index eef1aff1a2..0347d2dae1 100644 --- a/src/gopher.cc +++ b/src/gopher.cc @@ -21,6 +21,7 @@ #include "HttpRequest.h" #include "MemBuf.h" #include "mime.h" +#include "parser/Tokenizer.h" #include "rfc1738.h" #include "SquidConfig.h" #include "SquidTime.h" @@ -253,23 +254,27 @@ gopherMimeCreate(GopherStateData * gopherState) static void gopher_request_parse(const HttpRequest * req, char *type_id, char *request) { - const char *path = req->urlpath.termedBuf(); + ::Parser::Tokenizer tok(req->url.path()); if (request) - request[0] = '\0'; + *request = 0; - if (path && (*path == '/')) - ++path; + tok.skip('/'); // ignore failures? path could be ab-empty - if (!path || !*path) { + if (tok.atEnd()) { *type_id = GOPHER_DIRECTORY; return; } - *type_id = path[0]; + static const CharacterSet anyByte("UTF-8",0x00, 0xFF); + + SBuf typeId; + (void)tok.prefix(typeId, anyByte, 1); // never fails since !atEnd() + *type_id = typeId[0]; if (request) { - xstrncpy(request, path + 1, MAX_URL); + SBuf path = tok.remaining().substr(0, MAX_URL); + xstrncpy(request, path.rawContent(), path.length()); /* convert %xx to char */ rfc1738_unescape(request); } diff --git a/src/http.cc b/src/http.cc index 44286d77b9..87bacf1085 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1309,7 +1309,7 @@ HttpStateData::continueAfterParsingHeader() const Http::StatusCode s = vrep->sline.status(); const AnyP::ProtocolVersion &v = vrep->sline.version; if (s == Http::scInvalidHeader && v != Http::ProtocolVersion(0,9)) { - debugs(11, DBG_IMPORTANT, "WARNING: HTTP: Invalid Response: Bad header encountered from " << entry->url() << " AKA " << request->url.host() << request->urlpath.termedBuf()); + debugs(11, DBG_IMPORTANT, "WARNING: HTTP: Invalid Response: Bad header encountered from " << entry->url() << " AKA " << request->url); error = ERR_INVALID_RESP; } else if (s == Http::scHeaderTooLarge) { fwd->dontRetry(true); @@ -1319,18 +1319,17 @@ HttpStateData::continueAfterParsingHeader() } } else { // parsed headers but got no reply - debugs(11, DBG_IMPORTANT, "WARNING: HTTP: Invalid Response: No reply at all for " << entry->url() << " AKA " << request->url.host() << request->urlpath.termedBuf()); + debugs(11, DBG_IMPORTANT, "WARNING: HTTP: Invalid Response: No reply at all for " << entry->url() << " AKA " << request->url); error = ERR_INVALID_RESP; } } else { assert(eof); if (inBuf.length()) { error = ERR_INVALID_RESP; - debugs(11, DBG_IMPORTANT, "WARNING: HTTP: Invalid Response: Headers did not parse at all for " << entry->url() << " AKA " << request->url.host() << request->urlpath.termedBuf()); + debugs(11, DBG_IMPORTANT, "WARNING: HTTP: Invalid Response: Headers did not parse at all for " << entry->url() << " AKA " << request->url); } else { error = ERR_ZERO_SIZE_OBJECT; - debugs(11, (request->flags.accelerated?DBG_IMPORTANT:2), "WARNING: HTTP: Invalid Response: No object data received for " << - entry->url() << " AKA " << request->url.host() << request->urlpath.termedBuf()); + debugs(11, (request->flags.accelerated?DBG_IMPORTANT:2), "WARNING: HTTP: Invalid Response: No object data received for " << entry->url() << " AKA " << request->url); } } @@ -2164,14 +2163,11 @@ HttpStateData::buildRequestPrefix(MemBuf * mb) * not the one we are sending. Needs checking. */ const AnyP::ProtocolVersion httpver = Http::ProtocolVersion(); - const char * url; - if (_peer && !_peer->options.originserver) - url = urlCanonical(request); - else - url = request->urlpath.termedBuf(); - mb->appendf(SQUIDSBUFPH " %s %s/%d.%d\r\n", + const bool canonical = (_peer && !_peer->options.originserver); + const SBuf url = canonical ? SBuf(urlCanonical(request)) : request->url.path(); + mb->appendf(SQUIDSBUFPH " " SQUIDSBUFPH " %s/%d.%d\r\n", SQUIDSBUFPRINT(request->method.image()), - url && *url ? url : "/", + SQUIDSBUFPRINT(url), AnyP::ProtocolType_str[httpver.protocol], httpver.major,httpver.minor); /* build and pack headers */ diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc index 167bbc73d0..f7fc4a1bee 100644 --- a/src/icmp/net_db.cc +++ b/src/icmp/net_db.cc @@ -1289,7 +1289,8 @@ netdbExchangeStart(void *data) { #if USE_ICMP CachePeer *p = (CachePeer *)data; - char *uri = internalRemoteUri(p->host, p->http_port, "/squid-internal-dynamic/", "netdb"); + static const SBuf netDB("netdb"); + char *uri = internalRemoteUri(p->host, p->http_port, "/squid-internal-dynamic/", netDB); debugs(38, 3, "netdbExchangeStart: Requesting '" << uri << "'"); assert(NULL != uri); HttpRequest *req = HttpRequest::CreateFromUrl(uri); diff --git a/src/internal.cc b/src/internal.cc index f9e7c49d07..ec0e609849 100644 --- a/src/internal.cc +++ b/src/internal.cc @@ -32,12 +32,16 @@ void internalStart(const Comm::ConnectionPointer &clientConn, HttpRequest * request, StoreEntry * entry) { ErrorState *err; - const char *upath = request->urlpath.termedBuf(); - debugs(76, 3, HERE << clientConn << " requesting '" << upath << "'"); + const SBuf upath = request->url.path(); + debugs(76, 3, clientConn << " requesting '" << upath << "'"); - if (0 == strcmp(upath, "/squid-internal-dynamic/netdb")) { + static const SBuf netdbUri("/squid-internal-dynamic/netdb"); + static const SBuf storeDigestUri("/squid-internal-periodic/store_digest"); + static const SBuf mgrPfx("/squid-internal-mgr/"); + + if (upath == netdbUri) { netdbBinaryExchange(entry); - } else if (0 == strcmp(upath, "/squid-internal-periodic/store_digest")) { + } else if (upath == storeDigestUri) { #if USE_CACHE_DIGESTS const char *msgbuf = "This cache is currently building its digest.\n"; #else @@ -50,8 +54,8 @@ internalStart(const Comm::ConnectionPointer &clientConn, HttpRequest * request, entry->replaceHttpReply(reply); entry->append(msgbuf, strlen(msgbuf)); entry->complete(); - } else if (0 == strncmp(upath, "/squid-internal-mgr/", 20)) { - debugs(17, 2, "calling CacheManager due to URL-path /squid-internal-mgr/"); + } else if (upath.startsWith(mgrPfx)) { + debugs(17, 2, "calling CacheManager due to URL-path " << mgrPfx); CacheManager::GetInstance()->Start(clientConn, request, entry); } else { debugObj(76, 1, "internalStart: unknown request:\n", @@ -61,26 +65,28 @@ internalStart(const Comm::ConnectionPointer &clientConn, HttpRequest * request, } } -int -internalCheck(const char *urlpath) +bool +internalCheck(const SBuf &urlPath) { - return (0 == strncmp(urlpath, "/squid-internal-", 16)); + static const SBuf InternalPfx("/squid-internal-"); + return urlPath.startsWith(InternalPfx); } -int -internalStaticCheck(const char *urlpath) +bool +internalStaticCheck(const SBuf &urlPath) { - return (0 == strncmp(urlpath, "/squid-internal-static", 22)); + static const SBuf InternalStaticPfx("/squid-internal-static"); + return urlPath.startsWith(InternalStaticPfx); } /* * makes internal url with a given host and port (remote internal url) */ char * -internalRemoteUri(const char *host, unsigned short port, const char *dir, const char *name) +internalRemoteUri(const char *host, unsigned short port, const char *dir, const SBuf &name) { static char lc_host[SQUIDHOSTNAMELEN]; - assert(host && name); + assert(host && !name.isEmpty()); /* convert host name to lower case */ xstrncpy(lc_host, host, SQUIDHOSTNAMELEN); Tolower(lc_host); @@ -115,7 +121,7 @@ internalRemoteUri(const char *host, unsigned short port, const char *dir, const if (dir) mb.append(dir, strlen(dir)); - mb.append(name, strlen(name)); + mb.append(name.rawContent(), name.length()); /* return a pointer to a local static buffer */ return mb.buf; @@ -125,7 +131,7 @@ internalRemoteUri(const char *host, unsigned short port, const char *dir, const * makes internal url with local host and port */ char * -internalLocalUri(const char *dir, const char *name) +internalLocalUri(const char *dir, const SBuf &name) { return internalRemoteUri(getMyHostname(), getMyPort(), dir, name); diff --git a/src/internal.h b/src/internal.h index 7f69972102..919ed20ae8 100644 --- a/src/internal.h +++ b/src/internal.h @@ -15,14 +15,16 @@ #define SQUID_INTERNAL_H_ #include "comm/forward.h" + class HttpRequest; +class SBuf; class StoreEntry; void internalStart(const Comm::ConnectionPointer &clientConn, HttpRequest *, StoreEntry *); -int internalCheck(const char *urlpath); -int internalStaticCheck(const char *urlpath); -char *internalLocalUri(const char *dir, const char *name); -char *internalRemoteUri(const char *, unsigned short, const char *, const char *); +bool internalCheck(const SBuf &urlPath); +bool internalStaticCheck(const SBuf &urlPath); +char *internalLocalUri(const char *dir, const SBuf &name); +char *internalRemoteUri(const char *, unsigned short, const char *, const SBuf &); const char *internalHostname(void); int internalHostnameIs(const char *); diff --git a/src/mime.cc b/src/mime.cc index d8d25c1345..ec8d77d11f 100644 --- a/src/mime.cc +++ b/src/mime.cc @@ -118,7 +118,7 @@ mimeGetEntry(const char *fn, int skip_encodings) MimeIcon::MimeIcon(const char *aName) : icon_(aName) { - url_ = xstrdup(internalLocalUri("/squid-internal-static/icons/", icon_.c_str())); + url_ = xstrdup(internalLocalUri("/squid-internal-static/icons/", icon_)); } MimeIcon::~MimeIcon() @@ -131,7 +131,7 @@ MimeIcon::setName(char const *aString) { xfree(url_); icon_ = aString; - url_ = xstrdup(internalLocalUri("/squid-internal-static/icons/", icon_.c_str())); + url_ = xstrdup(internalLocalUri("/squid-internal-static/icons/", icon_)); } SBuf @@ -166,7 +166,7 @@ mimeGetIconURL(const char *fn) mb.append(icon); return mb.c_str(); } else { - return internalLocalUri("/squid-internal-static/icons/", icon.c_str()); + return internalLocalUri("/squid-internal-static/icons/", icon); } } diff --git a/src/peer_digest.cc b/src/peer_digest.cc index 2aae9d46a0..648f6c3c4c 100644 --- a/src/peer_digest.cc +++ b/src/peer_digest.cc @@ -330,7 +330,7 @@ peerDigestRequest(PeerDigest * pd) if (p->digest_url) url = xstrdup(p->digest_url); else - url = xstrdup(internalRemoteUri(p->host, p->http_port, "/squid-internal-periodic/", StoreDigestFileName)); + url = xstrdup(internalRemoteUri(p->host, p->http_port, "/squid-internal-periodic/", SBuf(StoreDigestFileName))); req = HttpRequest::CreateFromUrl(url); diff --git a/src/tests/testHttpRequest.cc b/src/tests/testHttpRequest.cc index 67f4fbdc46..9bc8f47339 100644 --- a/src/tests/testHttpRequest.cc +++ b/src/tests/testHttpRequest.cc @@ -49,7 +49,7 @@ testHttpRequest::testCreateFromUrlAndMethod() CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port()); CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->url.host())); - CPPUNIT_ASSERT_EQUAL(String("/bar"), aRequest->urlpath); + CPPUNIT_ASSERT_EQUAL(SBuf("/bar"), aRequest->url.path()); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast(aRequest->url.getScheme())); CPPUNIT_ASSERT_EQUAL(String("http://foo:90/bar"), String(url)); xfree(url); @@ -61,7 +61,7 @@ testHttpRequest::testCreateFromUrlAndMethod() CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port()); CPPUNIT_ASSERT(aRequest->method == Http::METHOD_PUT); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->url.host())); - CPPUNIT_ASSERT_EQUAL(String("/bar"), aRequest->urlpath); + CPPUNIT_ASSERT_EQUAL(SBuf("/bar"), aRequest->url.path()); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast(aRequest->url.getScheme())); CPPUNIT_ASSERT_EQUAL(String("http://foo/bar"), String(url)); xfree(url); @@ -79,7 +79,7 @@ testHttpRequest::testCreateFromUrlAndMethod() CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port()); CPPUNIT_ASSERT(aRequest->method == Http::METHOD_CONNECT); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->url.host())); - CPPUNIT_ASSERT_EQUAL(String(""), aRequest->urlpath); + CPPUNIT_ASSERT_EQUAL(SBuf(), aRequest->url.path()); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_NONE, static_cast(aRequest->url.getScheme())); CPPUNIT_ASSERT_EQUAL(String("foo:45"), String(url)); xfree(url); @@ -99,7 +99,7 @@ testHttpRequest::testCreateFromUrl() CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port()); CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->url.host())); - CPPUNIT_ASSERT_EQUAL(String("/bar"), aRequest->urlpath); + CPPUNIT_ASSERT_EQUAL(SBuf("/bar"), aRequest->url.path()); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast(aRequest->url.getScheme())); CPPUNIT_ASSERT_EQUAL(String("http://foo:90/bar"), String(url)); xfree(url); @@ -122,7 +122,7 @@ testHttpRequest::testIPv6HostColonBug() CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port()); CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->url.host())); - CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); + CPPUNIT_ASSERT_EQUAL(SBuf("/foo"), aRequest->url.path()); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast(aRequest->url.getScheme())); CPPUNIT_ASSERT_EQUAL(String("http://[2000:800::45]/foo"), String(url)); xfree(url); @@ -134,7 +134,7 @@ testHttpRequest::testIPv6HostColonBug() CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port()); CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->url.host())); - CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); + CPPUNIT_ASSERT_EQUAL(SBuf("/foo"), aRequest->url.path()); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast(aRequest->url.getScheme())); CPPUNIT_ASSERT_EQUAL(String("http://[2000:800::45]:90/foo"), String(url)); xfree(url); @@ -146,7 +146,7 @@ testHttpRequest::testIPv6HostColonBug() CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port()); CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->url.host())); - CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); + CPPUNIT_ASSERT_EQUAL(SBuf("/foo"), aRequest->url.path()); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast(aRequest->url.getScheme())); CPPUNIT_ASSERT_EQUAL(String("http://2000:800::45/foo"), String(url)); xfree(url); diff --git a/src/url.cc b/src/url.cc index d7d97367e7..626d92bf1f 100644 --- a/src/url.cc +++ b/src/url.cc @@ -44,6 +44,13 @@ URL::Asterisk() return star; } +const SBuf & +URL::SlashPath() +{ + static SBuf slash("/"); + return slash; +} + void URL::host(const char *src) { @@ -60,6 +67,18 @@ URL::host(const char *src) touch(); } +const SBuf & +URL::path() const +{ + // RFC 3986 section 3.3 says path can be empty (path-abempty). + // RFC 7230 sections 2.7.3, 5.3.1, 5.7.2 - says path cannot be empty, default to "/" + // at least when sending and using. We must still accept path-abempty as input. + if (path_.isEmpty() && (scheme_ == AnyP::PROTO_HTTP || scheme_ == AnyP::PROTO_HTTPS)) + return SlashPath(); + + return path_; +} + void urlInitialize(void) { @@ -486,8 +505,8 @@ urlCanonical(HttpRequest * request) return request->canonical; if (request->url.getScheme() == AnyP::PROTO_URN) { - snprintf(urlbuf, MAX_URL, "urn:" SQUIDSTRINGPH, - SQUIDSTRINGPRINT(request->urlpath)); + snprintf(urlbuf, MAX_URL, "urn:" SQUIDSBUFPH, + SQUIDSBUFPRINT(request->url.path())); } else { SBuf authorityForm; switch (request->method.id()) { @@ -499,12 +518,12 @@ urlCanonical(HttpRequest * request) default: { authorityForm = request->url.authority(); // host[:port] - snprintf(urlbuf, MAX_URL, "%s://" SQUIDSBUFPH "%s" SQUIDSBUFPH SQUIDSTRINGPH, + snprintf(urlbuf, MAX_URL, "%s://" SQUIDSBUFPH "%s" SQUIDSBUFPH SQUIDSBUFPH, request->url.getScheme().c_str(), SQUIDSBUFPRINT(request->url.userInfo()), !request->url.userInfo().isEmpty() ? "@" : "", SQUIDSBUFPRINT(authorityForm), - SQUIDSTRINGPRINT(request->urlpath)); + SQUIDSBUFPRINT(request->url.path())); } } } @@ -523,8 +542,8 @@ urlCanonicalClean(const HttpRequest * request) char *t; if (request->url.getScheme() == AnyP::PROTO_URN) { - snprintf(buf, MAX_URL, "urn:" SQUIDSTRINGPH, - SQUIDSTRINGPRINT(request->urlpath)); + snprintf(buf, MAX_URL, "urn:" SQUIDSBUFPH, + SQUIDSBUFPRINT(request->url.path())); } else { SBuf authorityForm; switch (request->method.id()) { @@ -536,12 +555,12 @@ urlCanonicalClean(const HttpRequest * request) default: { authorityForm = request->url.authority(); // host[:port] - snprintf(buf, MAX_URL, "%s://" SQUIDSBUFPH "%s" SQUIDSBUFPH SQUIDSTRINGPH, + snprintf(buf, MAX_URL, "%s://" SQUIDSBUFPH "%s" SQUIDSBUFPH SQUIDSBUFPH, request->url.getScheme().c_str(), SQUIDSBUFPRINT(request->url.userInfo()), !request->url.userInfo().isEmpty() ? "@" : "", SQUIDSBUFPRINT(authorityForm), - SQUIDSTRINGPRINT(request->urlpath)); + SQUIDSBUFPRINT(request->url.path())); // strip arguments AFTER a question-mark if (Config.onoff.strip_query_terms) @@ -628,8 +647,8 @@ urlMakeAbsolute(const HttpRequest * req, const char *relUrl) char *urlbuf = (char *)xmalloc(MAX_URL * sizeof(char)); if (req->url.getScheme() == AnyP::PROTO_URN) { - snprintf(urlbuf, MAX_URL, "urn:" SQUIDSTRINGPH, - SQUIDSTRINGPRINT(req->urlpath)); + snprintf(urlbuf, MAX_URL, "urn:" SQUIDSBUFPH, + SQUIDSBUFPRINT(req->url.path())); return (urlbuf); } @@ -640,26 +659,31 @@ urlMakeAbsolute(const HttpRequest * req, const char *relUrl) !req->url.userInfo().isEmpty() ? "@" : "", SQUIDSBUFPRINT(authorityForm)); + // if the first char is '/' assume its a relative path + // XXX: this breaks on scheme-relative URLs, + // but we should not see those outside ESI, and rarely there. if (relUrl[0] == '/') { - strncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1); + xstrncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1); } else { - const char *path = req->urlpath.termedBuf(); - const char *last_slash = strrchr(path, '/'); + SBuf path = req->url.path(); + SBuf::size_type lastSlashPos = path.rfind('/'); - if (last_slash == NULL) { + if (lastSlashPos == SBuf::npos) { + // replace the whole path with the given bit(s) urlbuf[urllen] = '/'; ++urllen; - strncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1); + xstrncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1); } else { - ++last_slash; - size_t pathlen = last_slash - path; - if (pathlen > MAX_URL - urllen - 1) { - pathlen = MAX_URL - urllen - 1; + // replace only the last (file?) segment with the given bit(s) + ++lastSlashPos; + if (lastSlashPos > MAX_URL - urllen - 1) { + // XXX: crops bits in the middle of the combined URL. + lastSlashPos = MAX_URL - urllen - 1; } - strncpy(&urlbuf[urllen], path, pathlen); - urllen += pathlen; + xstrncpy(&urlbuf[urllen], path.rawContent(), lastSlashPos); + urllen += lastSlashPos; if (urllen + 1 < MAX_URL) { - strncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1); + xstrncpy(&urlbuf[urllen], relUrl, MAX_URL - urllen - 1); } } } @@ -770,7 +794,7 @@ urlCheckRequest(const HttpRequest * r) // we support OPTIONS and TRACE directed at us (with a 501 reply, for now) // we also support forwarding OPTIONS and TRACE, except for the *-URI ones if (r->method == Http::METHOD_OPTIONS || r->method == Http::METHOD_TRACE) - return (r->header.getInt64(HDR_MAX_FORWARDS) == 0 || URL::Asterisk().cmp(r->urlpath.rawBuf(), r->urlpath.size()) != 0); + return (r->header.getInt64(HDR_MAX_FORWARDS) == 0 || r->url.path() != URL::Asterisk()); if (r->method == Http::METHOD_PURGE) return 1; diff --git a/src/urn.cc b/src/urn.cc index 2bfa4647bf..454b45f499 100644 --- a/src/urn.cc +++ b/src/urn.cc @@ -35,11 +35,8 @@ class UrnState : public StoreClient public: void created (StoreEntry *newEntry); void start (HttpRequest *, StoreEntry *); - char *getHost (String &urlpath); + char *getHost(const SBuf &urlpath); void setUriResFromRequest(HttpRequest *); - bool RequestNeedsMenu(HttpRequest *r); - void updateRequestURL(HttpRequest *r, char const *newPath, const size_t newPath_len); - void createUriResRequest (String &uri); virtual ~UrnState(); @@ -128,60 +125,38 @@ urnFindMinRtt(url_entry * urls, const HttpRequestMethod &, int *rtt_ret) } char * -UrnState::getHost (String &urlpath) +UrnState::getHost(const SBuf &urlpath) { char * result; size_t p; /** FIXME: this appears to be parsing the URL. *very* badly. */ /* a proper encapsulated URI/URL type needs to clear this up. */ - if ((p=urlpath.find(':')) != String::npos) { - result=xstrndup(urlpath.rawBuf(),p-1); + if ((p = urlpath.find(':')) != SBuf::npos) { + result = xstrndup(urlpath.rawContent(), p-1); } else { - result = xstrndup(urlpath.rawBuf(),urlpath.size()); + result = xstrndup(urlpath.rawContent(), urlpath.length()); } return result; } -bool -UrnState::RequestNeedsMenu(HttpRequest *r) -{ - if (r->urlpath.size() < 5) - return false; - //now we're sure it's long enough - return strncasecmp(r->urlpath.rawBuf(), "menu.", 5) == 0; -} - void -UrnState::updateRequestURL(HttpRequest *r, char const *newPath, const size_t newPath_len) +UrnState::setUriResFromRequest(HttpRequest *r) { - char *new_path = xstrndup (newPath, newPath_len); - r->urlpath = new_path; - xfree(new_path); -} + static const SBuf menu(".menu"); + if (r->url.path().startsWith(menu)) { + r->url.path(r->url.path().substr(5)); // strip prefix "menu." + flags.force_menu = true; + } -void -UrnState::createUriResRequest (String &uri) -{ + SBuf uri = r->url.path(); LOCAL_ARRAY(char, local_urlres, 4096); - char *host = getHost (uri); - snprintf(local_urlres, 4096, "http://%s/uri-res/N2L?urn:" SQUIDSTRINGPH, - host, SQUIDSTRINGPRINT(uri)); + char *host = getHost(uri); + snprintf(local_urlres, 4096, "http://%s/uri-res/N2L?urn:" SQUIDSBUFPH, host, SQUIDSBUFPRINT(uri)); safe_free(host); safe_free(urlres); urlres = xstrdup(local_urlres); urlres_r = HttpRequest::CreateFromUrl(urlres); -} - -void -UrnState::setUriResFromRequest(HttpRequest *r) -{ - if (RequestNeedsMenu(r)) { - updateRequestURL(r, r->urlpath.rawBuf() + 5, r->urlpath.size() - 5 ); - flags.force_menu = true; - } - - createUriResRequest (r->urlpath); if (urlres_r == NULL) { debugs(52, 3, "urnStart: Bad uri-res URL " << urlres); diff --git a/src/whois.cc b/src/whois.cc index fe0831a142..25b5972c91 100644 --- a/src/whois.cc +++ b/src/whois.cc @@ -56,8 +56,6 @@ whoisWriteComplete(const Comm::ConnectionPointer &, char *buf, size_t, Comm::Fla void whoisStart(FwdState * fwd) { - char *buf; - size_t l; WhoisState *p = new WhoisState; p->request = fwd->request; p->entry = fwd->entry; @@ -67,12 +65,11 @@ whoisStart(FwdState * fwd) p->entry->lock("whoisStart"); comm_add_close_handler(fwd->serverConnection()->fd, whoisClose, p); - l = p->request->urlpath.size() + 3; + size_t l = p->request->url.path().length() + 3; + char *buf = (char *)xmalloc(l); - buf = (char *)xmalloc(l); - - String str_print=p->request->urlpath.substr(1,p->request->urlpath.size()); - snprintf(buf, l, SQUIDSTRINGPH"\r\n", SQUIDSTRINGPRINT(str_print)); + const SBuf str_print = p->request->url.path().substr(1); + snprintf(buf, l, SQUIDSBUFPH "\r\n", SQUIDSBUFPRINT(str_print)); AsyncCall::Pointer writeCall = commCbCall(5,5, "whoisWriteComplete", CommIoCbPtrFun(whoisWriteComplete, p));