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 1d21ff3fd537f8699dad293448731dcbd54f4fbe..8ca441555dd124267f52795ed8a035332ac7c444 100644 (file)
--- 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<HttpRequest> req(new HttpRequest());
+    if (urlParse(method, url, *req))
+        return req.release();
+    return nullptr;
 }
 
 /**

diff --git a/src/URL.h b/src/URL.h
index 4bbe45381c74797f94a6f9f340661278d4b5924a..6d2eeaba4ac384509f10d41c4dacaf054e6bb99e 100644 (file)
--- 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 584e9cd927bdf62f8193bd56a8bf4680a60754dc..be572e94371ac1789d068a75ace5203ce95451e3 100644 (file)
--- 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 899c4a18ceebf773f41a73bfa8ac712bdb4b4abd..3a8f0254f433c80ac777586f4a7ba38db71a2e0c 100644 (file)
--- 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<char*>(urlNote), new_request)) {
+                if (urlParse(old_request->method, const_cast<char*>(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 ac2e6e59248394c3d7a70c4fa85b41982ca50aac..8ca856b702ffa7c0aeef16d643b5dc62fe4533a5 100644 (file)
--- 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 08e1e1bd320badfb16ed36a3a3b61f9d07a31f01..07392c1f0ca997025f2ddf770de42458bf978ecf 100644 (file)
--- 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