From c2a7cefd2e76ef704cd3cd7cec6577f220d01bf6 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Fri, 26 Oct 2012 05:36:45 -0600 Subject: [PATCH] Polish: Http::MethodType upgrade Convert the METHOD_* enum into a class with accessors to determine method string/image, idempotency, safety (as per RFC 2616 terminology). * creates the initial http/ directory and http/libsquid-http.la library as per SourceLayout needs - libsquid-http is chosen to prevent symbols mistakes with libhttp which may exist on some systems under the libhttp or http/libhttp name. enum changes: * migrates _method_t to Http::MethodType and makes it a enum with auto-generated strings array provided by the new library. * the enum list is extended to contain all methods in the HTTPbis WG new method registry - documentation now references that registry and HTTPbis draft document listing methods HttpRequest class changes: * cacheable() changed to maybeCacheable() accessor - to reflect that it determines NON-cacheability with certainty, but when returns TRUE it can still be overridden by Http Reply details and/or local environment details. - updated to cache the complex calculation result. HttpRequestMethod class changes: * The new isIdempotent() and isHttpSafe() accessors are altered slightly from the original code to match the RFCs. The existing code produces incorrect true/false values on some methods. * new accessor shouldInvalidate() which produces true if the request method alone is enough to determine an RFC SHOULD invalidate condition. - purgesOthers() accessor is still present with the old behaviour, but for better caching we should convert the code to using shouldInvalidate() * new accessors respMaybeCacheable() which produces true if the request method alone is enough to determine an RFC MAY cache condition. - replaces isCacheable() accessor which did not produce any certainty as implied by the name, and was producing incorrect assessments anyway for some methods. - it is more of a test whether *Squid* is known to be able to cache the response to this method, than whether the RFC permit it. - There are several cases where it should produce true according to the RFCs but isCacheable() was doing false due to bugs or missing feature support. These cases are now documented with reasons and things to test or fix before they can be made to return true. * the remainders of obsolete extension_methods directive code is dropped Global httpCachable(method) function dropped - replaced by HttpRequest::maybeCacheable() accessor which does better cacheability checks. ... and there are a lot of METHOD_FOO -> Http::METHOD_FOO symbol-only changes fluffing out the patch size. --- configure.ac | 1 + src/AccessLogEntry.h | 2 +- src/HttpReply.cc | 4 +- src/HttpRequest.cc | 46 ++-- src/HttpRequest.h | 6 +- src/HttpRequestMethod.cc | 339 +++++++++++++++-------------- src/HttpRequestMethod.h | 137 +++++------- src/Makefile.am | 17 +- src/acl/Asn.cc | 4 +- src/adaptation/ecap/MessageRep.cc | 2 +- src/adaptation/icap/ModXact.cc | 4 +- src/auth/digest/UserRequest.cc | 4 +- src/cache_cf.cc | 1 - src/client_side.cc | 20 +- src/client_side_reply.cc | 44 ++-- src/client_side_request.cc | 20 +- src/errorpage.cc | 2 +- src/esi/Include.cc | 5 +- src/forward.cc | 27 +-- src/ftp.cc | 6 +- src/htcp.cc | 2 +- src/http.cc | 21 +- src/http/Makefile.am | 14 ++ src/http/MethodType.h | 94 ++++++++ src/icmp/net_db.cc | 2 +- src/icp_v2.cc | 4 +- src/icp_v3.cc | 2 +- src/mgr/ActionParams.cc | 11 +- src/mgr/ActionParams.h | 2 +- src/mime.cc | 7 +- src/neighbors.cc | 4 +- src/store.cc | 12 +- src/store_digest.cc | 2 +- src/tests/testHttpRequest.cc | 28 +-- src/tests/testHttpRequestMethod.cc | 56 ++--- src/tests/testRock.cc | 2 +- src/tests/testUfs.cc | 2 +- src/url.cc | 127 +++++------ src/urn.cc | 4 +- 39 files changed, 573 insertions(+), 514 deletions(-) create mode 100644 src/http/Makefile.am create mode 100644 src/http/MethodType.h diff --git a/configure.ac b/configure.ac index e4927993f4..27265260ec 100644 --- a/configure.ac +++ b/configure.ac @@ -3565,6 +3565,7 @@ AC_CONFIG_FILES([\ src/esi/Makefile \ src/eui/Makefile \ src/format/Makefile \ + src/http/Makefile \ src/icmp/Makefile \ src/ident/Makefile \ src/ip/Makefile \ diff --git a/src/AccessLogEntry.h b/src/AccessLogEntry.h index 96e2ab7ce5..eb011e8ea0 100644 --- a/src/AccessLogEntry.h +++ b/src/AccessLogEntry.h @@ -81,7 +81,7 @@ public: { public: - HttpDetails() : method(METHOD_NONE), code(0), content_type(NULL), + HttpDetails() : method(Http::METHOD_NONE), code(0), content_type(NULL), timedout(false), aborted(false) {} HttpRequestMethod method; diff --git a/src/HttpReply.cc b/src/HttpReply.cc index e0dd59162a..1d91dc5415 100644 --- a/src/HttpReply.cc +++ b/src/HttpReply.cc @@ -428,7 +428,7 @@ HttpReply::bodySize(const HttpRequestMethod& method) const { if (sline.version.major < 1) return -1; - else if (method.id() == METHOD_HEAD) + else if (method.id() == Http::METHOD_HEAD) return 0; else if (sline.status == HTTP_OK) (void) 0; /* common case, continue */ @@ -532,7 +532,7 @@ HttpReply::expectingBody(const HttpRequestMethod& req_method, int64_t& theSize) { bool expectBody = true; - if (req_method == METHOD_HEAD) + if (req_method == Http::METHOD_HEAD) expectBody = false; else if (sline.status == HTTP_NO_CONTENT) expectBody = false; diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index e8e97c156a..a8a95803f8 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -87,7 +87,7 @@ HttpRequest::initHTTP(const HttpRequestMethod& aMethod, AnyP::ProtocolType aProt void HttpRequest::init() { - method = METHOD_NONE; + method = Http::METHOD_NONE; protocol = AnyP::PROTO_NONE; urlpath = NULL; login[0] = '\0'; @@ -293,7 +293,7 @@ HttpRequest::sanityCheckStartLine(MemBuf *buf, const size_t hdr_len, http_status } /* See if the request buffer starts with a known HTTP request method. */ - if (HttpRequestMethod(buf->content(),NULL) == METHOD_NONE) { + if (HttpRequestMethod(buf->content(),NULL) == Http::METHOD_NONE) { debugs(73, 3, "HttpRequest::sanityCheckStartLine: did not find HTTP request method"); *error = HTTP_INVALID_HEADER; return false; @@ -308,7 +308,7 @@ HttpRequest::parseFirstLine(const char *start, const char *end) const char *t = start + strcspn(start, w_space); method = HttpRequestMethod(start, t); - if (method == METHOD_NONE) + if (method == Http::METHOD_NONE) return false; start = t + strspn(t, w_space); @@ -568,15 +568,15 @@ HttpRequest::CreateFromUrlAndMethod(char * url, const HttpRequestMethod& method) HttpRequest * HttpRequest::CreateFromUrl(char * url) { - return urlParse(METHOD_GET, url, NULL); + 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. */ bool -HttpRequest::cacheable() const +HttpRequest::maybeCacheable() { // Intercepted request with Host: header which cannot be trusted. // Because it failed verification, or someone bypassed the security tests @@ -585,27 +585,29 @@ HttpRequest::cacheable() const if (!flags.hostVerified && (flags.intercepted || flags.spoofClientIp)) return false; - if (protocol == AnyP::PROTO_HTTP) - return httpCachable(method); - - /* - * The below looks questionable: what non HTTP protocols use connect, - * trace, put and post? RC - */ + switch(protocol) + { + case AnyP::PROTO_HTTP: + if (!method.respMaybeCacheable()) + return false; - if (!method.isCacheble()) - return false; + // XXX: this would seem the correct place to detect request cache-controls + // no-store, private and related which block cacheability + break; - /* - * XXX POST may be cached sometimes.. ignored - * for now - */ - if (protocol == AnyP::PROTO_GOPHER) - return gopherCachable(this); + case AnyP::PROTO_GOPHER: + if (!gopherCachable(this)) + return false; + break; - if (protocol == AnyP::PROTO_CACHE_OBJECT) + case AnyP::PROTO_CACHE_OBJECT: return false; + //case AnyP::PROTO_FTP: + default: + break; + } + return true; } diff --git a/src/HttpRequest.h b/src/HttpRequest.h index 2f8ef03ba3..0b13b5ed7c 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -82,8 +82,10 @@ public: virtual HttpRequest *clone() const; - /* are responses to this request potentially cachable */ - bool cacheable() const; + /// Whether response to this request is potentially cachable + /// \retval false Not cacheable. + /// \retval true Possibly cacheable. Response factors will determine. + bool maybeCacheable(); bool conditional() const; ///< has at least one recognized If-* header diff --git a/src/HttpRequestMethod.cc b/src/HttpRequestMethod.cc index 3d5eabb51c..2f69b9379e 100644 --- a/src/HttpRequestMethod.cc +++ b/src/HttpRequestMethod.cc @@ -1,89 +1,25 @@ - /* * DEBUG: section 73 HTTP Request - * AUTHOR: Duane Wessels - * - * SQUID Web Proxy Cache http://www.squid-cache.org/ - * ---------------------------------------------------------- - * - * Squid is the result of efforts by numerous individuals from - * the Internet community; see the CONTRIBUTORS file for full - * details. Many organizations have provided support for Squid's - * development; see the SPONSORS file for full details. Squid is - * Copyrighted (C) 2001 by the Regents of the University of - * California; see the COPYRIGHT file for full details. Squid - * incorporates software developed and/or copyrighted by other - * sources; see the CREDITS file for full details. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA. - * - * Copyright (c) 2003, Robert Collins */ #include "squid.h" #include "HttpRequestMethod.h" #include "wordlist.h" -const char* HttpRequestMethod::RequestMethodStr[] = { - "NONE", - "GET", - "POST", - "PUT", - "HEAD", - "CONNECT", - "TRACE", - "PURGE", - "OPTIONS", - "DELETE", - "PROPFIND", - "PROPPATCH", - "MKCOL", - "COPY", - "MOVE", - "LOCK", - "UNLOCK", - "BMOVE", - "BDELETE", - "BPROPFIND", - "BPROPPATCH", - "BCOPY", - "SEARCH", - "SUBSCRIBE", - "UNSUBSCRIBE", - "POLL", - "REPORT", - "MKACTIVITY", - "CHECKOUT", - "MERGE", - "ERROR" -}; - -static -_method_t &operator++ (_method_t &aMethod) +static Http::MethodType & +operator++ (Http::MethodType &aMethod) { int tmp = (int)aMethod; - aMethod = (_method_t)(++tmp); + aMethod = (Http::MethodType)(++tmp); return aMethod; } -/* +/** * Construct a HttpRequestMethod from a NULL terminated string such as "GET" * or from a range of chars, * such as "GET" from "GETFOOBARBAZ" * (pass in pointer to G and pointer to F.) */ -HttpRequestMethod::HttpRequestMethod(char const *begin, char const *end) : theMethod (METHOD_NONE) +HttpRequestMethod::HttpRequestMethod(char const *begin, char const *end) : theMethod (Http::METHOD_NONE) { if (begin == NULL) return; @@ -106,137 +42,193 @@ HttpRequestMethod::HttpRequestMethod(char const *begin, char const *end) : theMe end = begin + strcspn(begin, w_space); if (end == begin) { - theMethod = METHOD_NONE; + theMethod = Http::METHOD_NONE; return; } - for (++theMethod; theMethod < METHOD_ENUM_END; ++theMethod) { - if (0 == strncasecmp(begin, RequestMethodStr[theMethod], end-begin)) { + for (++theMethod; theMethod < Http::METHOD_ENUM_END; ++theMethod) { + if (0 == strncasecmp(begin, Http::MethodType_str[theMethod], end-begin)) { return; } } // if method not found and method string is not null then it is other method - theMethod = METHOD_OTHER; + theMethod = Http::METHOD_OTHER; theImage.limitInit(begin,end-begin); } -/** \todo AYJ: this _should_ be obsolete. Since all such methods fit nicely into METHOD_OTHER now. */ -void -HttpRequestMethod::AddExtension(const char *mstr) +char const* +HttpRequestMethod::image() const { -#if 0 /* obsolete now that we have METHOD_OTHER always enabled */ - _method_t method = METHOD_NONE; - - for (++method; method < METHOD_ENUM_END; ++method) { - if (0 == strcmp(mstr, RequestMethodStr[method])) { - debugs(23, 2, "Extension method '" << mstr << "' already exists"); - return; + if (Http::METHOD_OTHER != theMethod) { + return Http::MethodType_str[theMethod]; + } else { + if (theImage.size()>0) { + return theImage.termedBuf(); + } else { + return "METHOD_OTHER"; } + } +} - if (0 != strncmp("%EXT", RequestMethodStr[method], 4)) - continue; +bool +HttpRequestMethod::isHttpSafe() const +{ + // Only a few methods are defined as safe. All others are "unsafe" - /* Don't free statically allocated "%EXTnn" string */ - RequestMethodStr[method] = xstrdup(mstr); + // NOTE: + // All known RFCs which register methods are listed in comments. + // if there is one not listed which defines methods, it needs + // checking and adding. If only to say it is known to define none. - debugs(23, DBG_IMPORTANT, "Extension method '" << mstr << "' added, enum=" << method); + switch(theMethod) + { + // RFC 2068 - none - return; - } + // RFC 2616 section 9.1.1 + case Http::METHOD_GET: + case Http::METHOD_HEAD: + case Http::METHOD_OPTIONS: - debugs(23, DBG_IMPORTANT, "WARNING: Could not add new extension method '" << mstr << "' due to lack of array space"); -#endif -} + // RFC 3253 section 3.6 + case Http::METHOD_REPORT: -void -HttpRequestMethod::Configure(SquidConfig &cfg) -{ -#if 0 /* extension methods obsolete now that we have METHOD_OTHER always enabled */ - wordlist *w = cfg.ext_methods; + // RFC 3648 - none + // RFC 3744 - none + // RFC 4437 - none + // RFC 4791 - none - while (w) { - char *s; + // RFC 4918 section 9.1 + case Http::METHOD_PROPFIND: - for (s = w->key; *s; ++s) - *s = xtoupper(*s); + // RFC 5323 section 2 + case Http::METHOD_SEARCH: - AddExtension(w->key); + // RFC 5789 - none + // RFC 5842 - none - w = w->next; - } -#endif -} + return true; -char const* -HttpRequestMethod::image() const -{ - if (METHOD_OTHER != theMethod) { - return RequestMethodStr[theMethod]; - } else { - if (theImage.size()>0) { - return theImage.termedBuf(); - } else { - return "METHOD_OTHER"; - } + default: + return false; } } bool -HttpRequestMethod::isCacheble() const +HttpRequestMethod::isIdempotent() const { - // TODO: optimize the lookup with a precomputed flags array - // XXX: the list seems wrong; e.g., Is METHOD_DELETE really cachable? - // see also http.cc::httpCachable() + // Only a few methods are defined as idempotent. + + // NOTE: + // All known RFCs which register methods are listed in comments. + // if there is one not listed which defines methods, it needs + // checking and adding. If only to say it is known to define none. + + switch(theMethod) + { + // RFC 2068 - TODO check LINK/UNLINK definition + + // RFC 2616 section 9.1.2 + case Http::METHOD_GET: + case Http::METHOD_HEAD: + case Http::METHOD_PUT: + case Http::METHOD_DELETE: + case Http::METHOD_OPTIONS: + case Http::METHOD_TRACE: + + // RFC 3253 - TODO check + // RFC 3648 - TODO check + // RFC 3744 - TODO check + // RFC 4437 - TODO check + // RFC 4791 - TODO check + + // RFC 4918 section 9 + case Http::METHOD_PROPFIND: + case Http::METHOD_PROPPATCH: + case Http::METHOD_MKCOL: + case Http::METHOD_COPY: + case Http::METHOD_MOVE: + case Http::METHOD_UNLOCK: + + // RFC 5323 - TODO check + // RFC 5789 - TODO check + // RFC 5842 - TODO check - if (theMethod == METHOD_CONNECT) - return false; + return true; - if (theMethod == METHOD_TRACE) + default: return false; + } +} - if (theMethod == METHOD_PUT) - return false; +bool +HttpRequestMethod::respMaybeCacheable() const +{ + // Only a few methods are defined as cacheable. + // All other methods from the below RFC are "MUST NOT cache" + switch(theMethod) + { + // RFC 2616 section 9 + case Http::METHOD_GET: + case Http::METHOD_HEAD: + return true; +#if WHEN_POST_CACHE_SUPPORTED + case Http::METHOD_POST: // Special case. + // RFC 2616 specifies POST as possibly cacheable + // However, Squid does not implement the required checks yet + return true; +#endif - if (theMethod == METHOD_POST) - return false; + // RFC 4918 section 9 +#if WHEN_PROPFIND_CACHE_SUPPORTED + case Http::METHOD_PROPFIND: // Special case. + // RFC 4918 specifies PROPFIND as possibly cacheable + // However, Squid does not implement the required checks yet + return true; +#endif - if (theMethod == METHOD_OTHER) - return false; + // RFC 5323 section 2 - defines no cacheable methods + + // RFC 3253 +#if WHEN_CC_NOCACHE_DOES_REVALIDATES_IS_CONFIRMED + case Http::METHOD_CHECKOUT: + case Http::METHOD_CHECKIN: + case Http::METHOD_UNCHECKOUT: + case Http::METHOD_MKWORKSPACE: + case Http::METHOD_VERSION_CONTROL: + case Http::METHOD_UPDATE: + case Http::METHOD_LABEL: + case Http::METHOD_MERGE: + case Http::METHOD_BASELINE_CONTROL: + case Http::METHOD_MKACTIVITY: + // RFC 3253 defines these methods using "MUST include Cache-Control: no-cache". + // + // XXX: follow RFC 2616 definition of "no-cache" meaning "MAY cache, always revalidate" + // XXX: or treat as unregistered/undefined methods ?? + // However, Squid may not implement the required revalidation checks yet + return ??; +#endif - return true; + // Special Squid method tokens are not cacheable. + // RFC 2616 defines all unregistered or unspecified methods as non-cacheable + // until such time as an RFC defines them cacheable. + default: + return false; + } } bool -HttpRequestMethod::purgesOthers() const +HttpRequestMethod::shouldInvalidate() const { - // TODO: optimize the lookup with a precomputed flags array - switch (theMethod) { - /* common sense suggests purging is not required? */ - case METHOD_GET: // XXX: but we do purge HEAD on successful GET - case METHOD_HEAD: - case METHOD_NONE: - case METHOD_CONNECT: - case METHOD_TRACE: - case METHOD_OPTIONS: - case METHOD_PROPFIND: - case METHOD_BPROPFIND: - case METHOD_COPY: - case METHOD_BCOPY: - case METHOD_LOCK: - case METHOD_UNLOCK: - case METHOD_SEARCH: - return false; - - /* purging mandated by RFC 2616 */ - case METHOD_POST: - case METHOD_PUT: - case METHOD_DELETE: + /* RFC 2616 section 13.10 - "MUST invalidate" */ + case Http::METHOD_POST: + case Http::METHOD_PUT: + case Http::METHOD_DELETE: return true; - /* purging suggested by common sense */ - case METHOD_PURGE: + /* Squid extension to force invalidation */ + case Http::METHOD_PURGE: return true; /* @@ -245,10 +237,37 @@ HttpRequestMethod::purgesOthers() const * understand SHOULD invalidate any entities referred to by the * Request-URI. */ - case METHOD_OTHER: - default: + case Http::METHOD_OTHER: return true; + + default: + // Methods which are known but not required to invalidate. + return false; } +} - return true; // not reached, but just in case +bool +HttpRequestMethod::purgesOthers() const +{ + if (shouldInvalidate()) + return true; + + switch (theMethod) { + /* common sense suggests purging is not required? */ + case Http::METHOD_GET: // XXX: but we do purge HEAD on successful GET + case Http::METHOD_HEAD: + case Http::METHOD_NONE: + case Http::METHOD_CONNECT: + case Http::METHOD_TRACE: + case Http::METHOD_OPTIONS: + case Http::METHOD_PROPFIND: + case Http::METHOD_COPY: + case Http::METHOD_LOCK: + case Http::METHOD_UNLOCK: + case Http::METHOD_SEARCH: + return false; + + default: + return true; + } } diff --git a/src/HttpRequestMethod.h b/src/HttpRequestMethod.h index 11090dc229..e6584a4cd8 100644 --- a/src/HttpRequestMethod.h +++ b/src/HttpRequestMethod.h @@ -1,94 +1,29 @@ -/* - * - * SQUID Web Proxy Cache http://www.squid-cache.org/ - * ---------------------------------------------------------- - * - * Squid is the result of efforts by numerous individuals from - * the Internet community; see the CONTRIBUTORS file for full - * details. Many organizations have provided support for Squid's - * development; see the SPONSORS file for full details. Squid is - * Copyrighted (C) 2001 by the Regents of the University of - * California; see the COPYRIGHT file for full details. Squid - * incorporates software developed and/or copyrighted by other - * sources; see the CREDITS file for full details. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA. - * - */ - #ifndef SQUID_HTTPREQUESTMETHOD_H #define SQUID_HTTPREQUESTMETHOD_H +#include "http/MethodType.h" +#include "SquidString.h" #include "SquidString.h" class SquidConfig; #include -enum _method_t { - METHOD_NONE, /* 000 */ - METHOD_GET, /* 001 */ - METHOD_POST, /* 010 */ - METHOD_PUT, /* 011 */ - METHOD_HEAD, /* 100 */ - METHOD_CONNECT, /* 101 */ - METHOD_TRACE, /* 110 */ - METHOD_PURGE, /* 111 */ - METHOD_OPTIONS, - METHOD_DELETE, /* RFC2616 section 9.7 */ - METHOD_PROPFIND, - METHOD_PROPPATCH, - METHOD_MKCOL, - METHOD_COPY, - METHOD_MOVE, - METHOD_LOCK, - METHOD_UNLOCK, - METHOD_BMOVE, - METHOD_BDELETE, - METHOD_BPROPFIND, - METHOD_BPROPPATCH, - METHOD_BCOPY, - METHOD_SEARCH, - METHOD_SUBSCRIBE, - METHOD_UNSUBSCRIBE, - METHOD_POLL, - METHOD_REPORT, - METHOD_MKACTIVITY, - METHOD_CHECKOUT, - METHOD_MERGE, - METHOD_OTHER, - METHOD_ENUM_END // MUST be last, (yuck) this is used as an array-initialization index constant! -}; - /** * This class represents an HTTP Request METHOD * - i.e. PUT, POST, GET etc. * It has a runtime extension facility to allow it to * efficiently support new methods - \ingroup POD */ class HttpRequestMethod { public: - static void AddExtension(const char *methodString); - static void Configure(SquidConfig &Config); +// static void Configure(SquidConfig &Config); - HttpRequestMethod() : theMethod(METHOD_NONE), theImage() {} + HttpRequestMethod() : theMethod(Http::METHOD_NONE), theImage() {} - HttpRequestMethod(_method_t const aMethod) : theMethod(aMethod), theImage() {} + HttpRequestMethod(Http::MethodType const aMethod) : theMethod(aMethod), theImage() {} /** \param begin string to convert to request method. @@ -104,19 +39,19 @@ public: return *this; } - HttpRequestMethod & operator = (_method_t const aMethod) { + HttpRequestMethod & operator = (Http::MethodType const aMethod) { theMethod = aMethod; theImage.clean(); return *this; } - bool operator == (_method_t const & aMethod) const { return theMethod == aMethod; } + bool operator == (Http::MethodType const & aMethod) const { return theMethod == aMethod; } bool operator == (HttpRequestMethod const & aMethod) const { return theMethod == aMethod.theMethod && - (theMethod != METHOD_OTHER || theImage == aMethod.theImage); + (theMethod != Http::METHOD_OTHER || theImage == aMethod.theImage); } - bool operator != (_method_t const & aMethod) const { return theMethod != aMethod; } + bool operator != (Http::MethodType const & aMethod) const { return theMethod != aMethod; } bool operator != (HttpRequestMethod const & aMethod) const { return !operator==(aMethod); } @@ -125,30 +60,62 @@ public: HttpRequestMethod& operator++() { // TODO: when this operator is used in more than one place, // replace it with HttpRequestMethods::Iterator API - // XXX: this interface can create METHOD_OTHER without an image - assert(theMethod < METHOD_ENUM_END); - theMethod = (_method_t)(1 + (int)theMethod); + // XXX: this interface can create Http::METHOD_OTHER without an image + assert(theMethod < Http::METHOD_ENUM_END); + theMethod = (Http::MethodType)(1 + (int)theMethod); return *this; } /** Get an ID representation of the method. - \retval METHOD_NONE the method is unset - \retval METHOD_OTHER the method is not recognized and has no unique ID - \retval * the method is on of the recognized HTTP methods. + * \retval Http::METHOD_NONE the method is unset + * \retval Http::METHOD_OTHER the method is not recognized and has no unique ID + * \retval * the method is on of the recognized HTTP methods. */ - _method_t id() const { return theMethod; } + Http::MethodType id() const { return theMethod; } /** Get a char string representation of the method. */ char const * image() const; - bool isCacheble() const; + /// Whether this method is defined as a "safe" in HTTP/1.1 + /// see RFC 2616 section 9.1.1 + bool isHttpSafe() const; + + /// Whether this method is defined as "idempotent" in HTTP/1.1 + /// see RFC 2616 section 9.1.2 + bool isIdempotent() const; + + /** Whether responses to this method MAY be cached. + * \retval false Not cacheable. + * \retval true Possibly cacheable. Other details will determine. + */ + bool respMaybeCacheable() const; + + /** Whether this method SHOULD (or MUST) invalidate existing cached entries. + * Invalidation is always determined by the response + * + * RFC 2616 defines invalidate as either immediate purge + * or delayed explicit revalidate all stored copies on next use. + * + * \retval true SHOULD invalidate. Response details can raise this to a MUST. + * \retval false Other details will determine. Method is not a factor. + */ + bool shouldInvalidate() const; + + /* Whether this method invalidates existing cached entries. + * Kept for backward-compatibility. This is the old 2.x-3.2 invalidation behaviour. + * + * NOTE: + * purgesOthers differs from shouldInvalidate() in that purgesOthers() returns + * true on any methods the MAY invalidate (Squid opts to do so). + * shouldInvalidate() only returns true on methods which SHOULD invalidate. + */ bool purgesOthers() const; private: static const char *RequestMethodStr[]; - _method_t theMethod; ///< Method type - String theImage; ///< Used for store METHOD_OTHER only + Http::MethodType theMethod; ///< Method type + String theImage; ///< Used for storing the Http::METHOD_OTHER only. A copy of the parsed method text. }; inline std::ostream & @@ -159,7 +126,7 @@ operator << (std::ostream &os, HttpRequestMethod const &method) } inline const char* -RequestMethodStr(const _method_t m) +RequestMethodStr(const Http::MethodType m) { return HttpRequestMethod(m).image(); } diff --git a/src/Makefile.am b/src/Makefile.am index 803c430db8..7a6b3d0164 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -43,8 +43,8 @@ check_PROGRAMS+= tests/testACLMaxUserIP endif DIST_SUBDIRS += auth -SUBDIRS += ip icmp ident log ipc mgr -DIST_SUBDIRS += ip icmp ident log ipc mgr +SUBDIRS += http ip icmp ident log ipc mgr +DIST_SUBDIRS += http ip icmp ident log ipc mgr if ENABLE_SSL SUBDIRS += ssl @@ -628,6 +628,7 @@ squid_LDADD = \ anyp/libanyp.la \ comm/libcomm.la \ eui/libeui.la \ + http/libsquid-http.la \ icmp/libicmp.la icmp/libicmp-core.la \ log/liblog.la \ format/libformat.la \ @@ -1324,6 +1325,7 @@ tests_testACLMaxUserIP_SOURCES= \ nodist_tests_testACLMaxUserIP_SOURCES= \ $(TESTSOURCES) tests_testACLMaxUserIP_LDADD= \ + http/libsquid-http.la \ $(AUTH_ACL_LIBS) \ ident/libident.la \ acl/libacls.la \ @@ -1569,6 +1571,7 @@ nodist_tests_testCacheManager_SOURCES = \ $(DISKIO_GEN_SOURCE) # comm.cc only requires comm/libcomm.la until fdc_table is dead. tests_testCacheManager_LDADD = \ + http/libsquid-http.la \ $(AUTH_ACL_LIBS) \ ident/libident.la \ acl/libacls.la \ @@ -1746,6 +1749,7 @@ nodist_tests_testDiskIO_SOURCES= \ SquidMath.h \ swap_log_op.cc tests_testDiskIO_LDADD = \ + http/libsquid-http.la \ SquidConfig.o \ CommCalls.o \ DnsLookupDetails.o \ @@ -1986,6 +1990,7 @@ nodist_tests_testEvent_SOURCES = \ $(BUILT_SOURCES) \ $(DISKIO_GEN_SOURCE) tests_testEvent_LDADD = \ + http/libsquid-http.la \ $(AUTH_ACL_LIBS) \ ident/libident.la \ acl/libacls.la \ @@ -2228,6 +2233,7 @@ nodist_tests_testEventLoop_SOURCES = \ $(BUILT_SOURCES) \ $(DISKIO_GEN_SOURCE) tests_testEventLoop_LDADD = \ + http/libsquid-http.la \ $(AUTH_ACL_LIBS) \ ident/libident.la \ acl/libacls.la \ @@ -2464,6 +2470,7 @@ nodist_tests_test_http_range_SOURCES = \ $(BUILT_SOURCES) \ $(DISKIO_GEN_SOURCE) tests_test_http_range_LDADD = \ + http/libsquid-http.la \ $(AUTH_ACL_LIBS) \ ident/libident.la \ acl/libacls.la \ @@ -2530,6 +2537,7 @@ tests_testHttpParser_SOURCES = \ nodist_tests_testHttpParser_SOURCES = \ $(TESTSOURCES) tests_testHttpParser_LDADD= \ + http/libsquid-http.la \ SquidConfig.o \ base/libbase.la \ ip/libip.la \ @@ -2757,6 +2765,7 @@ tests_testHttpRequest_LDADD = \ comm/libcomm.la \ log/liblog.la \ format/libformat.la \ + http/libsquid-http.la \ $(REPL_OBJS) \ $(ADAPTATION_LIBS) \ $(ESI_LIBS) \ @@ -2916,6 +2925,7 @@ nodist_tests_testStore_SOURCES= \ swap_log_op.cc tests_testStore_LDADD= \ + http/libsquid-http.la \ $(AUTH_ACL_LIBS) \ ident/libident.la \ acl/libacls.la \ @@ -3138,6 +3148,7 @@ nodist_tests_testUfs_SOURCES = \ SquidMath.h \ swap_log_op.cc tests_testUfs_LDADD = \ + http/libsquid-http.la \ CommCalls.o \ DnsLookupDetails.o \ $(AUTH_ACL_LIBS) \ @@ -3299,6 +3310,7 @@ nodist_tests_testRock_SOURCES = \ SquidMath.h \ $(TESTSOURCES) tests_testRock_LDADD = \ + http/libsquid-http.la \ libsquid.la \ comm/libcomm.la \ anyp/libanyp.la \ @@ -3838,6 +3850,7 @@ tests_testURL_SOURCES = \ nodist_tests_testURL_SOURCES = \ $(BUILT_SOURCES) tests_testURL_LDADD = \ + http/libsquid-http.la \ anyp/libanyp.la \ $(AUTH_ACL_LIBS) \ ident/libident.la \ diff --git a/src/acl/Asn.cc b/src/acl/Asn.cc index f53420b440..0cda034e6c 100644 --- a/src/acl/Asn.cc +++ b/src/acl/Asn.cc @@ -248,8 +248,8 @@ asnCacheStart(int as) assert(NULL != req); asState->request = HTTPMSGLOCK(req); - if ((e = storeGetPublic(asres, METHOD_GET)) == NULL) { - e = storeCreateEntry(asres, asres, RequestFlags(), METHOD_GET); + if ((e = storeGetPublic(asres, Http::METHOD_GET)) == NULL) { + e = storeCreateEntry(asres, asres, RequestFlags(), Http::METHOD_GET); asState->sc = storeClientListAdd(e, asState); FwdState::fwdStart(Comm::ConnectionPointer(), e, asState->request); } else { diff --git a/src/adaptation/ecap/MessageRep.cc b/src/adaptation/ecap/MessageRep.cc index 156e63d60a..b3b4fa2e64 100644 --- a/src/adaptation/ecap/MessageRep.cc +++ b/src/adaptation/ecap/MessageRep.cc @@ -227,7 +227,7 @@ Adaptation::Ecap::RequestLineRep::method(const Name &aMethod) const int id = aMethod.hostId(); Must(METHOD_NONE < id && id < METHOD_ENUM_END); Must(id != METHOD_OTHER); - theMessage.method = HttpRequestMethod(static_cast<_method_t>(id)); + theMessage.method = HttpRequestMethod(static_cast(id)); } else { const std::string &image = aMethod.image(); theMessage.method = HttpRequestMethod(image.data(), diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index a6d3259d70..9c83f912a2 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -1746,12 +1746,12 @@ void Adaptation::Icap::ModXact::estimateVirginBody() else if (HttpRequest *req = dynamic_cast(msg)) method = req->method; else - method = METHOD_NONE; + method = Http::METHOD_NONE; int64_t size; // expectingBody returns true for zero-sized bodies, but we will not // get a pipe for that body, so we treat the message as bodyless - if (method != METHOD_NONE && msg->expectingBody(method, size) && size) { + if (method != Http::METHOD_NONE && msg->expectingBody(method, size) && size) { debugs(93, 6, HERE << "expects virgin body from " << virgin.body_pipe << "; size: " << size); diff --git a/src/auth/digest/UserRequest.cc b/src/auth/digest/UserRequest.cc index d53c340029..dfd72462aa 100644 --- a/src/auth/digest/UserRequest.cc +++ b/src/auth/digest/UserRequest.cc @@ -104,7 +104,7 @@ Auth::Digest::UserRequest::authenticate(HttpRequest * request, ConnStateData * c return; } - if (static_cast(Auth::Config::Find("digest"))->PostWorkaround && request->method != METHOD_GET) { + if (static_cast(Auth::Config::Find("digest"))->PostWorkaround && request->method != Http::METHOD_GET) { /* Ugly workaround for certain very broken browsers using the * wrong method to calculate the request-digest on POST request. * This should be deleted once Digest authentication becomes more @@ -113,7 +113,7 @@ Auth::Digest::UserRequest::authenticate(HttpRequest * request, ConnStateData * c */ DigestCalcResponse(SESSIONKEY, authenticateDigestNonceNonceb64(digest_request->nonce), digest_request->nc, digest_request->cnonce, digest_request->qop, - RequestMethodStr(METHOD_GET), digest_request->uri, HA2, Response); + RequestMethodStr(Http::METHOD_GET), digest_request->uri, HA2, Response); if (strcasecmp(digest_request->response, Response)) { auth_user->credentials(Auth::Failed); diff --git a/src/cache_cf.cc b/src/cache_cf.cc index c1807b61e2..58bab293b1 100644 --- a/src/cache_cf.cc +++ b/src/cache_cf.cc @@ -913,7 +913,6 @@ configDoConfigure(void) Config2.effectiveGroupID = grp->gr_gid; } - HttpRequestMethod::Configure(Config); #if USE_SSL debugs(3, DBG_IMPORTANT, "Initializing https proxy context"); diff --git a/src/client_side.cc b/src/client_side.cc index 5af18295f8..2973d3a6f9 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -858,9 +858,9 @@ clientIsContentLengthValid(HttpRequest * r) { switch (r->method.id()) { - case METHOD_GET: + case Http::METHOD_GET: - case METHOD_HEAD: + case Http::METHOD_HEAD: /* We do not want to see a request entity on GET/HEAD requests */ return (r->content_length <= 0 || Config.onoff.request_entities); @@ -2149,7 +2149,7 @@ parseHttpRequest(ConnStateData *csd, HttpParser *hp, HttpRequestMethod * method_ int r; /* pre-set these values to make aborting simpler */ - *method_p = METHOD_NONE; + *method_p = Http::METHOD_NONE; /* NP: don't be tempted to move this down or remove again. * It's the only DDoS protection old-String has against long URL */ @@ -2211,7 +2211,7 @@ parseHttpRequest(ConnStateData *csd, HttpParser *hp, HttpRequestMethod * method_ *method_p = HttpRequestMethod(&hp->buf[hp->req.m_start], &hp->buf[hp->req.m_end]+1); /* deny CONNECT via accelerated ports */ - if (*method_p == METHOD_CONNECT && csd && csd->port && csd->port->accel) { + if (*method_p == Http::METHOD_CONNECT && csd && csd->port && csd->port->accel) { debugs(33, DBG_IMPORTANT, "WARNING: CONNECT method received on " << csd->port->protocol << " Accelerator port " << csd->port->s.GetPort() ); /* XXX need a way to say "this many character length string" */ debugs(33, DBG_IMPORTANT, "WARNING: for request: " << hp->buf); @@ -2219,7 +2219,7 @@ parseHttpRequest(ConnStateData *csd, HttpParser *hp, HttpRequestMethod * method_ return parseHttpRequestAbort(csd, "error:method-not-allowed"); } - if (*method_p == METHOD_NONE) { + if (*method_p == Http::METHOD_NONE) { /* XXX need a way to say "this many character length string" */ debugs(33, DBG_IMPORTANT, "clientParseRequestMethod: Unsupported method in request '" << hp->buf << "'"); hp->request_parse_status = HTTP_METHOD_NOT_ALLOWED; @@ -2445,7 +2445,7 @@ ConnStateData::checkHeaderLimits() clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); repContext->setReplyToError(ERR_TOO_BIG, - HTTP_BAD_REQUEST, METHOD_NONE, NULL, + HTTP_BAD_REQUEST, Http::METHOD_NONE, NULL, clientConnection->remote, NULL, NULL, NULL); context->registerWithConn(); context->pullData(); @@ -2713,7 +2713,7 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c unsupportedTe = te.size() && te != "identity"; } // else implied identity coding - mustReplyToOptions = (method == METHOD_OPTIONS) && + mustReplyToOptions = (method == Http::METHOD_OPTIONS) && (request->header.getInt64(HDR_MAX_FORWARDS) == 0); if (!urlCheckRequest(request) || mustReplyToOptions || unsupportedTe) { clientStreamNode *node = context->getClientReplyContext(); @@ -2760,7 +2760,7 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c clientSetKeepaliveFlag(http); // Let tunneling code be fully responsible for CONNECT requests - if (http->request->method == METHOD_CONNECT) { + if (http->request->method == Http::METHOD_CONNECT) { context->mayUseConnection(true); conn->flags.readMore = false; } @@ -2788,7 +2788,7 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c assert (repContext); conn->quitAfterError(request); repContext->setReplyToError(ERR_TOO_BIG, - HTTP_REQUEST_ENTITY_TOO_LARGE, METHOD_NONE, NULL, + HTTP_REQUEST_ENTITY_TOO_LARGE, Http::METHOD_NONE, NULL, conn->clientConnection->remote, http->request, NULL, NULL); assert(context->http->out.offset == 0); context->pullData(); @@ -3216,7 +3216,7 @@ ConnStateData::requestTimeout(const CommTimeoutCbParams &io) clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); repContext->setReplyToError(ERR_LIFETIME_EXP, - HTTP_REQUEST_TIMEOUT, METHOD_NONE, "N/A", &CachePeer.sin_addr, + HTTP_REQUEST_TIMEOUT, Http::METHOD_NONE, "N/A", &CachePeer.sin_addr, NULL, NULL, NULL); /* No requests can be outstanded */ assert(chr == NULL); diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc index befef0d35e..44f9b35634 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -534,7 +534,7 @@ clientReplyContext::cacheHit(StoreIOBuffer result) return; } - if (r->method == METHOD_PURGE) { + if (r->method == Http::METHOD_PURGE) { removeClientStoreReference(&sc, http); e = NULL; purgeRequest(); @@ -635,13 +635,13 @@ clientReplyContext::processMiss() } /** Check if its a PURGE request to be actioned. */ - if (r->method == METHOD_PURGE) { + if (r->method == Http::METHOD_PURGE) { purgeRequest(); return; } /** Check if its an 'OTHER' request. Purge all cached entries if so and continue. */ - if (r->method == METHOD_OTHER) { + if (r->method == Http::METHOD_OTHER) { purgeAllCached(); } @@ -779,7 +779,7 @@ clientReplyContext::purgeRequestFindObjectToPurge() // TODO: can we use purgeAllCached() here instead of doing the // getPublicByRequestMethod() dance? - StoreEntry::getPublicByRequestMethod(this, http->request, METHOD_GET); + StoreEntry::getPublicByRequestMethod(this, http->request, Http::METHOD_GET); } // Purges all entries with a given url @@ -795,13 +795,13 @@ purgeEntriesByUrl(HttpRequest * req, const char *url) bool get_or_head_sent = false; #endif - for (HttpRequestMethod m(METHOD_NONE); m != METHOD_ENUM_END; ++m) { - if (m.isCacheble()) { + for (HttpRequestMethod m(Http::METHOD_NONE); m != Http::METHOD_ENUM_END; ++m) { + if (m.respMaybeCacheable()) { if (StoreEntry *entry = storeGetPublic(url, m)) { debugs(88, 5, "purging " << RequestMethodStr(m) << ' ' << url); #if USE_HTCP neighborsHtcpClear(entry, url, req, m, HTCP_CLR_INVALIDATION); - if (m == METHOD_GET || m == METHOD_HEAD) { + if (m == Http::METHOD_GET || m == Http::METHOD_HEAD) { get_or_head_sent = true; } #endif @@ -812,7 +812,7 @@ purgeEntriesByUrl(HttpRequest * req, const char *url) #if USE_HTCP if (!get_or_head_sent) { - neighborsHtcpClear(NULL, url, req, HttpRequestMethod(METHOD_GET), HTCP_CLR_INVALIDATION); + neighborsHtcpClear(NULL, url, req, HttpRequestMethod(Http::METHOD_GET), HTCP_CLR_INVALIDATION); } #endif } @@ -844,7 +844,7 @@ clientReplyContext::purgeFoundGet(StoreEntry *newEntry) { if (newEntry->isNull()) { lookingforstore = 2; - StoreEntry::getPublicByRequestMethod(this, http->request, METHOD_HEAD); + StoreEntry::getPublicByRequestMethod(this, http->request, Http::METHOD_HEAD); } else purgeFoundObject (newEntry); } @@ -923,7 +923,7 @@ clientReplyContext::purgeDoMissPurge() { http->logType = LOG_TCP_MISS; lookingforstore = 3; - StoreEntry::getPublicByRequestMethod(this,http->request, METHOD_GET); + StoreEntry::getPublicByRequestMethod(this,http->request, Http::METHOD_GET); } void @@ -937,14 +937,14 @@ clientReplyContext::purgeDoPurgeGet(StoreEntry *newEntry) /* Release the cached URI */ debugs(88, 4, "clientPurgeRequest: GET '" << newEntry->url() << "'" ); #if USE_HTCP - neighborsHtcpClear(newEntry, NULL, http->request, HttpRequestMethod(METHOD_GET), HTCP_CLR_PURGE); + neighborsHtcpClear(newEntry, NULL, http->request, HttpRequestMethod(Http::METHOD_GET), HTCP_CLR_PURGE); #endif newEntry->release(); purgeStatus = HTTP_OK; } lookingforstore = 4; - StoreEntry::getPublicByRequestMethod(this, http->request, METHOD_HEAD); + StoreEntry::getPublicByRequestMethod(this, http->request, Http::METHOD_HEAD); } void @@ -953,7 +953,7 @@ clientReplyContext::purgeDoPurgeHead(StoreEntry *newEntry) if (newEntry && !newEntry->isNull()) { debugs(88, 4, "clientPurgeRequest: HEAD '" << newEntry->url() << "'" ); #if USE_HTCP - neighborsHtcpClear(newEntry, NULL, http->request, HttpRequestMethod(METHOD_HEAD), HTCP_CLR_PURGE); + neighborsHtcpClear(newEntry, NULL, http->request, HttpRequestMethod(Http::METHOD_HEAD), HTCP_CLR_PURGE); #endif newEntry->release(); purgeStatus = HTTP_OK; @@ -963,23 +963,23 @@ clientReplyContext::purgeDoPurgeHead(StoreEntry *newEntry) if (http->request->vary_headers && !strstr(http->request->vary_headers, "=")) { - StoreEntry *entry = storeGetPublic(urlCanonical(http->request), METHOD_GET); + StoreEntry *entry = storeGetPublic(urlCanonical(http->request), Http::METHOD_GET); if (entry) { debugs(88, 4, "clientPurgeRequest: Vary GET '" << entry->url() << "'" ); #if USE_HTCP - neighborsHtcpClear(entry, NULL, http->request, HttpRequestMethod(METHOD_GET), HTCP_CLR_PURGE); + neighborsHtcpClear(entry, NULL, http->request, HttpRequestMethod(Http::METHOD_GET), HTCP_CLR_PURGE); #endif entry->release(); purgeStatus = HTTP_OK; } - entry = storeGetPublic(urlCanonical(http->request), METHOD_HEAD); + entry = storeGetPublic(urlCanonical(http->request), Http::METHOD_HEAD); if (entry) { debugs(88, 4, "clientPurgeRequest: Vary HEAD '" << entry->url() << "'" ); #if USE_HTCP - neighborsHtcpClear(entry, NULL, http->request, HttpRequestMethod(METHOD_HEAD), HTCP_CLR_PURGE); + neighborsHtcpClear(entry, NULL, http->request, HttpRequestMethod(Http::METHOD_HEAD), HTCP_CLR_PURGE); #endif entry->release(); purgeStatus = HTTP_OK; @@ -1697,14 +1697,14 @@ clientGetMoreData(clientStreamNode * aNode, ClientHttpRequest * http) return; } - if (context->http->request->method == METHOD_PURGE) { + if (context->http->request->method == Http::METHOD_PURGE) { context->purgeRequest(); return; } // OPTIONS with Max-Forwards:0 handled in clientProcessRequest() - if (context->http->request->method == METHOD_TRACE) { + if (context->http->request->method == Http::METHOD_TRACE) { if (context->http->request->header.getInt64(HDR_MAX_FORWARDS) == 0) { context->traceReply(aNode); return; @@ -1908,8 +1908,8 @@ clientReplyContext::sendNotModified() void clientReplyContext::sendNotModifiedOrPreconditionFailedError() { - if (http->request->method == METHOD_GET || - http->request->method == METHOD_HEAD) + if (http->request->method == Http::METHOD_GET || + http->request->method == Http::METHOD_HEAD) sendNotModified(); else sendPreconditionFailedError(); @@ -2015,7 +2015,7 @@ clientReplyContext::processReplyAccessResult(const allow_t &accessAllowed) #endif - if (http->request->method == METHOD_HEAD) { + if (http->request->method == Http::METHOD_HEAD) { /* do not forward body for HEAD replies */ body_size = 0; http->flags.done_copying = 1; diff --git a/src/client_side_request.cc b/src/client_side_request.cc index 39e28bd19b..4070550e8b 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -581,7 +581,7 @@ ClientRequestContext::hostHeaderVerifyFailed(const char *A, const char *B) { // IP address validation for Host: failed. Admin wants to ignore them. // NP: we do not yet handle CONNECT tunnels well, so ignore for them - if (!Config.onoff.hostStrictVerify && http->request->method != METHOD_CONNECT) { + if (!Config.onoff.hostStrictVerify && http->request->method != Http::METHOD_CONNECT) { debugs(85, 3, "SECURITY ALERT: Host header forgery detected on " << http->getConn()->clientConnection << " (" << A << " does not match " << B << ") on URL: " << urlCanonical(http->request)); @@ -691,7 +691,7 @@ ClientRequestContext::hostHeaderVerify() // Verify forward-proxy requested URL domain matches the Host: header debugs(85, 3, HERE << "FAIL on validate URL port " << http->request->port << " matches Host: port " << portStr); hostHeaderVerifyFailed("URL port", portStr); - } else if (!portStr && http->request->method != METHOD_CONNECT && http->request->port != urlDefaultPort(http->request->protocol)) { + } else if (!portStr && http->request->method != Http::METHOD_CONNECT && http->request->port != urlDefaultPort(http->request->protocol)) { // Verify forward-proxy requested URL domain matches the Host: header // Special case: we don't have a default-port to check for CONNECT. Assume URL is correct. debugs(85, 3, HERE << "FAIL on validate URL port " << http->request->port << " matches Host: default port " << urlDefaultPort(http->request->protocol)); @@ -942,10 +942,10 @@ clientHierarchical(ClientHttpRequest * http) if (request->flags.auth) return 0; - if (method == METHOD_TRACE) + if (method == Http::METHOD_TRACE) return 1; - if (method != METHOD_GET) + if (method != Http::METHOD_GET) return 0; /* scan hierarchy_stoplist */ @@ -957,7 +957,7 @@ clientHierarchical(ClientHttpRequest * http) return 0; if (request->protocol == AnyP::PROTO_HTTP) - return httpCachable(method); + return method.respMaybeCacheable(); if (request->protocol == AnyP::PROTO_GOPHER) return gopherCachable(request); @@ -1077,7 +1077,7 @@ clientInterpretRequestHeaders(ClientHttpRequest * http) } } - if (request->method == METHOD_OTHER) { + if (request->method == Http::METHOD_OTHER) { no_cache=true; } @@ -1095,7 +1095,7 @@ clientInterpretRequestHeaders(ClientHttpRequest * http) } /* ignore range header in non-GETs or non-HEADs */ - if (request->method == METHOD_GET || request->method == METHOD_HEAD) { + if (request->method == Http::METHOD_GET || request->method == Http::METHOD_HEAD) { // XXX: initialize if we got here without HttpRequest::parseHeader() if (!request->range) request->range = req_hdr->getRange(); @@ -1166,7 +1166,7 @@ clientInterpretRequestHeaders(ClientHttpRequest * http) #endif - request->flags.cachable = http->request->cacheable(); + request->flags.cachable = http->request->maybeCacheable(); if (clientHierarchical(http)) request->flags.hierarchical = 1; @@ -1308,7 +1308,7 @@ ClientRequestContext::sslBumpAccessCheck() // Bumping here can only start with a CONNECT request on a bumping port // (bumping of intercepted SSL conns is decided before we get 1st request). // We also do not bump redirected CONNECT requests. - if (http->request->method != METHOD_CONNECT || http->redirect.status || + if (http->request->method != Http::METHOD_CONNECT || http->redirect.status || !Config.accessList.ssl_bump || !http->getConn()->port->sslBump) { http->al->ssl.bumpMode = Ssl::bumpEnd; // SslBump does not apply; log - debugs(85, 5, HERE << "cannot SslBump this request"); @@ -1369,7 +1369,7 @@ ClientHttpRequest::processRequest() { debugs(85, 4, "clientProcessRequest: " << RequestMethodStr(request->method) << " '" << uri << "'"); - if (request->method == METHOD_CONNECT && !redirect.status) { + if (request->method == Http::METHOD_CONNECT && !redirect.status) { #if USE_SSL if (sslBumpNeeded()) { sslBumpStart(); diff --git a/src/errorpage.cc b/src/errorpage.cc index aeb3e299a7..58837c358e 100644 --- a/src/errorpage.cc +++ b/src/errorpage.cc @@ -1167,7 +1167,7 @@ ErrorState::BuildHttpReply() status = httpStatus; else { // Use 307 for HTTP/1.1 non-GET/HEAD requests. - if (request->method != METHOD_GET && request->method != METHOD_HEAD && request->http_ver >= HttpVersion(1,1)) + if (request->method != Http::METHOD_GET && request->method != Http::METHOD_HEAD && request->http_ver >= HttpVersion(1,1)) status = HTTP_TEMPORARY_REDIRECT; } diff --git a/src/esi/Include.cc b/src/esi/Include.cc index 120d4ca30b..1feaad0957 100644 --- a/src/esi/Include.cc +++ b/src/esi/Include.cc @@ -334,10 +334,9 @@ ESIInclude::Start (ESIStreamContext::Pointer stream, char const *url, ESIVarStat /* tempUrl is eaten by the request */ char const *tempUrl = vars->extractChar (); - debugs(86, 5, "ESIIncludeStart: Starting subrequest with url '" << tempUrl << - "'"); + debugs(86, 5, "ESIIncludeStart: Starting subrequest with url '" << tempUrl << "'"); - if (clientBeginRequest(METHOD_GET, tempUrl, esiBufferRecipient, esiBufferDetach, stream.getRaw(), &tempheaders, stream->localbuffer->buf, HTTP_REQBUF_SZ)) { + if (clientBeginRequest(Http::METHOD_GET, tempUrl, esiBufferRecipient, esiBufferDetach, stream.getRaw(), &tempheaders, stream->localbuffer->buf, HTTP_REQBUF_SZ)) { debugs(86, DBG_CRITICAL, "starting new ESI subrequest failed"); } diff --git a/src/forward.cc b/src/forward.cc index 4fe87ecee5..bb056dd833 100644 --- a/src/forward.cc +++ b/src/forward.cc @@ -563,29 +563,14 @@ FwdState::checkRetry() bool FwdState::checkRetriable() { - /* RFC2616 9.1 Safe and Idempotent Methods */ - switch (request->method.id()) { - /* 9.1.1 Safe Methods */ - - case METHOD_GET: - - case METHOD_HEAD: - /* 9.1.2 Idempotent Methods */ - - case METHOD_PUT: - - case METHOD_DELETE: - - case METHOD_OPTIONS: - - case METHOD_TRACE: - break; - - default: + // Optimize: A compliant proxy may retry PUTs, but Squid lacks the [rather + // complicated] code required to protect the PUT request body from being + // nibbled during the first try. Thus, Squid cannot retry some PUTs today. + if (request->body_pipe != NULL) return false; - } - return true; + // RFC2616 9.1 Safe and Idempotent Methods + return (request->method.isHttpSafe() || request->method.isIdempotent()); } void diff --git a/src/ftp.cc b/src/ftp.cc index 878cb7e4d9..611ca7e561 100644 --- a/src/ftp.cc +++ b/src/ftp.cc @@ -507,7 +507,7 @@ FtpStateData::FtpStateData(FwdState *theFwdState, const Comm::ConnectionPointer AsyncCall::Pointer closer = JobCallback(9, 5, Dialer, this, FtpStateData::ctrlClosed); ctrl.opened(conn, closer); - if (request->method == METHOD_PUT) + if (request->method == Http::METHOD_PUT) flags.put = 1; } @@ -1336,7 +1336,7 @@ FtpStateData::processReplyBody() { debugs(9, 3, HERE << "FtpStateData::processReplyBody starting."); - if (request->method == METHOD_HEAD && (flags.isdir || theSize != -1)) { + if (request->method == Http::METHOD_HEAD && (flags.isdir || theSize != -1)) { serverComplete(); return; } @@ -2537,7 +2537,7 @@ ftpSendPassive(FtpStateData * ftpState) /** \par * Checks for 'HEAD' method request and passes off for special handling by FtpStateData::processHeadResponse(). */ - if (ftpState->request->method == METHOD_HEAD && (ftpState->flags.isdir || ftpState->theSize != -1)) { + if (ftpState->request->method == Http::METHOD_HEAD && (ftpState->flags.isdir || ftpState->theSize != -1)) { ftpState->processHeadResponse(); // may call serverComplete return; } diff --git a/src/htcp.cc b/src/htcp.cc index 26c4ca6831..a671bb5f2a 100644 --- a/src/htcp.cc +++ b/src/htcp.cc @@ -748,7 +748,7 @@ htcpUnpackSpecifier(char *buf, int sz) */ method = HttpRequestMethod(s->method, NULL); - s->request = HttpRequest::CreateFromUrlAndMethod(s->uri, method == METHOD_NONE ? HttpRequestMethod(METHOD_GET) : method); + s->request = HttpRequest::CreateFromUrlAndMethod(s->uri, method == Http::METHOD_NONE ? HttpRequestMethod(Http::METHOD_GET) : method); if (s->request) HTTPMSGLOCK(s->request); diff --git a/src/http.cc b/src/http.cc index e50a962a48..d0311509cd 100644 --- a/src/http.cc +++ b/src/http.cc @@ -176,19 +176,6 @@ HttpStateData::httpStateConnClosed(const CommCloseCbParams ¶ms) mustStop("HttpStateData::httpStateConnClosed"); } -int -httpCachable(const HttpRequestMethod& method) -{ - /* GET and HEAD are cachable. Others are not. */ - - // TODO: replase to HttpRequestMethod::isCachable() ? - if (method != METHOD_GET && method != METHOD_HEAD) - return 0; - - /* else cachable */ - return 1; -} - void HttpStateData::httpTimeout(const CommTimeoutCbParams ¶ms) { @@ -283,14 +270,14 @@ httpMaybeRemovePublic(StoreEntry * e, http_status status) * changed. */ if (e->mem_obj->request) - pe = storeGetPublicByRequestMethod(e->mem_obj->request, METHOD_HEAD); + pe = storeGetPublicByRequestMethod(e->mem_obj->request, Http::METHOD_HEAD); else - pe = storeGetPublic(e->mem_obj->url, METHOD_HEAD); + pe = storeGetPublic(e->mem_obj->url, Http::METHOD_HEAD); if (pe != NULL) { assert(e != pe); #if USE_HTCP - neighborsHtcpClear(e, NULL, e->mem_obj->request, HttpRequestMethod(METHOD_HEAD), HTCP_CLR_INVALIDATION); + neighborsHtcpClear(e, NULL, e->mem_obj->request, HttpRequestMethod(Http::METHOD_HEAD), HTCP_CLR_INVALIDATION); #endif pe->release(); } @@ -1952,7 +1939,7 @@ copyOneHeaderFromClientsideRequestToUpstreamRequest(const HttpHeaderEntry *e, co case HDR_MAX_FORWARDS: /** \par Max-Forwards: * pass only on TRACE or OPTIONS requests */ - if (request->method == METHOD_TRACE || request->method == METHOD_OPTIONS) { + if (request->method == Http::METHOD_TRACE || request->method == Http::METHOD_OPTIONS) { const int64_t hops = e->getInt64(); if (hops > 0) diff --git a/src/http/Makefile.am b/src/http/Makefile.am new file mode 100644 index 0000000000..bff9700cad --- /dev/null +++ b/src/http/Makefile.am @@ -0,0 +1,14 @@ +include $(top_srcdir)/src/Common.am +include $(top_srcdir)/src/TestHeaders.am + +noinst_LTLIBRARIES = libsquid-http.la + +libsquid_http_la_SOURCES = \ + MethodType.cc \ + MethodType.h + +MethodType.cc: MethodType.h $(top_srcdir)/src/mk-string-arrays.awk + ($(AWK) -f $(top_srcdir)/src/mk-string-arrays.awk < $(srcdir)/MethodType.h | sed -e 's%METHOD_%%' | \ + sed -e 's%_C%-C%' >$@) || ($(RM) -f $@ && exit 1) + +CLEANFILES += MethodType.cc diff --git a/src/http/MethodType.h b/src/http/MethodType.h new file mode 100644 index 0000000000..b66c9fad5e --- /dev/null +++ b/src/http/MethodType.h @@ -0,0 +1,94 @@ +#ifndef SQUID_SRC_HTTP_METHODTYPE_H +#define SQUID_SRC_HTTP_METHODTYPE_H + +namespace Http +{ + +// see IANA registry: +// also: https://datatracker.ietf.org/doc/draft-ietf-httpbis-method-registrations +typedef enum _method_t { + METHOD_NONE = 0, + +#if NO_SPECIAL_HANDLING + // RFC 2068 + METHOD_LINK, + METHOD_UNLINK, +#endif + + // RFC 2616 (HTTP) + METHOD_GET, + METHOD_POST, + METHOD_PUT, + METHOD_HEAD, + METHOD_CONNECT, + METHOD_TRACE, + METHOD_OPTIONS, + METHOD_DELETE, + + // RFC 3253 + METHOD_CHECKOUT, + METHOD_CHECKIN, + METHOD_UNCHECKOUT, + METHOD_MKWORKSPACE, + METHOD_VERSION_CONTROL, + METHOD_REPORT, + METHOD_UPDATE, + METHOD_LABEL, + METHOD_MERGE, + METHOD_BASELINE_CONTROL, + METHOD_MKACTIVITY, + +#if NO_SPECIAL_HANDLING + // RFC 3648 + METHOD_ORDERPATCH, + + // RFC 3744 + METHOD_ACL, + + // RFC 4437 + METHOD_MKREDIRECTREF, + METHOD_UPDATEREDIRECTREF, + + // RFC 4791 + METHOD_MKCALENDAR, +#endif + + // RFC 4918 (WebDAV) + METHOD_PROPFIND, + METHOD_PROPPATCH, + METHOD_MKCOL, + METHOD_COPY, + METHOD_MOVE, + METHOD_LOCK, + METHOD_UNLOCK, + + // RFC 5323 + METHOD_SEARCH, + +#if NO_SPECIAL_HANDLING + // RFC 5789 + METHOD_PATCH, + + // RFC 5842 + METHOD_BIND, + METHOD_REBIND, + METHOD_UNBIND, +#endif + + // Squid extension methods + METHOD_PURGE, + METHOD_OTHER, + METHOD_ENUM_END // MUST be last, (yuck) this is used as an array-initialization index constant! +} MethodType; + +extern const char *MethodType_str[]; + +inline const char* +MethodStr(const MethodType m) +{ + return MethodType_str[m]; +} + +}; // namespace Http + +#endif /* SQUID_SRC_HTTP_METHODTYPE_H */ diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc index 195c888101..4df0c0beaf 100644 --- a/src/icmp/net_db.cc +++ b/src/icmp/net_db.cc @@ -1335,7 +1335,7 @@ netdbExchangeStart(void *data) assert(NULL != ex->r); ex->r->http_ver = HttpVersion(1,1); ex->connstate = STATE_HEADER; - ex->e = storeCreateEntry(uri, uri, RequestFlags(), METHOD_GET); + ex->e = storeCreateEntry(uri, uri, RequestFlags(), Http::METHOD_GET); ex->buf_sz = NETDB_REQBUF_SZ; assert(NULL != ex->e); ex->sc = storeClientListAdd(ex->e, ex); diff --git a/src/icp_v2.cc b/src/icp_v2.cc index 6b253bcd5f..5de16354fe 100644 --- a/src/icp_v2.cc +++ b/src/icp_v2.cc @@ -509,7 +509,7 @@ doV2Query(int fd, Ip::Address &from, char *buf, icp_common_t header) state->src_rtt = src_rtt; - StoreEntry::getPublic (state, url, METHOD_GET); + StoreEntry::getPublic (state, url, Http::METHOD_GET); HTTPMSGUNLOCK(icp_request); } @@ -853,5 +853,5 @@ icpGetCacheKey(const char *url, int reqnum) if (neighbors_do_private_keys && reqnum) return queried_keys[reqnum & N_QUERIED_KEYS_MASK]; - return storeKeyPublic(url, METHOD_GET); + return storeKeyPublic(url, Http::METHOD_GET); } diff --git a/src/icp_v3.cc b/src/icp_v3.cc index c5cd745b47..5bfe8680a1 100644 --- a/src/icp_v3.cc +++ b/src/icp_v3.cc @@ -78,7 +78,7 @@ doV3Query(int fd, Ip::Address &from, char *buf, icp_common_t header) state->url = xstrdup (url); - StoreEntry::getPublic (state, url, METHOD_GET); + StoreEntry::getPublic (state, url, Http::METHOD_GET); } ICP3State::~ICP3State() diff --git a/src/mgr/ActionParams.cc b/src/mgr/ActionParams.cc index 8c2764d95a..75d48aed4b 100644 --- a/src/mgr/ActionParams.cc +++ b/src/mgr/ActionParams.cc @@ -8,7 +8,7 @@ #include "ipc/TypedMsgHdr.h" #include "mgr/ActionParams.h" -Mgr::ActionParams::ActionParams(): httpMethod(METHOD_NONE) +Mgr::ActionParams::ActionParams(): httpMethod(Http::METHOD_NONE) { } @@ -17,8 +17,10 @@ Mgr::ActionParams::ActionParams(const Ipc::TypedMsgHdr &msg) msg.getString(httpUri); const int m = msg.getInt(); - Must(METHOD_NONE <= m && m < METHOD_ENUM_END); - httpMethod = static_cast<_method_t>(m); + Must(Http::METHOD_NONE <= m && m < Http::METHOD_ENUM_END); + String method; + msg.getString(method); + httpMethod = HttpRequestMethod(method.termedBuf(), NULL); msg.getPod(httpFlags); msg.getString(httpOrigin); @@ -33,7 +35,8 @@ void Mgr::ActionParams::pack(Ipc::TypedMsgHdr &msg) const { msg.putString(httpUri); - msg.putInt(httpMethod); + String foo(httpMethod.image()); + msg.putString(foo); msg.putPod(httpFlags); msg.putString(httpOrigin); diff --git a/src/mgr/ActionParams.h b/src/mgr/ActionParams.h index 75ab674e51..d3fd4a1c6b 100644 --- a/src/mgr/ActionParams.h +++ b/src/mgr/ActionParams.h @@ -26,7 +26,7 @@ public: public: /* details of the client HTTP request that caused the action */ String httpUri; ///< HTTP request URI - _method_t httpMethod; ///< HTTP request method + HttpRequestMethod httpMethod; ///< HTTP request method RequestFlags httpFlags; ///< HTTP request flags String httpOrigin; ///< HTTP Origin: header (if any) diff --git a/src/mime.cc b/src/mime.cc index 2d14023fd2..6b3cb5a04d 100644 --- a/src/mime.cc +++ b/src/mime.cc @@ -417,7 +417,7 @@ MimeIcon::load() if (type == NULL) fatal("Unknown icon format while reading mime.conf\n"); - StoreEntry::getPublic(this, url, METHOD_GET); + StoreEntry::getPublic(this, url, Http::METHOD_GET); } void @@ -456,10 +456,7 @@ MimeIcon::created (StoreEntry *newEntry) } flags.cachable = 1; - StoreEntry *e = storeCreateEntry(url, - url, - flags, - METHOD_GET); + StoreEntry *e = storeCreateEntry(url,url,flags,Http::METHOD_GET); assert(e != NULL); EBIT_SET(e->flags, ENTRY_SPECIAL); e->setPublicKey(); diff --git a/src/neighbors.cc b/src/neighbors.cc index 73977af2f9..bbfe22ddb4 100644 --- a/src/neighbors.cc +++ b/src/neighbors.cc @@ -181,7 +181,7 @@ peerAllowedToUse(const CachePeer * p, HttpRequest * request) // CONNECT requests are proxy requests. Not to be forwarded to origin servers. // Unless the destination port matches, in which case we MAY perform a 'DIRECT' to this CachePeer. - if (p->options.originserver && request->method == METHOD_CONNECT && request->port != p->in_addr.GetPort()) + if (p->options.originserver && request->method == Http::METHOD_CONNECT && request->port != p->in_addr.GetPort()) return false; if (p->peer_domain == NULL && p->access == NULL) @@ -1378,7 +1378,7 @@ peerCountMcastPeersStart(void *data) snprintf(url, MAX_URL, "http://"); p->in_addr.ToURL(url+7, MAX_URL -8 ); strcat(url, "/"); - fake = storeCreateEntry(url, url, RequestFlags(), METHOD_GET); + fake = storeCreateEntry(url, url, RequestFlags(), Http::METHOD_GET); HttpRequest *req = HttpRequest::CreateFromUrl(url); psstate = new ps_state; psstate->request = HTTPMSGLOCK(req); diff --git a/src/store.cc b/src/store.cc index f61827151c..79ff65185f 100644 --- a/src/store.cc +++ b/src/store.cc @@ -642,9 +642,9 @@ storeGetPublicByRequest(HttpRequest * req) { StoreEntry *e = storeGetPublicByRequestMethod(req, req->method); - if (e == NULL && req->method == METHOD_HEAD) + if (e == NULL && req->method == Http::METHOD_HEAD) /* We can generate a HEAD reply from a cached GET object */ - e = storeGetPublicByRequestMethod(req, METHOD_GET); + e = storeGetPublicByRequestMethod(req, Http::METHOD_GET); return e; } @@ -688,7 +688,7 @@ StoreEntry::setPrivateKey() mem_obj->id = getKeyCounter(); newkey = storeKeyPrivate(mem_obj->url, mem_obj->method, mem_obj->id); } else { - newkey = storeKeyPrivate("JUNK", METHOD_NONE, getKeyCounter()); + newkey = storeKeyPrivate("JUNK", Http::METHOD_NONE, getKeyCounter()); } assert(hash_lookup(store_table, newkey) == NULL); @@ -982,7 +982,7 @@ StoreEntry::checkCachable() { #if CACHE_ALL_METHODS - if (mem_obj->method != METHOD_GET) { + if (mem_obj->method != Http::METHOD_GET) { debugs(20, 2, "StoreEntry::checkCachable: NO: non-GET method"); ++store_check_cachable_hist.no.non_get; } else @@ -1383,7 +1383,7 @@ StoreEntry::validLength() const return 1; } - if (mem_obj->method == METHOD_HEAD) { + if (mem_obj->method == Http::METHOD_HEAD) { debugs(20, 5, "storeEntryValidLength: HEAD request: " << getMD5Text()); return 1; } @@ -1965,7 +1965,7 @@ StoreEntry::hasIfNoneMatchEtag(const HttpRequest &request) const const String reqETags = request.header.getList(HDR_IF_NONE_MATCH); // weak comparison is allowed only for HEAD or full-body GET requests const bool allowWeakMatch = !request.flags.isRanged && - (request.method == METHOD_GET || request.method == METHOD_HEAD); + (request.method == Http::METHOD_GET || request.method == Http::METHOD_HEAD); return hasOneOfEtags(reqETags, allowWeakMatch); } diff --git a/src/store_digest.cc b/src/store_digest.cc index b1133c83ac..8f94ac0b26 100644 --- a/src/store_digest.cc +++ b/src/store_digest.cc @@ -393,7 +393,7 @@ storeDigestRewriteStart(void *datanotused) /* make new store entry */ url = internalLocalUri("/squid-internal-periodic/", StoreDigestFileName); flags.cachable = 1; - e = storeCreateEntry(url, url, flags, METHOD_GET); + e = storeCreateEntry(url, url, flags, Http::METHOD_GET); assert(e); sd_state.rewrite_lock = e; debugs(71, 3, "storeDigestRewrite: url: " << url << " key: " << e->getMD5Text()); diff --git a/src/tests/testHttpRequest.cc b/src/tests/testHttpRequest.cc index d7e32398f2..b71b9148dd 100644 --- a/src/tests/testHttpRequest.cc +++ b/src/tests/testHttpRequest.cc @@ -36,11 +36,11 @@ testHttpRequest::testCreateFromUrlAndMethod() /* vanilla url */ unsigned short expected_port; char * url = xstrdup("http://foo:90/bar"); - HttpRequest *aRequest = HttpRequest::CreateFromUrlAndMethod(url, METHOD_GET); + HttpRequest *aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET); expected_port = 90; HttpRequest *nullRequest = NULL; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); - CPPUNIT_ASSERT(aRequest->method == METHOD_GET); + CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/bar"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, aRequest->protocol); @@ -49,10 +49,10 @@ testHttpRequest::testCreateFromUrlAndMethod() /* vanilla url, different method */ url = xstrdup("http://foo/bar"); - aRequest = HttpRequest::CreateFromUrlAndMethod(url, METHOD_PUT); + aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_PUT); expected_port = 80; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); - CPPUNIT_ASSERT(aRequest->method == METHOD_PUT); + CPPUNIT_ASSERT(aRequest->method == Http::METHOD_PUT); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/bar"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, aRequest->protocol); @@ -60,16 +60,16 @@ testHttpRequest::testCreateFromUrlAndMethod() /* a connect url with non-CONNECT data */ url = xstrdup(":foo/bar"); - aRequest = HttpRequest::CreateFromUrlAndMethod(url, METHOD_CONNECT); + aRequest = HttpRequest::CreateFromUrlAndMethod(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, METHOD_CONNECT); + aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_CONNECT); expected_port = 45; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); - CPPUNIT_ASSERT(aRequest->method == METHOD_CONNECT); + CPPUNIT_ASSERT(aRequest->method == Http::METHOD_CONNECT); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String(""), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_NONE, aRequest->protocol); @@ -89,7 +89,7 @@ testHttpRequest::testCreateFromUrl() HttpRequest *aRequest = HttpRequest::CreateFromUrl(url); expected_port = 90; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); - CPPUNIT_ASSERT(aRequest->method == METHOD_GET); + CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/bar"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, aRequest->protocol); @@ -109,10 +109,10 @@ testHttpRequest::testIPv6HostColonBug() /* valid IPv6 address without port */ url = xstrdup("http://[2000:800::45]/foo"); - aRequest = HttpRequest::CreateFromUrlAndMethod(url, METHOD_GET); + aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET); expected_port = 80; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); - CPPUNIT_ASSERT(aRequest->method == METHOD_GET); + CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, aRequest->protocol); @@ -121,10 +121,10 @@ testHttpRequest::testIPv6HostColonBug() /* valid IPv6 address with port */ url = xstrdup("http://[2000:800::45]:90/foo"); - aRequest = HttpRequest::CreateFromUrlAndMethod(url, METHOD_GET); + aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET); expected_port = 90; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); - CPPUNIT_ASSERT(aRequest->method == METHOD_GET); + CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, aRequest->protocol); @@ -133,10 +133,10 @@ testHttpRequest::testIPv6HostColonBug() /* IPv6 address as invalid (bug trigger) */ url = xstrdup("http://2000:800::45/foo"); - aRequest = HttpRequest::CreateFromUrlAndMethod(url, METHOD_GET); + aRequest = HttpRequest::CreateFromUrlAndMethod(url, Http::METHOD_GET); expected_port = 80; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); - CPPUNIT_ASSERT(aRequest->method == METHOD_GET); + CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET); CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(AnyP::PROTO_HTTP, aRequest->protocol); diff --git a/src/tests/testHttpRequestMethod.cc b/src/tests/testHttpRequestMethod.cc index a771be0a1c..3855514c22 100644 --- a/src/tests/testHttpRequestMethod.cc +++ b/src/tests/testHttpRequestMethod.cc @@ -19,11 +19,11 @@ CPPUNIT_TEST_SUITE_REGISTRATION( testHttpRequestMethod ); void testHttpRequestMethod::testConstructCharStart() { - /* parse an empty string -> METHOD_NONE */ - CPPUNIT_ASSERT(HttpRequestMethod(NULL,NULL) == METHOD_NONE); + /* parse an empty string -> Http::METHOD_NONE */ + CPPUNIT_ASSERT(HttpRequestMethod(NULL,NULL) == Http::METHOD_NONE); /* parsing a literal should work */ - CPPUNIT_ASSERT(HttpRequestMethod("GET", NULL) == METHOD_GET); - CPPUNIT_ASSERT(HttpRequestMethod("QWERTY", NULL) == METHOD_OTHER); + CPPUNIT_ASSERT(HttpRequestMethod("GET", NULL) == Http::METHOD_GET); + CPPUNIT_ASSERT(HttpRequestMethod("QWERTY", NULL) == Http::METHOD_OTHER); } /* @@ -33,48 +33,48 @@ void testHttpRequestMethod::testConstructCharStartEnd() { char const * buffer; - /* parse an empty string -> METHOD_NONE */ - CPPUNIT_ASSERT(HttpRequestMethod(NULL, NULL) == METHOD_NONE); + /* parse an empty string -> Http::METHOD_NONE */ + CPPUNIT_ASSERT(HttpRequestMethod(NULL, NULL) == Http::METHOD_NONE); /* parsing a literal should work */ - CPPUNIT_ASSERT(HttpRequestMethod("GET", NULL) == METHOD_GET); + CPPUNIT_ASSERT(HttpRequestMethod("GET", NULL) == Http::METHOD_GET); /* parsing with an explicit end should work */ buffer = "POSTPLUS"; - CPPUNIT_ASSERT(HttpRequestMethod(buffer, buffer + 4) == METHOD_POST); + CPPUNIT_ASSERT(HttpRequestMethod(buffer, buffer + 4) == Http::METHOD_POST); } /* - * we should be able to assign a method_t to a HttpRequestMethod + * we should be able to assign a Http::MethodType to a HttpRequestMethod */ void testHttpRequestMethod::testAssignFrommethod_t() { HttpRequestMethod method; - method = METHOD_NONE; - CPPUNIT_ASSERT_EQUAL(HttpRequestMethod(METHOD_NONE), method); - method = METHOD_POST; - CPPUNIT_ASSERT_EQUAL(HttpRequestMethod(METHOD_POST), method); + method = Http::METHOD_NONE; + CPPUNIT_ASSERT_EQUAL(HttpRequestMethod(Http::METHOD_NONE), method); + method = Http::METHOD_POST; + CPPUNIT_ASSERT_EQUAL(HttpRequestMethod(Http::METHOD_POST), method); } /* - * a default constructed HttpRequestMethod is == METHOD_NONE + * a default constructed HttpRequestMethod is == Http::METHOD_NONE */ void testHttpRequestMethod::testDefaultConstructor() { HttpRequestMethod lhs; - HttpRequestMethod rhs(METHOD_NONE); + HttpRequestMethod rhs(Http::METHOD_NONE); CPPUNIT_ASSERT_EQUAL(lhs, rhs); } /* - * we should be able to construct a HttpRequestMethod from a method_t + * we should be able to construct a HttpRequestMethod from a Http::MethodType */ void testHttpRequestMethod::testConstructmethod_t() { - CPPUNIT_ASSERT_EQUAL(HttpRequestMethod(METHOD_NONE), HttpRequestMethod(METHOD_NONE)); - CPPUNIT_ASSERT_EQUAL(HttpRequestMethod(METHOD_POST), HttpRequestMethod(METHOD_POST)); - CPPUNIT_ASSERT(HttpRequestMethod(METHOD_NONE) != HttpRequestMethod(METHOD_POST)); + CPPUNIT_ASSERT_EQUAL(HttpRequestMethod(Http::METHOD_NONE), HttpRequestMethod(Http::METHOD_NONE)); + CPPUNIT_ASSERT_EQUAL(HttpRequestMethod(Http::METHOD_POST), HttpRequestMethod(Http::METHOD_POST)); + CPPUNIT_ASSERT(HttpRequestMethod(Http::METHOD_NONE) != HttpRequestMethod(Http::METHOD_POST)); } /* @@ -87,16 +87,16 @@ testHttpRequestMethod::testImage() } /* - * an HttpRequestMethod should be comparable to a method_t without false + * an HttpRequestMethod should be comparable to a Http::MethodType without false * matches */ void testHttpRequestMethod::testEqualmethod_t() { - CPPUNIT_ASSERT(HttpRequestMethod(METHOD_NONE) == METHOD_NONE); - CPPUNIT_ASSERT(not (HttpRequestMethod(METHOD_POST) == METHOD_GET)); - CPPUNIT_ASSERT(HttpRequestMethod(METHOD_GET) == METHOD_GET); - CPPUNIT_ASSERT(not (HttpRequestMethod(METHOD_TRACE) == METHOD_SEARCH)); + CPPUNIT_ASSERT(HttpRequestMethod(Http::METHOD_NONE) == Http::METHOD_NONE); + CPPUNIT_ASSERT(not (HttpRequestMethod(Http::METHOD_POST) == Http::METHOD_GET)); + CPPUNIT_ASSERT(HttpRequestMethod(Http::METHOD_GET) == Http::METHOD_GET); + CPPUNIT_ASSERT(not (HttpRequestMethod(Http::METHOD_TRACE) == Http::METHOD_SEARCH)); } /* @@ -105,10 +105,10 @@ testHttpRequestMethod::testEqualmethod_t() void testHttpRequestMethod::testNotEqualmethod_t() { - CPPUNIT_ASSERT(HttpRequestMethod(METHOD_NONE) != METHOD_GET); - CPPUNIT_ASSERT(not (HttpRequestMethod(METHOD_POST) != METHOD_POST)); - CPPUNIT_ASSERT(HttpRequestMethod(METHOD_GET) != METHOD_NONE); - CPPUNIT_ASSERT(not (HttpRequestMethod(METHOD_SEARCH) != METHOD_SEARCH)); + CPPUNIT_ASSERT(HttpRequestMethod(Http::METHOD_NONE) != Http::METHOD_GET); + CPPUNIT_ASSERT(not (HttpRequestMethod(Http::METHOD_POST) != Http::METHOD_POST)); + CPPUNIT_ASSERT(HttpRequestMethod(Http::METHOD_GET) != Http::METHOD_NONE); + CPPUNIT_ASSERT(not (HttpRequestMethod(Http::METHOD_SEARCH) != Http::METHOD_SEARCH)); } /* diff --git a/src/tests/testRock.cc b/src/tests/testRock.cc index c1d3ae5969..f935ae0d93 100644 --- a/src/tests/testRock.cc +++ b/src/tests/testRock.cc @@ -177,7 +177,7 @@ testRock::createEntry(const int i) snprintf(url, sizeof(url), "dummy url %i", i); url[sizeof(url) - 1] = '\0'; StoreEntry *const pe = - storeCreateEntry(url, "dummy log url", flags, METHOD_GET); + storeCreateEntry(url, "dummy log url", flags, Http::METHOD_GET); HttpReply *const rep = const_cast(pe->getReply()); rep->setHeaders(HTTP_OK, "dummy test object", "x-squid-internal/test", -1, -1, squid_curtime + 100000); diff --git a/src/tests/testUfs.cc b/src/tests/testUfs.cc index 1213414642..7c929d6782 100644 --- a/src/tests/testUfs.cc +++ b/src/tests/testUfs.cc @@ -143,7 +143,7 @@ testUfs::testUfsSearch() /* Create "vary" base object */ RequestFlags flags; flags.cachable = 1; - StoreEntry *pe = storeCreateEntry("dummy url", "dummy log url", flags, METHOD_GET); + StoreEntry *pe = storeCreateEntry("dummy url", "dummy log url", flags, Http::METHOD_GET); HttpReply *rep = (HttpReply *) pe->getReply(); // bypass const rep->setHeaders(HTTP_OK, "dummy test object", "x-squid-internal/test", -1, -1, squid_curtime + 100000); diff --git a/src/url.cc b/src/url.cc index 760a887ddb..08eeaf0f08 100644 --- a/src/url.cc +++ b/src/url.cc @@ -201,7 +201,7 @@ urlDefaultPort(AnyP::ProtocolType p) * This abuses HttpRequest as a way of representing the parsed url * and its components. * method is used to switch parsers and to init the HttpRequest. - * If method is METHOD_CONNECT, then rather than a URL a hostname:port is + * If method is Http::METHOD_CONNECT, then rather than a URL a hostname:port is * looked for. * The url is non const so that if its too long we can NULL-terminate it in place. */ @@ -236,14 +236,14 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request) debugs(23, DBG_IMPORTANT, "urlParse: URL too large (" << l << " bytes)"); return NULL; } - if (method == METHOD_CONNECT) { + if (method == Http::METHOD_CONNECT) { port = CONNECT_PORT; if (sscanf(url, "[%[^]]]:%d", host, &port) < 1) if (sscanf(url, "%[^:]:%d", host, &port) < 1) return NULL; - } else if ((method == METHOD_OPTIONS || method == METHOD_TRACE) && + } else if ((method == Http::METHOD_OPTIONS || method == Http::METHOD_TRACE) && strcmp(url, "*") == 0) { protocol = AnyP::PROTO_HTTP; port = urlDefaultPort(protocol); @@ -497,7 +497,6 @@ const char * urlCanonical(HttpRequest * request) { LOCAL_ARRAY(char, portbuf, 32); -/// \todo AYJ: Performance: making this a ptr and allocating when needed will be better than a write and future xstrdup(). LOCAL_ARRAY(char, urlbuf, MAX_URL); if (request->canonical) @@ -506,31 +505,22 @@ urlCanonical(HttpRequest * request) if (request->protocol == AnyP::PROTO_URN) { snprintf(urlbuf, MAX_URL, "urn:" SQUIDSTRINGPH, SQUIDSTRINGPRINT(request->urlpath)); + } else if (request->method.id() == Http::METHOD_CONNECT) { + snprintf(urlbuf, MAX_URL, "%s:%d", request->GetHost(), request->port); } else { -/// \todo AYJ: this could use "if..else and method == METHOD_CONNECT" easier. - switch (request->method.id()) { - - case METHOD_CONNECT: - snprintf(urlbuf, MAX_URL, "%s:%d", request->GetHost(), request->port); - break; - - default: - portbuf[0] = '\0'; - - if (request->port != urlDefaultPort(request->protocol)) - snprintf(portbuf, 32, ":%d", request->port); - - const URLScheme sch = request->protocol; // temporary, until bug 1961 URL handling is fixed. - snprintf(urlbuf, MAX_URL, "%s://%s%s%s%s" SQUIDSTRINGPH, - sch.const_str(), - request->login, - *request->login ? "@" : null_string, - request->GetHost(), - portbuf, - SQUIDSTRINGPRINT(request->urlpath)); - - break; - } + portbuf[0] = '\0'; + + if (request->port != urlDefaultPort(request->protocol)) + snprintf(portbuf, 32, ":%d", request->port); + + const URLScheme sch = request->protocol; // temporary, until bug 1961 URL handling is fixed. + snprintf(urlbuf, MAX_URL, "%s://%s%s%s%s" SQUIDSTRINGPH, + sch.const_str(), + request->login, + *request->login ? "@" : null_string, + request->GetHost(), + portbuf, + SQUIDSTRINGPRINT(request->urlpath)); } return (request->canonical = xstrdup(urlbuf)); @@ -551,52 +541,39 @@ urlCanonicalClean(const HttpRequest * request) if (request->protocol == AnyP::PROTO_URN) { snprintf(buf, MAX_URL, "urn:" SQUIDSTRINGPH, SQUIDSTRINGPRINT(request->urlpath)); + } else if (request->method.id() == Http::METHOD_CONNECT) { + snprintf(buf, MAX_URL, "%s:%d", request->GetHost(), request->port); } else { -/// \todo AYJ: this could use "if..else and method == METHOD_CONNECT" easier. - switch (request->method.id()) { + portbuf[0] = '\0'; - case METHOD_CONNECT: - snprintf(buf, MAX_URL, "%s:%d", - request->GetHost(), - request->port); - break; - - default: - portbuf[0] = '\0'; + if (request->port != urlDefaultPort(request->protocol)) + snprintf(portbuf, 32, ":%d", request->port); - if (request->port != urlDefaultPort(request->protocol)) - snprintf(portbuf, 32, ":%d", request->port); + loginbuf[0] = '\0'; - loginbuf[0] = '\0'; + if ((int) strlen(request->login) > 0) { + strcpy(loginbuf, request->login); - if ((int) strlen(request->login) > 0) { - strcpy(loginbuf, request->login); + if ((t = strchr(loginbuf, ':'))) + *t = '\0'; - if ((t = strchr(loginbuf, ':'))) - *t = '\0'; - - strcat(loginbuf, "@"); - } - - const URLScheme sch = request->protocol; // temporary, until bug 1961 URL handling is fixed. - snprintf(buf, MAX_URL, "%s://%s%s%s" SQUIDSTRINGPH, - sch.const_str(), - loginbuf, - request->GetHost(), - portbuf, - SQUIDSTRINGPRINT(request->urlpath)); - /* - * strip arguments AFTER a question-mark - */ + strcat(loginbuf, "@"); + } - if (Config.onoff.strip_query_terms) - if ((t = strchr(buf, '?'))) { - ++t; - *t = '\0'; - } + const URLScheme sch = request->protocol; // temporary, until bug 1961 URL handling is fixed. + snprintf(buf, MAX_URL, "%s://%s%s%s" SQUIDSTRINGPH, + sch.const_str(), + loginbuf, + request->GetHost(), + portbuf, + SQUIDSTRINGPRINT(request->urlpath)); + /* + * strip arguments AFTER a question-mark + */ - break; - } + if (Config.onoff.strip_query_terms) + if ((t = strchr(buf, '?'))) + *(++t) = '\0'; } if (stringHasCntl(buf)) @@ -607,7 +584,7 @@ urlCanonicalClean(const HttpRequest * request) /** * Yet another alternative to urlCanonical. - * This one addes the https:// parts to METHOD_CONNECT URL + * This one adds the https:// parts to Http::METHOD_CONNECT URL * for use in error page outputs. * Luckily we can leverage the others instead of duplicating. */ @@ -617,7 +594,7 @@ urlCanonicalFakeHttps(const HttpRequest * request) LOCAL_ARRAY(char, buf, MAX_URL); // method CONNECT and port HTTPS - if (request->method == METHOD_CONNECT && request->port == 443) { + if (request->method == Http::METHOD_CONNECT && request->port == 443) { snprintf(buf, MAX_URL, "https://%s/*", request->GetHost()); return buf; } @@ -669,7 +646,7 @@ char * urlMakeAbsolute(const HttpRequest * req, const char *relUrl) { - if (req->method.id() == METHOD_CONNECT) { + if (req->method.id() == Http::METHOD_CONNECT) { return (NULL); } @@ -840,15 +817,15 @@ urlCheckRequest(const HttpRequest * r) * do not have a default protocol from the client side of HTTP. */ - if (r->method == METHOD_CONNECT) + if (r->method == Http::METHOD_CONNECT) return 1; // we support OPTIONS and TRACE directed at us (with a 501 reply, for now) // we also support forwarding OPTIONS and TRACE, except for the *-URI ones - if (r->method == METHOD_OPTIONS || r->method == METHOD_TRACE) + if (r->method == Http::METHOD_OPTIONS || r->method == Http::METHOD_TRACE) return (r->header.getInt64(HDR_MAX_FORWARDS) == 0 || r->urlpath != "*"); - if (r->method == METHOD_PURGE) + if (r->method == Http::METHOD_PURGE) return 1; /* does method match the protocol? */ @@ -864,7 +841,7 @@ urlCheckRequest(const HttpRequest * r) case AnyP::PROTO_FTP: - if (r->method == METHOD_PUT) + if (r->method == Http::METHOD_PUT) rc = 1; case AnyP::PROTO_GOPHER: @@ -872,9 +849,9 @@ urlCheckRequest(const HttpRequest * r) case AnyP::PROTO_WAIS: case AnyP::PROTO_WHOIS: - if (r->method == METHOD_GET) + if (r->method == Http::METHOD_GET) rc = 1; - else if (r->method == METHOD_HEAD) + else if (r->method == Http::METHOD_HEAD) rc = 1; break; diff --git a/src/urn.cc b/src/urn.cc index 0fbdd13709..e9967443b6 100644 --- a/src/urn.cc +++ b/src/urn.cc @@ -247,7 +247,7 @@ UrnState::start(HttpRequest * r, StoreEntry * e) if (urlres_r == NULL) return; - StoreEntry::getPublic (this, urlres, METHOD_GET); + StoreEntry::getPublic (this, urlres, Http::METHOD_GET); } void @@ -256,7 +256,7 @@ UrnState::created(StoreEntry *newEntry) urlres_e = newEntry; if (urlres_e->isNull()) { - urlres_e = storeCreateEntry(urlres, urlres, RequestFlags(), METHOD_GET); + urlres_e = storeCreateEntry(urlres, urlres, RequestFlags(), Http::METHOD_GET); sc = storeClientListAdd(urlres_e, this); FwdState::fwdStart(Comm::ConnectionPointer(), urlres_e, urlres_r); } else { -- 2.39.2