From: Christos Tsantilas Date: Sun, 25 Jun 2017 01:01:24 +0000 (+1200) Subject: Bug 1961 partial: Redesign urlParse API X-Git-Tag: SQUID_4_0_21~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=def915dbcf75f90b75b674e6bd6be8c0089f4214;p=thirdparty%2Fsquid.git Bug 1961 partial: Redesign urlParse API The urlParse modified to require an HttpRequest object as parameter and return boolean value to indicate URL parsing status. This is a Measurement Factory project --- diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index b786eec976..0b68e29e94 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -327,14 +327,11 @@ HttpRequest::parseFirstLine(const char *start, const char *end) * (char *) end = '\0'; // temp terminate URI, XXX dangerous? - HttpRequest *tmp = urlParse(method, (char *) start, this); + const bool ret = urlParse(method, (char *) start, *this); * (char *) end = save; - if (NULL == tmp) - return false; - - return true; + return ret; } /* swaps out request using httpRequestPack */ @@ -520,7 +517,10 @@ HttpRequest::expectingBody(const HttpRequestMethod &, int64_t &theSize) const HttpRequest * HttpRequest::CreateFromUrl(char * url, const HttpRequestMethod& method) { - return urlParse(method, url, NULL); + std::unique_ptr req(new HttpRequest()); + if (urlParse(method, url, *req)) + return req.release(); + return nullptr; } /** diff --git a/src/URL.h b/src/URL.h index 4bbe45381c..6d2eeaba4a 100644 --- a/src/URL.h +++ b/src/URL.h @@ -166,7 +166,7 @@ class HttpRequest; class HttpRequestMethod; void urlInitialize(void); -HttpRequest *urlParse(const HttpRequestMethod&, char *, HttpRequest *request = NULL); +bool urlParse(const HttpRequestMethod&, char *, HttpRequest &request); char *urlCanonicalClean(const HttpRequest *); const char *urlCanonicalFakeHttps(const HttpRequest * request); bool urlIsRelative(const char *); diff --git a/src/adaptation/ecap/MessageRep.cc b/src/adaptation/ecap/MessageRep.cc index 99a1c151dc..83e5165e27 100644 --- a/src/adaptation/ecap/MessageRep.cc +++ b/src/adaptation/ecap/MessageRep.cc @@ -203,7 +203,7 @@ Adaptation::Ecap::RequestLineRep::uri(const Area &aUri) // Can we change urlParse API to remove the method parameter? // TODO: optimize: urlPath should take constant URL buffer char *buf = xstrdup(aUri.toString().c_str()); - const bool ok = urlParse(theMessage.method, buf, &theMessage); + const bool ok = urlParse(theMessage.method, buf, theMessage); xfree(buf); Must(ok); } diff --git a/src/client_side_request.cc b/src/client_side_request.cc index 782002c286..4506502750 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -1276,7 +1276,7 @@ ClientRequestContext::clientRedirectDone(const Helper::Reply &reply) // XXX: validate the URL properly *without* generating a whole new request object right here. // XXX: the clone() should be done only AFTER we know the new URL is valid. HttpRequest *new_request = old_request->clone(); - if (urlParse(old_request->method, const_cast(urlNote), new_request)) { + if (urlParse(old_request->method, const_cast(urlNote), *new_request)) { debugs(61, 2, "URL-rewriter diverts URL from " << old_request->effectiveRequestUri() << " to " << new_request->effectiveRequestUri()); // update the new request to flag the re-writing was done on it diff --git a/src/url.cc b/src/url.cc index 08e1e1bd32..b3ae1806c2 100644 --- a/src/url.cc +++ b/src/url.cc @@ -16,15 +16,15 @@ #include "SquidString.h" #include "URL.h" -static HttpRequest *urlParseFinish(const HttpRequestMethod& method, +static bool urlParseFinish(const HttpRequestMethod& method, const AnyP::ProtocolType protocol, const char *const protoStr, const char *const urlpath, const char *const host, const SBuf &login, const int port, - HttpRequest *request); -static HttpRequest *urnParse(const HttpRequestMethod& method, char *urn, HttpRequest *request); + HttpRequest &request); +static bool urnParse(const HttpRequestMethod& method, char *urn, HttpRequest &request); static const char valid_hostname_chars_u[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz" @@ -179,8 +179,7 @@ urlParseProtocol(const char *b) /* * Parse a URI/URL. * - * If the 'request' arg is non-NULL, put parsed values there instead - * of allocating a new HttpRequest. + * Stores parsed values in the `request` argument. * * This abuses HttpRequest as a way of representing the parsed url * and its components. @@ -197,8 +196,8 @@ urlParseProtocol(const char *b) * its partial or not (ie, it handles the case of no trailing slash as * being "end of host with implied path of /". */ -HttpRequest * -urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) +bool +urlParse(const HttpRequestMethod& method, char *url, HttpRequest &request) { LOCAL_ARRAY(char, proto, MAX_URL); LOCAL_ARRAY(char, login, MAX_URL); @@ -218,14 +217,14 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) /* terminate so it doesn't overflow other buffers */ *(url + (MAX_URL >> 1)) = '\0'; debugs(23, DBG_IMPORTANT, "urlParse: URL too large (" << l << " bytes)"); - return NULL; + return false; } if (method == Http::METHOD_CONNECT) { port = CONNECT_PORT; if (sscanf(url, "[%[^]]]:%d", host, &port) < 1) if (sscanf(url, "%[^:]:%d", host, &port) < 1) - return NULL; + return false; } else if ((method == Http::METHOD_OPTIONS || method == Http::METHOD_TRACE) && URL::Asterisk().cmp(url) == 0) { @@ -243,12 +242,12 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) *dst = *src; } if (i >= l) - return NULL; + return false; *dst = '\0'; /* Then its :// */ if ((i+3) > l || *src != ':' || *(src + 1) != '/' || *(src + 2) != '/') - return NULL; + return false; i += 3; src += 3; @@ -266,7 +265,7 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) * been -given- a valid URL and the path is just '/'. */ if (i > l) - return NULL; + return false; *dst = '\0'; // bug 3074: received 'path' starting with '?', '#', or '\0' implies '/' @@ -283,7 +282,7 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) /* We -could- be at the end of the buffer here */ if (i > l) - return NULL; + return false; /* If the URL path is empty we set it to be "/" */ if (dst == urlpath) { *dst = '/'; @@ -342,7 +341,7 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) // Bug 3183 sanity check: If scheme is present, host must be too. if (protocol != AnyP::PROTO_NONE && host[0] == '\0') { debugs(23, DBG_IMPORTANT, "SECURITY ALERT: Missing hostname in URL '" << url << "'. see access.log for details."); - return NULL; + return false; } if (t && *t == ':') { @@ -373,7 +372,7 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) if (Config.onoff.check_hostnames && strspn(host, Config.onoff.allow_underscore ? valid_hostname_chars_u : valid_hostname_chars) != strlen(host)) { debugs(23, DBG_IMPORTANT, "urlParse: Illegal character in hostname '" << host << "'"); - return NULL; + return false; } /* For IPV6 addresses also check for a colon */ @@ -387,12 +386,12 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) /* reject duplicate or leading dots */ if (strstr(host, "..") || *host == '.') { debugs(23, DBG_IMPORTANT, "urlParse: Illegal hostname '" << host << "'"); - return NULL; + return false; } if (port < 1 || port > 65535) { debugs(23, 3, "urlParse: Invalid port '" << port << "'"); - return NULL; + return false; } #if HARDCODE_DENY_PORTS @@ -400,7 +399,7 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) * maybe someone wants them hardcoded... */ if (port == 7 || port == 9 || port == 19) { debugs(23, DBG_CRITICAL, "urlParse: Deny access to port " << port); - return NULL; + return false; } #endif @@ -410,7 +409,7 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) switch (Config.uri_whitespace) { case URI_WHITESPACE_DENY: - return NULL; + return false; case URI_WHITESPACE_ALLOW: break; @@ -446,7 +445,7 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) * non-NULL, put parsed values there instead of allocating a new * HttpRequest. */ -static HttpRequest * +static bool urlParseFinish(const HttpRequestMethod& method, const AnyP::ProtocolType protocol, const char *const protoStr, // for unknown protocols @@ -454,30 +453,21 @@ urlParseFinish(const HttpRequestMethod& method, const char *const host, const SBuf &login, const int port, - HttpRequest *request) + HttpRequest &request) { - if (NULL == request) - request = new HttpRequest(method, protocol, protoStr, urlpath); - else { - request->initHTTP(method, protocol, protoStr, urlpath); - } - - request->url.host(host); - request->url.userInfo(login); - request->url.port(port); - return request; + request.initHTTP(method, protocol, protoStr, urlpath); + request.url.host(host); + request.url.userInfo(login); + request.url.port(port); + return true; } -static HttpRequest * -urnParse(const HttpRequestMethod& method, char *urn, HttpRequest *request) +static bool +urnParse(const HttpRequestMethod& method, char *urn, HttpRequest &request) { debugs(50, 5, "urnParse: " << urn); - if (request) { - request->initHTTP(method, AnyP::PROTO_URN, "urn", urn + 4); - return request; - } - - return new HttpRequest(method, AnyP::PROTO_URN, "urn", urn + 4); + request.initHTTP(method, AnyP::PROTO_URN, "urn", urn + 4); + return true; } void