From 3aa8a71feffda7898542e30e05b221ae2b5b9292 Mon Sep 17 00:00:00 2001 From: "darin%netscape.com" Date: Tue, 13 May 2003 04:38:25 +0000 Subject: [PATCH] fixes bug 132957 "Implement full support for Vary header [was: Content negotiation prohibits caching]" r=suresh sr=alecf a=asa --- netwerk/protocol/http/src/nsHttp.h | 2 + netwerk/protocol/http/src/nsHttpChannel.cpp | 99 +++++++++++++++++++ netwerk/protocol/http/src/nsHttpChannel.h | 1 + .../protocol/http/src/nsHttpResponseHead.cpp | 19 ---- 4 files changed, 102 insertions(+), 19 deletions(-) diff --git a/netwerk/protocol/http/src/nsHttp.h b/netwerk/protocol/http/src/nsHttp.h index 6eb0716efd69..3068f2aff690 100644 --- a/netwerk/protocol/http/src/nsHttp.h +++ b/netwerk/protocol/http/src/nsHttp.h @@ -97,6 +97,8 @@ typedef PRUint8 nsHttpVersion; #define NS_HTTP_DEFAULT_PORT 80 #define NS_HTTPS_DEFAULT_PORT 443 +#define NS_HTTP_HEADER_SEPS ", \t" + //----------------------------------------------------------------------------- // http atoms... //----------------------------------------------------------------------------- diff --git a/netwerk/protocol/http/src/nsHttpChannel.cpp b/netwerk/protocol/http/src/nsHttpChannel.cpp index 1852ee58d5bd..53524320e5c7 100644 --- a/netwerk/protocol/http/src/nsHttpChannel.cpp +++ b/netwerk/protocol/http/src/nsHttpChannel.cpp @@ -829,6 +829,66 @@ nsHttpChannel::ProxyFailover() return rv; } +PRBool +nsHttpChannel::ResponseWouldVary() +{ + PRBool result = PR_FALSE; + nsCAutoString buf, metaKey; + mCachedResponseHead->GetHeader(nsHttp::Vary, buf); + if (!buf.IsEmpty()) { + NS_NAMED_LITERAL_CSTRING(prefix, "request-"); + + // enumerate the elements of the Vary header... + char *val = NS_CONST_CAST(char *, buf.get()); // going to munge buf + char *token = nsCRT::strtok(val, NS_HTTP_HEADER_SEPS, &val); + while (token) { + // + // if "*", then assume response would vary. technically speaking, + // "Vary: header, *" is not permitted, but we allow it anyways. + // + // if the response depends on the value of the "Cookie" header, then + // bail since we do not store cookies in the cache. this is done + // for the following reasons: + // + // 1- cookies can be very large in size + // + // 2- cookies may contain sensitive information. (for parity with + // out policy of not storing Set-cookie headers in the cache + // meta data, we likewise do not want to store cookie headers + // here.) + // + // this implementation is obviously not fully standards compliant, but + // it is perhaps most prudent given the above issues. + // + if ((*token == '*') || (PL_strcasecmp(token, "cookie") == 0)) { + result = PR_TRUE; + break; + } + else { + // build cache meta data key... + metaKey = prefix + nsDependentCString(token); + + // check the last value of the given request header to see if it has + // since changed. if so, then indeed the cached response is invalid. + nsXPIDLCString lastVal; + mCacheEntry->GetMetaDataElement(metaKey.get(), getter_Copies(lastVal)); + if (lastVal) { + nsHttpAtom atom = nsHttp::ResolveAtom(token); + const char *newVal = mRequestHead.PeekHeader(atom); + if (newVal && (strcmp(newVal, lastVal) != 0)) { + result = PR_TRUE; // yes, response would vary + break; + } + } + + // next token... + token = nsCRT::strtok(val, NS_HTTP_HEADER_SEPS, &val); + } + } + } + return result; +} + //----------------------------------------------------------------------------- // nsHttpChannel //----------------------------------------------------------------------------- @@ -1244,6 +1304,11 @@ nsHttpChannel::CheckCache() LOG(("Validating based on MustValidate() returning TRUE\n")); doValidation = PR_TRUE; } + + else if (ResponseWouldVary()) { + LOG(("Validating based on Vary headers returning TRUE\n")); + doValidation = PR_TRUE; + } // Check if the cache entry has expired... else { PRUint32 time = 0; // a temporary variable for storing time values... @@ -1474,6 +1539,40 @@ nsHttpChannel::InitCacheEntry() rv = StoreAuthorizationMetaData(); if (NS_FAILED(rv)) return rv; + // Iterate over the headers listed in the Vary response header, and + // store the value of the corresponding request header so we can verify + // that it has not varied when we try to re-use the cached response at + // a later time. Take care not to store "Cookie" headers though. We + // take care of "Vary: cookie" in ResponseWouldVary. + // + // NOTE: if "Vary: accept, cookie", then we will store the "accept" header + // in the cache. we could try to avoid needlessly storing the "accept" + // header in this case, but it doesn't seem worth the extra code to perform + // the check. + { + nsCAutoString buf, metaKey; + mResponseHead->GetHeader(nsHttp::Vary, buf); + if (!buf.IsEmpty()) { + NS_NAMED_LITERAL_CSTRING(prefix, "request-"); + + char *val = NS_CONST_CAST(char *, buf.get()); // going to munge buf + char *token = nsCRT::strtok(val, NS_HTTP_HEADER_SEPS, &val); + while (token) { + if ((*token != '*') && (PL_strcasecmp(token, "cookie") != 0)) { + nsHttpAtom atom = nsHttp::ResolveAtom(token); + const char *requestVal = mRequestHead.PeekHeader(atom); + if (requestVal) { + // build cache meta data key and set meta data element... + metaKey = prefix + nsDependentCString(token); + mCacheEntry->SetMetaDataElement(metaKey.get(), requestVal); + } + } + token = nsCRT::strtok(val, NS_HTTP_HEADER_SEPS, &val); + } + } + } + + // Store the received HTTP head with the cache entry as an element of // the meta data. nsCAutoString head; diff --git a/netwerk/protocol/http/src/nsHttpChannel.h b/netwerk/protocol/http/src/nsHttpChannel.h index 56d4ba4c7899..eb45aa2ad1f5 100644 --- a/netwerk/protocol/http/src/nsHttpChannel.h +++ b/netwerk/protocol/http/src/nsHttpChannel.h @@ -118,6 +118,7 @@ private: nsresult ProcessRedirection(PRUint32 httpStatus); nsresult ProcessAuthentication(PRUint32 httpStatus); nsresult GetCallback(const nsIID &aIID, void **aResult); + PRBool ResponseWouldVary(); // redirection specific methods void HandleAsyncRedirect(); diff --git a/netwerk/protocol/http/src/nsHttpResponseHead.cpp b/netwerk/protocol/http/src/nsHttpResponseHead.cpp index 4d084c793f73..162fade9df53 100644 --- a/netwerk/protocol/http/src/nsHttpResponseHead.cpp +++ b/netwerk/protocol/http/src/nsHttpResponseHead.cpp @@ -328,25 +328,6 @@ nsHttpResponseHead::MustValidate() return PR_TRUE; } - // Check the Vary header. Per comments on bug 37609, most of the request - // headers that we generate do not vary with the exception of Accept-Charset - // and Accept-Language, so we force validation only if these headers or "*" - // are listed with the Vary response header. - // - // XXX this may not be sufficient if embedders start tweaking or adding HTTP - // request headers. - // - // XXX will need to add the Accept header to this list if we start sending - // a full Accept header, since the addition of plugins could change this - // header (see bug 58040). - val = PeekHeader(nsHttp::Vary); - if (val && (PL_strstr(val, "*") || - PL_strcasestr(val, "accept-charset") || - PL_strcasestr(val, "accept-language"))) { - LOG(("Must validate based on \"%s\" header\n", val)); - return PR_TRUE; - } - LOG(("no mandatory validation requirement\n")); return PR_FALSE; }