From 79c8035eb53d5f59009787c37ef5ccfa2b97a70c Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Wed, 27 Oct 2010 17:29:55 -0600 Subject: [PATCH] HTTP Compliance: Support If-Match and If-None-Match requests. Add support for If-Match and If-None-Match headers as described in RFC 2616 (sections 14.24 and 14.26 in particular). Moved IMS handling from clientReplyContext::cacheHit() to clientReplyContext::processConditional() while preserving the original IMS logic, except for the case when a request has both IMS and If-None-Match. Co-Advisors test cases: test_clause/rfc2616/ifMatch-mismatch-strong test_clause/rfc2616/ifMatch-mismatch-weak test_clause/rfc2616/ifNoneMatch-match-imsNone and many more --- errors/Makefile.am | 1 + errors/list | 1 + src/HttpRequest.cc | 8 +++ src/HttpRequest.h | 2 + src/Store.h | 5 ++ src/client_side_reply.cc | 132 ++++++++++++++++++++++++++++----------- src/client_side_reply.h | 2 + src/err_type.h | 1 + src/store.cc | 45 +++++++++++++ 9 files changed, 162 insertions(+), 35 deletions(-) diff --git a/errors/Makefile.am b/errors/Makefile.am index afe76cace6..f45bab175a 100644 --- a/errors/Makefile.am +++ b/errors/Makefile.am @@ -36,6 +36,7 @@ ERROR_TEMPLATES = \ templates/ERR_LIFETIME_EXP \ templates/ERR_NO_RELAY \ templates/ERR_ONLY_IF_CACHED_MISS \ + templates/ERR_PRECONDITION_FAILED \ templates/ERR_READ_ERROR \ templates/ERR_READ_TIMEOUT \ templates/ERR_SECURE_CONNECT_FAIL \ diff --git a/errors/list b/errors/list index 01d0e8dac7..67388b4590 100644 --- a/errors/list +++ b/errors/list @@ -22,6 +22,7 @@ ERR_INVALID_URL ERR_LIFETIME_EXP ERR_NO_RELAY ERR_ONLY_IF_CACHED_MISS +ERR_PRECONDITION_FAILED ERR_READ_ERROR ERR_READ_TIMEOUT ERR_SECURE_CONNECT_FAIL diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index eb7276ac61..acac10511c 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -594,6 +594,14 @@ HttpRequest::cacheable() const return true; } +bool +HttpRequest::conditional() const +{ + return flags.ims || + header.has(HDR_IF_MATCH) || + header.has(HDR_IF_NONE_MATCH); +} + bool HttpRequest::inheritProperties(const HttpMsg *aMsg) { const HttpRequest* aReq = dynamic_cast(aMsg); diff --git a/src/HttpRequest.h b/src/HttpRequest.h index e6ad3d56ed..4c72c6d266 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -91,6 +91,8 @@ public: /* are responses to this request potentially cachable */ bool cacheable() const; + bool conditional() const; ///< has at least one recognized If-* header + /// whether the client is likely to be able to handle a 1xx reply bool canHandle1xx() const; diff --git a/src/Store.h b/src/Store.h index 0941059169..f08fa514a8 100644 --- a/src/Store.h +++ b/src/Store.h @@ -123,6 +123,10 @@ public: void setNoDelay (bool const); bool modifiedSince(HttpRequest * request) const; + /// has ETag matching at least one of the If-Match etags + bool hasIfMatchEtag(const HttpRequest &request) const; + /// has ETag matching at least one of the If-None-Match etags + bool hasIfNoneMatchEtag(const HttpRequest &request) const; /** What store does this entry belong too ? */ virtual RefCount store() const; @@ -189,6 +193,7 @@ private: static MemAllocator *pool; bool validLength() const; + bool hasOneOfEtags(const String &reqETags, const bool allowWeakMatch) const; }; /// \ingroup StoreAPI diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc index 14d5860f5d..908ca295e6 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -557,41 +557,9 @@ clientReplyContext::cacheHit(StoreIOBuffer result) http->logType = LOG_TCP_MISS; processMiss(); } - } else if (r->flags.ims) { - /* - * Handle If-Modified-Since requests from the client - */ - - if (e->getReply()->sline.status != HTTP_OK) { - debugs(88, 4, "clientCacheHit: Reply code " << - e->getReply()->sline.status << " != 200"); - http->logType = LOG_TCP_MISS; - processMiss(); - } else if (e->modifiedSince(http->request)) { - http->logType = LOG_TCP_IMS_HIT; - sendMoreData(result); - } else { - time_t const timestamp = e->timestamp; - HttpReply *temprep = e->getReply()->make304(); - http->logType = LOG_TCP_IMS_HIT; - removeClientStoreReference(&sc, http); - createStoreEntry(http->request->method, - request_flags()); - e = http->storeEntry(); - /* - * Copy timestamp from the original entry so the 304 - * reply has a meaningful Age: header. - */ - e->timestamp = timestamp; - e->replaceHttpReply(temprep); - e->complete(); - /* TODO: why put this in the store and then serialise it and then parse it again. - * Simply mark the request complete in our context and - * write the reply struct to the client side - */ - triggerInitialStoreRead(); - } - } else { + } else if (r->conditional()) + processConditional(result); + else { /* * plain ol' cache hit */ @@ -711,6 +679,87 @@ clientReplyContext::processOnlyIfCachedMiss() startError(err); } +/// process conditional request from client +void +clientReplyContext::processConditional(StoreIOBuffer &result) +{ + StoreEntry *e = http->storeEntry(); + + if (e->getReply()->sline.status != HTTP_OK) { + debugs(88, 4, "clientReplyContext::processConditional: Reply code " << + e->getReply()->sline.status << " != 200"); + http->logType = LOG_TCP_MISS; + processMiss(); + return; + } + + HttpRequest &r = *http->request; + + if (r.header.has(HDR_IF_MATCH) && !e->hasIfMatchEtag(r)) { + // RFC 2616: reply with 412 Precondition Failed if If-Match did not match + sendPreconditionFailedError(); + return; + } + + bool matchedIfNoneMatch = false; + if (r.header.has(HDR_IF_NONE_MATCH)) { + if (!e->hasIfNoneMatchEtag(r)) { + // RFC 2616: ignore IMS if If-None-Match did not match + r.flags.ims = 0; + r.ims = -1; + r.imslen = 0; + r.header.delById(HDR_IF_MODIFIED_SINCE); + http->logType = LOG_TCP_MISS; + sendMoreData(result); + return; + } + + if (!r.flags.ims) { + // RFC 2616: if If-None-Match matched and there is no IMS, + // reply with 412 Precondition Failed + sendPreconditionFailedError(); + return; + } + + // otherwise check IMS below to decide if we reply with 412 + matchedIfNoneMatch = true; + } + + if (r.flags.ims) { + // handle If-Modified-Since requests from the client + if (e->modifiedSince(&r)) { + http->logType = LOG_TCP_IMS_HIT; + sendMoreData(result); + return; + } + + if (matchedIfNoneMatch) { + // If-None-Match matched, reply with 412 Precondition Failed + sendPreconditionFailedError(); + return; + } + + // otherwise reply with 304 Not Modified + const time_t timestamp = e->timestamp; + HttpReply *const temprep = e->getReply()->make304(); + http->logType = LOG_TCP_IMS_HIT; + removeClientStoreReference(&sc, http); + createStoreEntry(http->request->method, request_flags()); + e = http->storeEntry(); + // Copy timestamp from the original entry so the 304 + // reply has a meaningful Age: header. + e->timestamp = timestamp; + e->replaceHttpReply(temprep); + e->complete(); + /* + * TODO: why put this in the store and then serialise it and + * then parse it again. Simply mark the request complete in + * our context and write the reply struct to the client side. + */ + triggerInitialStoreRead(); + } +} + void clientReplyContext::purgeRequestFindObjectToPurge() { @@ -1804,6 +1853,19 @@ clientReplyContext::sendBodyTooLargeError() } +/// send 412 (Precondition Failed) to client +void +clientReplyContext::sendPreconditionFailedError() +{ + http->logType = LOG_TCP_HIT; + ErrorState *const err = + clientBuildError(ERR_PRECONDITION_FAILED, HTTP_PRECONDITION_FAILED, + NULL, http->getConn()->peer, http->request); + removeClientStoreReference(&sc, http); + HTTPMSGUNLOCK(reply); + startError(err); +} + void clientReplyContext::processReplyAccess () { diff --git a/src/client_side_reply.h b/src/client_side_reply.h index e141b84c3f..c1787f736d 100644 --- a/src/client_side_reply.h +++ b/src/client_side_reply.h @@ -128,6 +128,7 @@ private: bool alwaysAllowResponse(http_status sline) const; int checkTransferDone(); void processOnlyIfCachedMiss(); + void processConditional(StoreIOBuffer &result); void cacheHit(StoreIOBuffer result); void handleIMSReply(StoreIOBuffer result); void sendMoreData(StoreIOBuffer result); @@ -136,6 +137,7 @@ private: void purgeAllCached(); void sendBodyTooLargeError(); + void sendPreconditionFailedError(); StoreEntry *old_entry; store_client *old_sc; /* ... for entry to be validated */ diff --git a/src/err_type.h b/src/err_type.h index 88e2ab0e10..fc131b3ce0 100644 --- a/src/err_type.h +++ b/src/err_type.h @@ -34,6 +34,7 @@ typedef enum { ERR_UNSUP_REQ, ERR_INVALID_URL, ERR_ZERO_SIZE_OBJECT, + ERR_PRECONDITION_FAILED, /* FTP Errors */ ERR_FTP_DISABLED, diff --git a/src/store.cc b/src/store.cc index 7d53bb7155..ba9955da29 100644 --- a/src/store.cc +++ b/src/store.cc @@ -1904,6 +1904,51 @@ StoreEntry::modifiedSince(HttpRequest * request) const } } +bool +StoreEntry::hasIfMatchEtag(const HttpRequest &request) const +{ + const String reqETags = request.header.getList(HDR_IF_MATCH); + return hasOneOfEtags(reqETags, false); +} + +bool +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.range && + (request.method == METHOD_GET || request.method == METHOD_HEAD); + return hasOneOfEtags(reqETags, allowWeakMatch); +} + +/// whether at least one of the request ETags matches entity ETag +bool +StoreEntry::hasOneOfEtags(const String &reqETags, const bool allowWeakMatch) const +{ + const ETag repETag = getReply()->header.getETag(HDR_ETAG); + if (!repETag.str) + return strListIsMember(&reqETags, "*", ','); + + bool matched = false; + const char *pos = NULL; + const char *item; + int ilen; + while (!matched && strListGetItem(&reqETags, ',', &item, &ilen, &pos)) { + if (!strncmp(item, "*", ilen)) + matched = true; + else { + String str; + str.append(item, ilen); + ETag reqETag; + if (etagParseInit(&reqETag, str.termedBuf())) { + matched = allowWeakMatch ? etagIsWeakEqual(repETag, reqETag) : + etagIsStrongEqual(repETag, reqETag); + } + } + } + return matched; +} + StorePointer StoreEntry::store() const { -- 2.47.2