From: Christos Tsantilas Date: Mon, 12 Jun 2017 17:54:02 +0000 (+0300) Subject: Redesign urlParse API X-Git-Tag: M-staged-PR71~125 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9157915c7aada28ce632a88a6a5d7411861aa1e9;p=thirdparty%2Fsquid.git 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 1d21ff3fd5..8ca441555d 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 584e9cd927..be572e9437 100644 --- a/src/adaptation/ecap/MessageRep.cc +++ b/src/adaptation/ecap/MessageRep.cc @@ -207,7 +207,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 899c4a18ce..3a8f0254f4 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/tests/stub_url.cc b/src/tests/stub_url.cc index ac2e6e5924..8ca856b702 100644 --- a/src/tests/stub_url.cc +++ b/src/tests/stub_url.cc @@ -30,7 +30,7 @@ const SBuf &URL::Asterisk() SBuf &URL::authority(bool) const STUB_RETVAL(nil) SBuf &URL::absolute() const STUB_RETVAL(nil) void urlInitialize() STUB -HttpRequest *urlParse(const HttpRequestMethod&, char *, HttpRequest *) STUB_RETVAL(nullptr) +bool urlParse(const HttpRequestMethod&, char *, HttpRequest &) STUB_RETVAL(true) char *urlCanonicalClean(const HttpRequest *) STUB_RETVAL(nullptr) const char *urlCanonicalFakeHttps(const HttpRequest *) STUB_RETVAL(nullptr) bool urlIsRelative(const char *) STUB_RETVAL(false) diff --git a/src/url.cc b/src/url.cc index 08e1e1bd32..07392c1f0c 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); @@ -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