From: Amos Jeffries Date: Thu, 17 Mar 2016 03:28:14 +0000 (+1300) Subject: Cleanup: de-duplicate HttpRequest CreateFromUrl functions X-Git-Tag: SQUID_4_0_8~30 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d6067ac1ee0abb9d8a70fbdc56e31c47cad59c5a;p=thirdparty%2Fsquid.git Cleanup: de-duplicate HttpRequest CreateFromUrl functions --- diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index 9f61778237..6d75c20305 100644 --- 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 d197e1e5fc..b4653f6cbe 100644 --- 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 2ea960e90f..df18d5e51a 100644 --- 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 17ff4b85ed..28aabb0f16 100644 --- 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 7fd3cd0501..45506c726e 100644 --- a/src/servers/FtpServer.cc +++ b/src/servers/FtpServer.cc @@ -722,7 +722,7 @@ Ftp::Server::parseOneRequest() ¶ms : 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 364185e037..a843586be7 100644 --- 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 70cc689f68..eddd426102 100644 --- 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 cd33afd179..322e5971eb 100644 --- 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(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(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(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 251b7c2aa0..c6d31ccf21 100644 --- 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();