Cleanup: de-duplicate HttpRequest CreateFromUrl functions

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index 9f61778237dc3403a977cdbbe2ed755cf9e43282..6d75c203050577e5a71be9af3f230c2cdd02a02d 100644 (file)
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -520,22 +520,11 @@ HttpRequest::expectingBody(const HttpRequestMethod &, int64_t &theSize) const
  * If the request cannot be created cleanly, NULL is returned
  */
 HttpRequest *
-HttpRequest::CreateFromUrlAndMethod(char * url, const HttpRequestMethod& method)
+HttpRequest::CreateFromUrl(char * url, const HttpRequestMethod& method)
 {
     return urlParse(method, url, NULL);
 }
 
-/*
- * Create a Request from a URL.
- *
- * If the request cannot be created cleanly, NULL is returned
- */
-HttpRequest *
-HttpRequest::CreateFromUrl(char * url)
-{
-    return urlParse(Http::METHOD_GET, url, NULL);
-}
-
 /**
  * Are responses to this request possible cacheable ?
  * If false then no matter what the response must not be cached.

diff --git a/src/HttpRequest.h b/src/HttpRequest.h
index d197e1e5fccb1b3170d2f3ee532ed57bd4889816..b4653f6cbe3797bafd2c874e36b7ef0b8550c680 100644 (file)
--- a/src/HttpRequest.h
+++ b/src/HttpRequest.h
@@ -193,9 +193,7 @@ public:
 
     static void httpRequestPack(void *obj, Packable *p);
 
-    static HttpRequest * CreateFromUrlAndMethod(char * url, const HttpRequestMethod& method);
-
-    static HttpRequest * CreateFromUrl(char * url);
+    static HttpRequest * CreateFromUrl(char * url, const HttpRequestMethod &method = Http::METHOD_GET);
 
     ConnStateData *pinnedConnection();
 

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 2ea960e90f77ffcc898e6069b1df350d353553e5..df18d5e51a6bb9228b10ae56cd711958743850cd 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -344,7 +344,7 @@ clientBeginRequest(const HttpRequestMethod& method, char const *url, CSCB * stre
     http->uri = (char *)xcalloc(url_sz, 1);
     strcpy(http->uri, url);
 
-    if ((request = HttpRequest::CreateFromUrlAndMethod(http->uri, method)) == NULL) {
+    if ((request = HttpRequest::CreateFromUrl(http->uri, method)) == NULL) {
         debugs(85, 5, "Invalid URL: " << http->uri);
         return -1;
     }

diff --git a/src/htcp.cc b/src/htcp.cc
index 17ff4b85ed3a72ee651577f782fced541e74dc52..28aabb0f16631c65a0067bdf4f7db6310ac76efa 100644 (file)
--- a/src/htcp.cc
+++ b/src/htcp.cc
@@ -711,7 +711,7 @@ htcpUnpackSpecifier(char *buf, int sz)
     // Parse the request
     method.HttpRequestMethodXXX(s->method);
 
-    s->request = HttpRequest::CreateFromUrlAndMethod(s->uri, method == Http::METHOD_NONE ? HttpRequestMethod(Http::METHOD_GET) : method);
+    s->request = HttpRequest::CreateFromUrl(s->uri, method == Http::METHOD_NONE ? HttpRequestMethod(Http::METHOD_GET) : method);
 
     if (s->request)
         HTTPMSGLOCK(s->request);

diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
index 7fd3cd0501f02f93c662b626af4310c8427f85f7..45506c726e666bde3f7c6a158398e6b6797e2eee 100644 (file)
--- a/src/servers/FtpServer.cc
+++ b/src/servers/FtpServer.cc
@@ -722,7 +722,7 @@ Ftp::Server::parseOneRequest()
                        &params : NULL;
     calcUri(path);
     char *newUri = xstrdup(uri.c_str());
-    HttpRequest *const request = HttpRequest::CreateFromUrlAndMethod(newUri, method);
+    HttpRequest *const request = HttpRequest::CreateFromUrl(newUri, method);
     if (!request) {
         debugs(33, 5, "Invalid FTP URL: " << uri);
         uri.clear();

diff --git a/src/servers/Http1Server.cc b/src/servers/Http1Server.cc
index 364185e0375f834a0dda7abc3b3e14e7b9be25ca..a843586be73a68d1d59adf1cb0158621eaf542e9 100644 (file)
--- a/src/servers/Http1Server.cc
+++ b/src/servers/Http1Server.cc
@@ -131,7 +131,7 @@ Http::One::Server::buildHttpRequest(Http::Stream *context)
         return false;
     }
 
-    if ((request = HttpRequest::CreateFromUrlAndMethod(http->uri, parser_->method())) == NULL) {
+    if ((request = HttpRequest::CreateFromUrl(http->uri, parser_->method())) == NULL) {
         debugs(33, 5, "Invalid URL: " << http->uri);
         // setLogUri should called before repContext->setReplyToError
         setLogUri(http, http->uri, true);

diff --git a/src/tests/stub_HttpRequest.cc b/src/tests/stub_HttpRequest.cc
index 70cc689f687ba83027c526a349a619c53c619852..eddd4261024b4239bd2a0edc0ba5ce6bf8f3821a 100644 (file)
--- a/src/tests/stub_HttpRequest.cc
+++ b/src/tests/stub_HttpRequest.cc
@@ -47,8 +47,7 @@ int HttpRequest::prefixLen() const STUB_RETVAL(0)
 void HttpRequest::swapOut(StoreEntry *) STUB
 void HttpRequest::pack(Packable *) const STUB
 void HttpRequest::httpRequestPack(void *, Packable *) STUB
-HttpRequest * HttpRequest::CreateFromUrlAndMethod(char *, const HttpRequestMethod &) STUB_RETVAL(NULL)
-HttpRequest * HttpRequest::CreateFromUrl(char *) STUB_RETVAL(NULL)
+HttpRequest * HttpRequest::CreateFromUrl(char *, const HttpRequestMethod &) STUB_RETVAL(NULL)
 ConnStateData *HttpRequest::pinnedConnection() STUB_RETVAL(NULL)
 const SBuf HttpRequest::storeId() STUB_RETVAL(SBuf("."))
 void HttpRequest::ignoreRange(const char *) STUB

diff --git a/src/tests/testHttpRequest.cc b/src/tests/testHttpRequest.cc
index cd33afd179dfd74592da5d2d84420328e8aa0776..322e5971eb9074db5d4888fec99d0d9a35a54d69 100644 (file)
--- a/src/tests/testHttpRequest.cc
+++ b/src/tests/testHttpRequest.cc
@@ -38,14 +38,25 @@ testHttpRequest::setUp()
  * Test creating an HttpRequest object from a Url and method
  */
 void
-testHttpRequest::testCreateFromUrlAndMethod()
+testHttpRequest::testCreateFromUrl()
 {
-    /* vanilla url */
+    /* vanilla url, implict method */
     unsigned short expected_port;
     char * url = xstrdup("http://foo:90/bar");
-    HttpRequest *aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET);
+    HttpRequest *aRequest = HttpRequest::CreateFromUrl(url);
+    expected_port = 90;
+    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(SBuf("/bar"), aRequest->url.path());
+    CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast<AnyP::ProtocolType>(aRequest->url.getScheme()));
+    CPPUNIT_ASSERT_EQUAL(String("http://foo:90/bar"), String(url));
+    xfree(url);
+
+    /* vanilla url */
+    url = xstrdup("http://foo:90/bar");
+    aRequest = HttpRequest::CreateFromUrl(url, Http::METHOD_GET);
     expected_port = 90;
-    HttpRequest *nullRequest = NULL;
     CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET);
     CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->url.host()));
@@ -56,7 +67,7 @@ testHttpRequest::testCreateFromUrlAndMethod()
 
     /* vanilla url, different method */
     url = xstrdup("http://foo/bar");
-    aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_PUT);
+    aRequest = HttpRequest::CreateFromUrl(url, Http::METHOD_PUT);
     expected_port = 80;
     CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_PUT);
@@ -67,14 +78,15 @@ testHttpRequest::testCreateFromUrlAndMethod()
     xfree(url);
 
     /* a connect url with non-CONNECT data */
+    HttpRequest *nullRequest = nullptr;
     url = xstrdup(":foo/bar");
-    aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_CONNECT);
+    aRequest = HttpRequest::CreateFromUrl(url, Http::METHOD_CONNECT);
     xfree(url);
     CPPUNIT_ASSERT_EQUAL(nullRequest, aRequest);
 
     /* a CONNECT url with CONNECT data */
     url = xstrdup("foo:45");
-    aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_CONNECT);
+    aRequest = HttpRequest::CreateFromUrl(url, Http::METHOD_CONNECT);
     expected_port = 45;
     CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_CONNECT);
@@ -83,26 +95,8 @@ testHttpRequest::testCreateFromUrlAndMethod()
     CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_NONE, static_cast<AnyP::ProtocolType>(aRequest->url.getScheme()));
     CPPUNIT_ASSERT_EQUAL(String("foo:45"), String(url));
     xfree(url);
-}
 
-/*
- * Test creating an HttpRequest object from a Url alone.
- */
-void
-testHttpRequest::testCreateFromUrl()
-{
-    /* vanilla url */
-    unsigned short expected_port;
-    char * url = xstrdup("http://foo:90/bar");
-    HttpRequest *aRequest = HttpRequest::CreateFromUrl(url);
-    expected_port = 90;
-    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(SBuf("/bar"), aRequest->url.path());
-    CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, static_cast<AnyP::ProtocolType>(aRequest->url.getScheme()));
-    CPPUNIT_ASSERT_EQUAL(String("http://foo:90/bar"), String(url));
-    xfree(url);
+    // XXX: check METHOD_NONE input handling
 }
 
 /*
@@ -117,7 +111,7 @@ testHttpRequest::testIPv6HostColonBug()
 
     /* valid IPv6 address without port */
     url = xstrdup("http://[2000:800::45]/foo");
-    aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET);
+    aRequest = HttpRequest::CreateFromUrl(url, Http::METHOD_GET);
     expected_port = 80;
     CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET);
@@ -129,7 +123,7 @@ testHttpRequest::testIPv6HostColonBug()
 
     /* valid IPv6 address with port */
     url = xstrdup("http://[2000:800::45]:90/foo");
-    aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET);
+    aRequest = HttpRequest::CreateFromUrl(url, Http::METHOD_GET);
     expected_port = 90;
     CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET);
@@ -141,7 +135,7 @@ testHttpRequest::testIPv6HostColonBug()
 
     /* IPv6 address as invalid (bug trigger) */
     url = xstrdup("http://2000:800::45/foo");
-    aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET);
+    aRequest = HttpRequest::CreateFromUrl(url, Http::METHOD_GET);
     expected_port = 80;
     CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET);

diff --git a/src/tests/testHttpRequest.h b/src/tests/testHttpRequest.h
index 251b7c2aa0ade340f9f82ccc0be1bb523d59b87a..c6d31ccf2146d075230acf3d209f860640ef0a42 100644 (file)
--- a/src/tests/testHttpRequest.h
+++ b/src/tests/testHttpRequest.h
@@ -18,7 +18,6 @@
 class testHttpRequest : public CPPUNIT_NS::TestFixture
 {
     CPPUNIT_TEST_SUITE( testHttpRequest );
-    CPPUNIT_TEST( testCreateFromUrlAndMethod );
     CPPUNIT_TEST( testCreateFromUrl );
     CPPUNIT_TEST( testIPv6HostColonBug );
     CPPUNIT_TEST( testSanityCheckStartLine );
@@ -28,7 +27,6 @@ public:
     void setUp();
 
 protected:
-    void testCreateFromUrlAndMethod();
     void testCreateFromUrl();
     void testIPv6HostColonBug();
     void testSanityCheckStartLine();