From ae599f98459ecef06d173c90188e335e081e12b7 Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Tue, 18 Mar 2014 12:36:18 -0400 Subject: [PATCH] bug 477578 - http methods should be case sensitive r=hurley --- content/base/src/nsXMLHttpRequest.cpp | 30 ++++++-- netwerk/ipc/NeckoChannelParams.ipdlh | 2 +- netwerk/protocol/http/Http2Stream.cpp | 16 ++--- netwerk/protocol/http/HttpBaseChannel.cpp | 38 ++++++---- netwerk/protocol/http/HttpBaseChannel.h | 6 ++ netwerk/protocol/http/HttpChannelChild.cpp | 4 +- netwerk/protocol/http/HttpChannelParent.cpp | 2 +- netwerk/protocol/http/HttpChannelParent.h | 2 +- netwerk/protocol/http/SpdyStream3.cpp | 14 ++-- netwerk/protocol/http/SpdyStream31.cpp | 14 ++-- netwerk/protocol/http/nsHttp.cpp | 29 -------- netwerk/protocol/http/nsHttp.h | 8 --- netwerk/protocol/http/nsHttpAtomList.h | 25 +------ netwerk/protocol/http/nsHttpChannel.cpp | 71 +++++++++++-------- netwerk/protocol/http/nsHttpChannel.h | 2 +- netwerk/protocol/http/nsHttpConnection.cpp | 2 +- netwerk/protocol/http/nsHttpRequestHead.cpp | 49 ++++++++++++- netwerk/protocol/http/nsHttpRequestHead.h | 33 +++++++-- netwerk/protocol/http/nsHttpTransaction.cpp | 9 +-- netwerk/test/mochitests/method.sjs | 12 ++++ netwerk/test/mochitests/mochitest.ini | 2 + .../test/mochitests/test_xhr_method_case.html | 66 +++++++++++++++++ netwerk/test/unit/test_bug412945.js | 2 +- netwerk/test/unit/test_bug477578.js | 51 +++++++++++++ netwerk/test/unit/test_bug618835.js | 4 +- netwerk/test/unit/xpcshell.ini | 1 + 26 files changed, 345 insertions(+), 149 deletions(-) create mode 100644 netwerk/test/mochitests/method.sjs create mode 100644 netwerk/test/mochitests/test_xhr_method_case.html create mode 100644 netwerk/test/unit/test_bug477578.js diff --git a/content/base/src/nsXMLHttpRequest.cpp b/content/base/src/nsXMLHttpRequest.cpp index a06b896b932c..b6bcc5f3375f 100644 --- a/content/base/src/nsXMLHttpRequest.cpp +++ b/content/base/src/nsXMLHttpRequest.cpp @@ -1537,11 +1537,11 @@ nsXMLHttpRequest::Open(const nsACString& method, const nsACString& url, } nsresult -nsXMLHttpRequest::Open(const nsACString& method, const nsACString& url, +nsXMLHttpRequest::Open(const nsACString& inMethod, const nsACString& url, bool async, const Optional& user, const Optional& password) { - NS_ENSURE_ARG(!method.IsEmpty()); + NS_ENSURE_ARG(!inMethod.IsEmpty()); if (!async && !DontWarnAboutSyncXHR() && GetOwner() && GetOwner()->GetExtantDoc()) { @@ -1555,9 +1555,29 @@ nsXMLHttpRequest::Open(const nsACString& method, const nsACString& url, // Disallow HTTP/1.1 TRACE method (see bug 302489) // and MS IIS equivalent TRACK (see bug 381264) - if (method.LowerCaseEqualsLiteral("trace") || - method.LowerCaseEqualsLiteral("track")) { - return NS_ERROR_INVALID_ARG; + // and CONNECT + if (inMethod.LowerCaseEqualsLiteral("trace") || + inMethod.LowerCaseEqualsLiteral("connect") || + inMethod.LowerCaseEqualsLiteral("track")) { + return NS_ERROR_DOM_SECURITY_ERR; + } + + nsAutoCString method; + // GET, POST, DELETE, HEAD, OPTIONS, PUT methods normalized to upper case + if (inMethod.LowerCaseEqualsLiteral("get")) { + method.Assign(NS_LITERAL_CSTRING("GET")); + } else if (inMethod.LowerCaseEqualsLiteral("post")) { + method.Assign(NS_LITERAL_CSTRING("POST")); + } else if (inMethod.LowerCaseEqualsLiteral("delete")) { + method.Assign(NS_LITERAL_CSTRING("DELETE")); + } else if (inMethod.LowerCaseEqualsLiteral("head")) { + method.Assign(NS_LITERAL_CSTRING("HEAD")); + } else if (inMethod.LowerCaseEqualsLiteral("options")) { + method.Assign(NS_LITERAL_CSTRING("OPTIONS")); + } else if (inMethod.LowerCaseEqualsLiteral("put")) { + method.Assign(NS_LITERAL_CSTRING("PUT")); + } else { + method = inMethod; // other methods are not normalized } // sync request is not allowed using withCredential or responseType diff --git a/netwerk/ipc/NeckoChannelParams.ipdlh b/netwerk/ipc/NeckoChannelParams.ipdlh index a8ddd29e785f..1ee2daa702a9 100644 --- a/netwerk/ipc/NeckoChannelParams.ipdlh +++ b/netwerk/ipc/NeckoChannelParams.ipdlh @@ -33,7 +33,7 @@ struct HttpChannelOpenArgs OptionalURIParams apiRedirectTo; uint32_t loadFlags; RequestHeaderTuples requestHeaders; - nsHttpAtom requestMethod; + nsCString requestMethod; OptionalInputStreamParams uploadStream; bool uploadStreamHasHeaders; uint16_t priority; diff --git a/netwerk/protocol/http/Http2Stream.cpp b/netwerk/protocol/http/Http2Stream.cpp index 7bbc04e8039c..a04d519d0831 100644 --- a/netwerk/protocol/http/Http2Stream.cpp +++ b/netwerk/protocol/http/Http2Stream.cpp @@ -300,7 +300,7 @@ Http2Stream::ParseHttpRequestHeaders(const char *buf, mOrigin, hashkey); // check the push cache for GET - if (mTransaction->RequestHead()->Method() == nsHttp::Get) { + if (mTransaction->RequestHead()->IsGet()) { // from :scheme, :authority, :path nsILoadGroupConnectionInfo *loadGroupCI = mTransaction->LoadGroupConnectionInfo(); SpdyPushCache *cache = nullptr; @@ -356,7 +356,7 @@ Http2Stream::ParseHttpRequestHeaders(const char *buf, nsCString compressedData; mSession->Compressor()->EncodeHeaderBlock(mFlatHttpRequestHeaders, - nsCString(mTransaction->RequestHead()->Method().get()), + mTransaction->RequestHead()->Method(), mTransaction->RequestHead()->RequestURI(), hostHeader, NS_LITERAL_CSTRING("https"), @@ -366,17 +366,17 @@ Http2Stream::ParseHttpRequestHeaders(const char *buf, // to wait for a data packet to put it on. uint8_t firstFrameFlags = Http2Session::kFlag_PRIORITY; - if (mTransaction->RequestHead()->Method() == nsHttp::Get || - mTransaction->RequestHead()->Method() == nsHttp::Connect || - mTransaction->RequestHead()->Method() == nsHttp::Head) { + if (mTransaction->RequestHead()->IsGet() || + mTransaction->RequestHead()->IsConnect() || + mTransaction->RequestHead()->IsHead()) { // for GET, CONNECT, and HEAD place the fin bit right on the // header packet SetSentFin(true); firstFrameFlags |= Http2Session::kFlag_END_STREAM; - } else if (mTransaction->RequestHead()->Method() == nsHttp::Post || - mTransaction->RequestHead()->Method() == nsHttp::Put || - mTransaction->RequestHead()->Method() == nsHttp::Options) { + } else if (mTransaction->RequestHead()->IsPost() || + mTransaction->RequestHead()->IsPut() || + mTransaction->RequestHead()->IsOptions()) { // place fin in a data frame even for 0 length messages for iterop } else if (!mRequestBodyLenRemaining) { // for other HTTP extension methods, rely on the content-length diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp index a87e26bce323..d55fea0910af 100644 --- a/netwerk/protocol/http/HttpBaseChannel.cpp +++ b/netwerk/protocol/http/HttpBaseChannel.cpp @@ -121,8 +121,8 @@ HttpBaseChannel::Init(nsIURI *aURI, if (NS_FAILED(rv)) return rv; LOG(("uri=%s\n", mSpec.get())); - // Set default request method - mRequestHead.SetMethod(nsHttp::Get); + // Assert default request method + MOZ_ASSERT(mRequestHead.EqualsMethod(nsHttpRequestHead::kMethod_Get)); // Set request headers nsAutoCString hostLine; @@ -490,10 +490,10 @@ HttpBaseChannel::SetUploadStream(nsIInputStream *stream, bool hasHeaders; if (contentType.IsEmpty()) { - method = nsHttp::Post; + method = NS_LITERAL_CSTRING("POST"); hasHeaders = true; } else { - method = nsHttp::Put; + method = NS_LITERAL_CSTRING("PUT"); hasHeaders = false; } return ExplicitSetUploadStream(stream, contentType, contentLength, @@ -503,7 +503,7 @@ HttpBaseChannel::SetUploadStream(nsIInputStream *stream, // if stream is null, ExplicitSetUploadStream returns error. // So we need special case for GET method. mUploadStreamHasHeaders = false; - mRequestHead.SetMethod(nsHttp::Get); // revert to GET request + mRequestHead.SetMethod(NS_LITERAL_CSTRING("GET")); // revert to GET request mUploadStream = stream; return NS_OK; } @@ -826,11 +826,7 @@ HttpBaseChannel::SetRequestMethod(const nsACString& aMethod) if (!nsHttp::IsValidToken(flatMethod)) return NS_ERROR_INVALID_ARG; - nsHttpAtom atom = nsHttp::ResolveAtom(flatMethod.get()); - if (!atom) - return NS_ERROR_FAILURE; - - mRequestHead.SetMethod(atom); + mRequestHead.SetMethod(flatMethod); return NS_OK; } @@ -1610,7 +1606,7 @@ HttpBaseChannel::GetEntityID(nsACString& aEntityID) { // Don't return an entity ID for Non-GET requests which require // additional data - if (mRequestHead.Method() != nsHttp::Get) { + if (!mRequestHead.IsGet()) { return NS_ERROR_NOT_RESUMABLE; } @@ -1747,6 +1743,22 @@ CopyProperties(const nsAString& aKey, nsIVariant *aData, void *aClosure) return PL_DHASH_NEXT; } +bool +HttpBaseChannel::ShouldRewriteRedirectToGET(uint32_t httpStatus, + nsHttpRequestHead::ParsedMethodType method) +{ + // for 301 and 302, only rewrite POST + if (httpStatus == 301 || httpStatus == 302) + return method == nsHttpRequestHead::kMethod_Post; + + // rewrite for 303 unless it was HEAD + if (httpStatus == 303) + return method != nsHttpRequestHead::kMethod_Head; + + // otherwise, such as for 307, do not rewrite + return false; +} + nsresult HttpBaseChannel::SetupReplacementChannel(nsIURI *newURI, nsIChannel *newChannel, @@ -1807,7 +1819,7 @@ HttpBaseChannel::SetupReplacementChannel(nsIURI *newURI, int64_t len = clen ? nsCRT::atoll(clen) : -1; uploadChannel2->ExplicitSetUploadStream( mUploadStream, nsDependentCString(ctype), len, - nsDependentCString(mRequestHead.Method()), + mRequestHead.Method(), mUploadStreamHasHeaders); } else { if (mUploadStreamHasHeaders) { @@ -1834,7 +1846,7 @@ HttpBaseChannel::SetupReplacementChannel(nsIURI *newURI, // we set the upload stream above. This means SetRequestMethod() will // be called twice if ExplicitSetUploadStream() gets called above. - httpChannel->SetRequestMethod(nsDependentCString(mRequestHead.Method())); + httpChannel->SetRequestMethod(mRequestHead.Method()); } // convey the referrer if one was used for this channel to the next one if (mReferrer) diff --git a/netwerk/protocol/http/HttpBaseChannel.h b/netwerk/protocol/http/HttpBaseChannel.h index cd2814f29bf7..e47563b5c2fe 100644 --- a/netwerk/protocol/http/HttpBaseChannel.h +++ b/netwerk/protocol/http/HttpBaseChannel.h @@ -205,6 +205,12 @@ public: public: /* Necko internal use only... */ + + // Return whether upon a redirect code of httpStatus for method, the + // request method should be rewritten to GET. + static bool ShouldRewriteRedirectToGET(uint32_t httpStatus, + nsHttpRequestHead::ParsedMethodType method); + protected: nsCOMArray mSecurityConsoleMessages; diff --git a/netwerk/protocol/http/HttpChannelChild.cpp b/netwerk/protocol/http/HttpChannelChild.cpp index 17a3bdc20a13..5d5c376dc71e 100644 --- a/netwerk/protocol/http/HttpChannelChild.cpp +++ b/netwerk/protocol/http/HttpChannelChild.cpp @@ -786,8 +786,8 @@ HttpChannelChild::Redirect1Begin(const uint32_t& newChannelId, mResponseHead = new nsHttpResponseHead(responseHead); SetCookie(mResponseHead->PeekHeader(nsHttp::Set_Cookie)); - bool rewriteToGET = nsHttp::ShouldRewriteRedirectToGET( - mResponseHead->Status(), mRequestHead.Method()); + bool rewriteToGET = HttpBaseChannel::ShouldRewriteRedirectToGET(mResponseHead->Status(), + mRequestHead.ParsedMethod()); rv = SetupReplacementChannel(uri, newChannel, !rewriteToGET); if (NS_FAILED(rv)) { diff --git a/netwerk/protocol/http/HttpChannelParent.cpp b/netwerk/protocol/http/HttpChannelParent.cpp index 5aa8195b0d90..a1150f4851a4 100644 --- a/netwerk/protocol/http/HttpChannelParent.cpp +++ b/netwerk/protocol/http/HttpChannelParent.cpp @@ -147,7 +147,7 @@ HttpChannelParent::DoAsyncOpen( const URIParams& aURI, const OptionalURIParams& aAPIRedirectToURI, const uint32_t& loadFlags, const RequestHeaderTuples& requestHeaders, - const nsHttpAtom& requestMethod, + const nsCString& requestMethod, const OptionalInputStreamParams& uploadStream, const bool& uploadStreamHasHeaders, const uint16_t& priority, diff --git a/netwerk/protocol/http/HttpChannelParent.h b/netwerk/protocol/http/HttpChannelParent.h index c7948f15ddd1..d41934daa53b 100644 --- a/netwerk/protocol/http/HttpChannelParent.h +++ b/netwerk/protocol/http/HttpChannelParent.h @@ -78,7 +78,7 @@ protected: const OptionalURIParams& internalRedirectUri, const uint32_t& loadFlags, const RequestHeaderTuples& requestHeaders, - const nsHttpAtom& requestMethod, + const nsCString& requestMethod, const OptionalInputStreamParams& uploadStream, const bool& uploadStreamHasHeaders, const uint16_t& priority, diff --git a/netwerk/protocol/http/SpdyStream3.cpp b/netwerk/protocol/http/SpdyStream3.cpp index 8359070ae5c5..869bb73b9ba7 100644 --- a/netwerk/protocol/http/SpdyStream3.cpp +++ b/netwerk/protocol/http/SpdyStream3.cpp @@ -289,7 +289,7 @@ SpdyStream3::ParseHttpRequestHeaders(const char *buf, mOrigin, hashkey); // check the push cache for GET - if (mTransaction->RequestHead()->Method() == nsHttp::Get) { + if (mTransaction->RequestHead()->IsGet()) { // from :scheme, :host, :path nsILoadGroupConnectionInfo *loadGroupCI = mTransaction->LoadGroupConnectionInfo(); SpdyPushCache *cache = nullptr; @@ -483,18 +483,18 @@ SpdyStream3::ParseHttpRequestHeaders(const char *buf, // Determine whether to put the fin bit on the syn stream frame or whether // to wait for a data packet to put it on. - if (mTransaction->RequestHead()->Method() == nsHttp::Get || - mTransaction->RequestHead()->Method() == nsHttp::Connect || - mTransaction->RequestHead()->Method() == nsHttp::Head) { + if (mTransaction->RequestHead()->IsGet() || + mTransaction->RequestHead()->IsConnect() || + mTransaction->RequestHead()->IsHead()) { // for GET, CONNECT, and HEAD place the fin bit right on the // syn stream packet mSentFinOnData = 1; mTxInlineFrame[4] = SpdySession3::kFlag_Data_FIN; } - else if (mTransaction->RequestHead()->Method() == nsHttp::Post || - mTransaction->RequestHead()->Method() == nsHttp::Put || - mTransaction->RequestHead()->Method() == nsHttp::Options) { + else if (mTransaction->RequestHead()->IsPost() || + mTransaction->RequestHead()->IsPut() || + mTransaction->RequestHead()->IsOptions()) { // place fin in a data frame even for 0 length messages, I've seen // the google gateway be unhappy with fin-on-syn for 0 length POST } diff --git a/netwerk/protocol/http/SpdyStream31.cpp b/netwerk/protocol/http/SpdyStream31.cpp index b1e001284721..5f36d2204ce1 100644 --- a/netwerk/protocol/http/SpdyStream31.cpp +++ b/netwerk/protocol/http/SpdyStream31.cpp @@ -295,7 +295,7 @@ SpdyStream31::ParseHttpRequestHeaders(const char *buf, mOrigin, hashkey); // check the push cache for GET - if (mTransaction->RequestHead()->Method() == nsHttp::Get) { + if (mTransaction->RequestHead()->IsGet()) { // from :scheme, :host, :path nsILoadGroupConnectionInfo *loadGroupCI = mTransaction->LoadGroupConnectionInfo(); SpdyPushCache *cache = nullptr; @@ -490,18 +490,18 @@ SpdyStream31::ParseHttpRequestHeaders(const char *buf, // Determine whether to put the fin bit on the syn stream frame or whether // to wait for a data packet to put it on. - if (mTransaction->RequestHead()->Method() == nsHttp::Get || - mTransaction->RequestHead()->Method() == nsHttp::Connect || - mTransaction->RequestHead()->Method() == nsHttp::Head) { + if (mTransaction->RequestHead()->IsGet() || + mTransaction->RequestHead()->IsConnect() || + mTransaction->RequestHead()->IsHead()) { // for GET, CONNECT, and HEAD place the fin bit right on the // syn stream packet mSentFinOnData = 1; mTxInlineFrame[4] = SpdySession31::kFlag_Data_FIN; } - else if (mTransaction->RequestHead()->Method() == nsHttp::Post || - mTransaction->RequestHead()->Method() == nsHttp::Put || - mTransaction->RequestHead()->Method() == nsHttp::Options) { + else if (mTransaction->RequestHead()->IsPost() || + mTransaction->RequestHead()->IsPut() || + mTransaction->RequestHead()->IsOptions()) { // place fin in a data frame even for 0 length messages, I've seen // the google gateway be unhappy with fin-on-syn for 0 length POST } diff --git a/netwerk/protocol/http/nsHttp.cpp b/netwerk/protocol/http/nsHttp.cpp index 40730468255f..390d79f4c27e 100644 --- a/netwerk/protocol/http/nsHttp.cpp +++ b/netwerk/protocol/http/nsHttp.cpp @@ -295,34 +295,5 @@ nsHttp::IsPermanentRedirect(uint32_t httpStatus) return httpStatus == 301 || httpStatus == 308; } -bool -nsHttp::ShouldRewriteRedirectToGET(uint32_t httpStatus, nsHttpAtom method) -{ - // for 301 and 302, only rewrite POST - if (httpStatus == 301 || httpStatus == 302) - return method == nsHttp::Post; - - // rewrite for 303 unless it was HEAD - if (httpStatus == 303) - return method != nsHttp::Head; - - // otherwise, such as for 307, do not rewrite - return false; -} - -bool -nsHttp::IsSafeMethod(nsHttpAtom method) -{ - // This code will need to be extended for new safe methods, otherwise - // they'll default to "not safe". - return method == nsHttp::Get || - method == nsHttp::Head || - method == nsHttp::Options || - method == nsHttp::Propfind || - method == nsHttp::Report || - method == nsHttp::Search || - method == nsHttp::Trace; -} - } // namespace mozilla::net } // namespace mozilla diff --git a/netwerk/protocol/http/nsHttp.h b/netwerk/protocol/http/nsHttp.h index f13f2e228d24..f94cbf9f7018 100644 --- a/netwerk/protocol/http/nsHttp.h +++ b/netwerk/protocol/http/nsHttp.h @@ -160,14 +160,6 @@ struct nsHttp // Return whether the HTTP status code represents a permanent redirect static bool IsPermanentRedirect(uint32_t httpStatus); - // Return whether upon a redirect code of httpStatus for method, the - // request method should be rewritten to GET. - static bool ShouldRewriteRedirectToGET(uint32_t httpStatus, nsHttpAtom method); - - // Return whether the specified method is safe as per RFC 2616, - // Section 9.1.1. - static bool IsSafeMethod(nsHttpAtom method); - // Declare all atoms // // The atom names and values are stored in nsHttpAtomList.h and are brought diff --git a/netwerk/protocol/http/nsHttpAtomList.h b/netwerk/protocol/http/nsHttpAtomList.h index 356be1a6bbe2..ac10a4e297b9 100644 --- a/netwerk/protocol/http/nsHttpAtomList.h +++ b/netwerk/protocol/http/nsHttpAtomList.h @@ -86,27 +86,4 @@ HTTP_ATOM(WWW_Authenticate, "WWW-Authenticate") HTTP_ATOM(Warning, "Warning") HTTP_ATOM(X_Firefox_Spdy, "X-Firefox-Spdy") -// methods are atoms too. -// -// Note: winnt.h defines DELETE macro, so we'll just keep the methods mixedcase -// even though they're normally written all uppercase. -- darin - -HTTP_ATOM(Connect, "CONNECT") -HTTP_ATOM(Copy, "COPY") -HTTP_ATOM(Delete, "DELETE") -HTTP_ATOM(Get, "GET") -HTTP_ATOM(Head, "HEAD") -HTTP_ATOM(Index, "INDEX") -HTTP_ATOM(Lock, "LOCK") -HTTP_ATOM(M_Post, "M-POST") -HTTP_ATOM(Mkcol, "MKCOL") -HTTP_ATOM(Move, "MOVE") -HTTP_ATOM(Options, "OPTIONS") -HTTP_ATOM(Post, "POST") -HTTP_ATOM(Propfind, "PROPFIND") -HTTP_ATOM(Proppatch, "PROPPATCH") -HTTP_ATOM(Put, "PUT") -HTTP_ATOM(Report, "REPORT") -HTTP_ATOM(Search, "SEARCH") -HTTP_ATOM(Trace, "TRACE") -HTTP_ATOM(Unlock, "UNLOCK") +// methods are case sensitive and do not use atom table diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 72cdb55bea3b..f0c37d47946e 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -622,6 +622,24 @@ nsHttpChannel::RetrieveSSLOptions() } } +static bool +SafeForPipelining(nsHttpRequestHead::ParsedMethodType method, + const nsCString &methodString) +{ + if (method == nsHttpRequestHead::kMethod_Get || + method == nsHttpRequestHead::kMethod_Head || + method == nsHttpRequestHead::kMethod_Options) { + return true; + } + + if (method != nsHttpRequestHead::kMethod_Custom) { + return false; + } + + return (!strcmp(methodString.get(), "PROPFIND") || + !strcmp(methodString.get(), "PROPPATCH")); +} + nsresult nsHttpChannel::SetupTransaction() { @@ -642,11 +660,7 @@ nsHttpChannel::SetupTransaction() // if (!mAllowPipelining || (mLoadFlags & (LOAD_INITIAL_DOCUMENT_URI | INHIBIT_PIPELINE)) || - !(mRequestHead.Method() == nsHttp::Get || - mRequestHead.Method() == nsHttp::Head || - mRequestHead.Method() == nsHttp::Options || - mRequestHead.Method() == nsHttp::Propfind || - mRequestHead.Method() == nsHttp::Proppatch)) { + !SafeForPipelining(mRequestHead.ParsedMethod(), mRequestHead.Method())) { LOG((" pipelining disallowed\n")); mCaps &= ~NS_HTTP_ALLOW_PIPELINING; } @@ -2483,7 +2497,7 @@ nsHttpChannel::OpenCacheEntry(bool usingSSL) nsresult rv; mLoadedFromApplicationCache = false; - mHasQueryString = HasQueryString(mRequestHead.Method(), mURI); + mHasQueryString = HasQueryString(mRequestHead.ParsedMethod(), mURI); LOG(("nsHttpChannel::OpenCacheEntry [this=%p]", this)); @@ -2492,15 +2506,14 @@ nsHttpChannel::OpenCacheEntry(bool usingSSL) nsAutoCString cacheKey; - if (mRequestHead.Method() == nsHttp::Post) { + if (mRequestHead.IsPost()) { // If the post id is already set then this is an attempt to replay // a post transaction via the cache. Otherwise, we need a unique // post id for this transaction. if (mPostID == 0) mPostID = gHttpHandler->GenerateUniqueID(); } - else if ((mRequestHead.Method() != nsHttp::Get) && - (mRequestHead.Method() != nsHttp::Head)) { + else if (!mRequestHead.IsGet() && !mRequestHead.IsHead()) { // don't use the cache for other types of requests return NS_OK; } @@ -2613,7 +2626,7 @@ bypassCacheEntryOpen: return NS_OK; } - if (mRequestHead.Method() != nsHttp::Get) { + if (!mRequestHead.IsGet()) { // only cache complete documents offline return NS_OK; } @@ -2685,12 +2698,15 @@ nsHttpChannel::OnCacheEntryCheck(nsICacheEntry* entry, nsIApplicationCache* appC rv = entry->GetMetaDataElement("request-method", getter_Copies(buf)); NS_ENSURE_SUCCESS(rv, rv); - nsHttpAtom method = nsHttp::ResolveAtom(buf); - if (method == nsHttp::Head) { + bool methodWasHead = buf.EqualsLiteral("HEAD"); + bool methodWasGet = buf.EqualsLiteral("GET"); + + if (methodWasHead) { // The cached response does not contain an entity. We can only reuse // the response if the current request is also HEAD. - if (mRequestHead.Method() != nsHttp::Head) + if (!mRequestHead.IsHead()) { return NS_OK; + } } buf.Adopt(0); @@ -2739,7 +2755,7 @@ nsHttpChannel::OnCacheEntryCheck(nsICacheEntry* entry, nsIApplicationCache* appC bool wantCompleteEntry = false; - if (method != nsHttp::Head && !isCachedRedirect) { + if (!methodWasHead && !isCachedRedirect) { // If the cached content-length is set and it does not match the data // size of the cached content, then the cached response is partial... // either we need to issue a byte range request or we need to refetch @@ -2870,7 +2886,7 @@ nsHttpChannel::OnCacheEntryCheck(nsICacheEntry* entry, nsIApplicationCache* appC } if (!doValidation && mRequestHead.PeekHeader(nsHttp::If_Match) && - (method == nsHttp::Get || method == nsHttp::Head)) { + (methodWasGet || methodWasHead)) { const char *requestedETag, *cachedETag; cachedETag = mCachedResponseHead->PeekHeader(nsHttp::ETag); requestedETag = mRequestHead.PeekHeader(nsHttp::If_Match); @@ -2939,8 +2955,7 @@ nsHttpChannel::OnCacheEntryCheck(nsICacheEntry* entry, nsIApplicationCache* appC // // do not override conditional headers when consumer has defined its own if (!mCachedResponseHead->NoStore() && - (mRequestHead.Method() == nsHttp::Get || - mRequestHead.Method() == nsHttp::Head) && + (mRequestHead.IsGet() || mRequestHead.IsHead()) && !mCustomConditionalRequest) { if (mConcurentCacheAccess) { @@ -3294,13 +3309,14 @@ nsHttpChannel::UpdateExpirationTime() } /*static*/ inline bool -nsHttpChannel::HasQueryString(nsHttpAtom method, nsIURI * uri) +nsHttpChannel::HasQueryString(nsHttpRequestHead::ParsedMethodType method, nsIURI * uri) { // Must be called on the main thread because nsIURI does not implement // thread-safe QueryInterface. MOZ_ASSERT(NS_IsMainThread()); - if (method != nsHttp::Get && method != nsHttp::Head) + if (method != nsHttpRequestHead::kMethod_Get && + method != nsHttpRequestHead::kMethod_Head) return false; nsAutoCString query; @@ -4179,12 +4195,11 @@ nsHttpChannel::ContinueProcessRedirectionAfterFallback(nsresult rv) } } - bool rewriteToGET = nsHttp::ShouldRewriteRedirectToGET( - mRedirectType, mRequestHead.Method()); + bool rewriteToGET = ShouldRewriteRedirectToGET(mRedirectType, + mRequestHead.ParsedMethod()); // prompt if the method is not safe (such as POST, PUT, DELETE, ...) - if (!rewriteToGET && - !nsHttp::IsSafeMethod(mRequestHead.Method())) { + if (!rewriteToGET && !mRequestHead.IsSafeMethod()) { rv = PromptTempRedirect(); if (NS_FAILED(rv)) return rv; } @@ -6054,13 +6069,11 @@ nsHttpChannel::MaybeInvalidateCacheEntryForSubsequentGet() // referred resource. POST, PUT and DELETE as well as any // other method not listed here will potentially invalidate // any cached copy of the resource - if (mRequestHead.Method() == nsHttp::Options || - mRequestHead.Method() == nsHttp::Get || - mRequestHead.Method() == nsHttp::Head || - mRequestHead.Method() == nsHttp::Trace || - mRequestHead.Method() == nsHttp::Connect) + if (mRequestHead.IsGet() || mRequestHead.IsOptions() || + mRequestHead.IsHead() || mRequestHead.IsTrace() || + mRequestHead.IsConnect()) { return; - + } // Invalidate the request-uri. #ifdef PR_LOGGING diff --git a/netwerk/protocol/http/nsHttpChannel.h b/netwerk/protocol/http/nsHttpChannel.h index db6a0c602f81..59647ea8813d 100644 --- a/netwerk/protocol/http/nsHttpChannel.h +++ b/netwerk/protocol/http/nsHttpChannel.h @@ -316,7 +316,7 @@ private: // and ensure the transaction is updated to use it. void UpdateAggregateCallbacks(); - static bool HasQueryString(nsHttpAtom method, nsIURI * uri); + static bool HasQueryString(nsHttpRequestHead::ParsedMethodType method, nsIURI * uri); bool ResponseWouldVary(nsICacheEntry* entry) const; bool MustValidateBasedOnQueryUrl() const; bool IsResumable(int64_t partialLen, int64_t contentLength, diff --git a/netwerk/protocol/http/nsHttpConnection.cpp b/netwerk/protocol/http/nsHttpConnection.cpp index 42fc86f56770..6985db17df17 100644 --- a/netwerk/protocol/http/nsHttpConnection.cpp +++ b/netwerk/protocol/http/nsHttpConnection.cpp @@ -1577,7 +1577,7 @@ nsHttpConnection::SetupProxyConnect() // CONNECT host:port HTTP/1.1 nsHttpRequestHead request; - request.SetMethod(nsHttp::Connect); + request.SetMethod(NS_LITERAL_CSTRING("CONNECT")); request.SetVersion(gHttpHandler->HttpVersion()); request.SetRequestURI(buf); request.SetHeader(nsHttp::User_Agent, gHttpHandler->UserAgent()); diff --git a/netwerk/protocol/http/nsHttpRequestHead.cpp b/netwerk/protocol/http/nsHttpRequestHead.cpp index b367497c4a1f..238788964608 100644 --- a/netwerk/protocol/http/nsHttpRequestHead.cpp +++ b/netwerk/protocol/http/nsHttpRequestHead.cpp @@ -15,12 +15,59 @@ namespace mozilla { namespace net { +nsHttpRequestHead::nsHttpRequestHead() + : mMethod(NS_LITERAL_CSTRING("GET")) + , mVersion(NS_HTTP_VERSION_1_1) + , mParsedMethod(kMethod_Get) +{ +} + +void +nsHttpRequestHead::SetMethod(const nsACString &method) +{ + mParsedMethod = kMethod_Custom; + mMethod = method; + if (!strcmp(mMethod.get(), "GET")) { + mParsedMethod = kMethod_Get; + } else if (!strcmp(mMethod.get(), "POST")) { + mParsedMethod = kMethod_Post; + } else if (!strcmp(mMethod.get(), "OPTIONS")) { + mParsedMethod = kMethod_Options; + } else if (!strcmp(mMethod.get(), "CONNECT")) { + mParsedMethod = kMethod_Connect; + } else if (!strcmp(mMethod.get(), "HEAD")) { + mParsedMethod = kMethod_Head; + } else if (!strcmp(mMethod.get(), "PUT")) { + mParsedMethod = kMethod_Put; + } else if (!strcmp(mMethod.get(), "TRACE")) { + mParsedMethod = kMethod_Trace; + } +} + +bool +nsHttpRequestHead::IsSafeMethod() const +{ + // This code will need to be extended for new safe methods, otherwise + // they'll default to "not safe". + if (IsGet() || IsHead() || IsOptions() || IsTrace()) { + return true; + } + + if (mParsedMethod != kMethod_Custom) { + return false; + } + + return (!strcmp(mMethod.get(), "PROPFIND") || + !strcmp(mMethod.get(), "REPORT") || + !strcmp(mMethod.get(), "SEARCH")); +} + void nsHttpRequestHead::Flatten(nsACString &buf, bool pruneProxyHeaders) { // note: the first append is intentional. - buf.Append(mMethod.get()); + buf.Append(mMethod); buf.Append(' '); buf.Append(mRequestURI); buf.AppendLiteral(" HTTP/"); diff --git a/netwerk/protocol/http/nsHttpRequestHead.h b/netwerk/protocol/http/nsHttpRequestHead.h index 9eefd3bfd883..75455390a00b 100644 --- a/netwerk/protocol/http/nsHttpRequestHead.h +++ b/netwerk/protocol/http/nsHttpRequestHead.h @@ -20,15 +20,15 @@ namespace mozilla { namespace net { class nsHttpRequestHead { public: - nsHttpRequestHead() : mMethod(nsHttp::Get), mVersion(NS_HTTP_VERSION_1_1) {} + nsHttpRequestHead(); - void SetMethod(nsHttpAtom method) { mMethod = method; } + void SetMethod(const nsACString &method); void SetVersion(nsHttpVersion version) { mVersion = version; } void SetRequestURI(const nsCSubstring &s) { mRequestURI = s; } const nsHttpHeaderArray &Headers() const { return mHeaders; } nsHttpHeaderArray & Headers() { return mHeaders; } - nsHttpAtom Method() const { return mMethod; } + const nsCString &Method() const { return mMethod; } nsHttpVersion Version() const { return mVersion; } const nsCSubstring &RequestURI() const { return mRequestURI; } @@ -63,12 +63,37 @@ public: return NS_OK; } + bool IsSafeMethod() const; + + enum ParsedMethodType + { + kMethod_Custom, + kMethod_Get, + kMethod_Post, + kMethod_Options, + kMethod_Connect, + kMethod_Head, + kMethod_Put, + kMethod_Trace + }; + + ParsedMethodType ParsedMethod() const { return mParsedMethod; } + bool EqualsMethod(ParsedMethodType aType) const { return mParsedMethod == aType; } + bool IsGet() const { return EqualsMethod(kMethod_Get); } + bool IsPost() const { return EqualsMethod(kMethod_Post); } + bool IsOptions() const { return EqualsMethod(kMethod_Options); } + bool IsConnect() const { return EqualsMethod(kMethod_Connect); } + bool IsHead() const { return EqualsMethod(kMethod_Head); } + bool IsPut() const { return EqualsMethod(kMethod_Put); } + bool IsTrace() const { return EqualsMethod(kMethod_Trace); } + private: // All members must be copy-constructable and assignable nsHttpHeaderArray mHeaders; - nsHttpAtom mMethod; + nsCString mMethod; nsHttpVersion mVersion; nsCString mRequestURI; + ParsedMethodType mParsedMethod; }; }} // namespace mozilla::net diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp index b538a2da2430..155f487bcd95 100644 --- a/netwerk/protocol/http/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/nsHttpTransaction.cpp @@ -274,8 +274,9 @@ nsHttpTransaction::Init(uint32_t caps, mConsumerTarget = target; mCaps = caps; - if (requestHead->Method() == nsHttp::Head) + if (requestHead->IsHead()) { mNoContent = true; + } // Make sure that there is "Content-Length: 0" header in the requestHead // in case of POST and PUT methods when there is no requestBody and @@ -289,7 +290,7 @@ nsHttpTransaction::Init(uint32_t caps, // For compatibility with HTTP/1.0 applications, HTTP/1.1 requests // containing a message-body MUST include a valid Content-Length header // field unless the server is known to be HTTP/1.1 compliant. - if ((requestHead->Method() == nsHttp::Post || requestHead->Method() == nsHttp::Put) && + if ((requestHead->IsPost() || requestHead->IsPut()) && !requestBody && !requestHead->PeekHeader(nsHttp::Transfer_Encoding)) { requestHead->SetHeader(nsHttp::Content_Length, NS_LITERAL_CSTRING("0")); } @@ -1314,7 +1315,7 @@ nsHttpTransaction::ParseHead(char *buf, char *p = LocateHttpStart(buf, std::min(count, 11), true); if (!p) { // Treat any 0.9 style response of a put as a failure. - if (mRequestHead->Method() == nsHttp::Put) + if (mRequestHead->IsPut()) return NS_ERROR_ABORT; mResponseHead->ParseStatusLine(""); @@ -1497,7 +1498,7 @@ nsHttpTransaction::HandleContentStart() // The verifier only initializes itself once (from the first iteration of // a transaction that gets far enough to have response headers) - if (mRequestHead->Method() == nsHttp::Get) + if (mRequestHead->IsGet()) mRestartInProgressVerifier.Set(mContentLength, mResponseHead); return NS_OK; diff --git a/netwerk/test/mochitests/method.sjs b/netwerk/test/mochitests/method.sjs new file mode 100644 index 000000000000..386c15d79439 --- /dev/null +++ b/netwerk/test/mochitests/method.sjs @@ -0,0 +1,12 @@ + +function handleRequest(request, response) +{ + // avoid confusing cache behaviors + response.setHeader("Cache-Control", "no-cache", false); + response.setHeader("Content-Type", "text/plain", false); + response.setHeader("Access-Control-Allow-Origin", "*", false); + + // echo method + response.write(request.method); +} + diff --git a/netwerk/test/mochitests/mochitest.ini b/netwerk/test/mochitests/mochitest.ini index c0200e62b385..e1cf93e66fdf 100644 --- a/netwerk/test/mochitests/mochitest.ini +++ b/netwerk/test/mochitests/mochitest.ini @@ -1,6 +1,7 @@ [DEFAULT] skip-if = buildapp == 'b2g' || e10s support-files = + method.sjs partial_content.sjs user_agent.sjs user_agent_update.sjs @@ -10,3 +11,4 @@ support-files = [test_user_agent_overrides.html] [test_user_agent_updates.html] [test_user_agent_updates_reset.html] +[test_xhr_method_case.html] \ No newline at end of file diff --git a/netwerk/test/mochitests/test_xhr_method_case.html b/netwerk/test/mochitests/test_xhr_method_case.html new file mode 100644 index 000000000000..236ab210b188 --- /dev/null +++ b/netwerk/test/mochitests/test_xhr_method_case.html @@ -0,0 +1,66 @@ + + + + + Test for XHR Method casing + + + + + + +

+ +
+
+ + + diff --git a/netwerk/test/unit/test_bug412945.js b/netwerk/test/unit/test_bug412945.js index a978c6e1c6c9..9d4f3f5bb9ec 100644 --- a/netwerk/test/unit/test_bug412945.js +++ b/netwerk/test/unit/test_bug412945.js @@ -27,7 +27,7 @@ function run_test() { "/bug412945", null, null); channel.QueryInterface(Components.interfaces.nsIHttpChannel); - channel.requestMethod = "post"; + channel.requestMethod = "POST"; channel.asyncOpen(new TestListener(), null); do_test_pending(); diff --git a/netwerk/test/unit/test_bug477578.js b/netwerk/test/unit/test_bug477578.js new file mode 100644 index 000000000000..54db57b5c1e8 --- /dev/null +++ b/netwerk/test/unit/test_bug477578.js @@ -0,0 +1,51 @@ +// test that methods are not normalized + +const testMethods = [ + ["GET"], + ["get"], + ["Get"], + ["gET"], + ["gEt"], + ["post"], + ["POST"], + ["head"], + ["HEAD"], + ["put"], + ["PUT"], + ["delete"], + ["DELETE"], + ["connect"], + ["CONNECT"], + ["options"], + ["trace"], + ["track"], + ["copy"], + ["index"], + ["lock"], + ["m-post"], + ["mkcol"], + ["move"], + ["propfind"], + ["proppatch"], + ["unlock"], + ["link"], + ["LINK"], + ["foo"], + ["foO"], + ["fOo"], + ["Foo"] +] + +function run_test() { + var ios = + Cc["@mozilla.org/network/io-service;1"]. + getService(Ci.nsIIOService); + + var chan = ios.newChannel("http://localhost/", null, null) + .QueryInterface(Components.interfaces.nsIHttpChannel); + + for (var i = 0; i < testMethods.length; i++) { + chan.requestMethod = testMethods[i]; + do_check_eq(chan.requestMethod, testMethods[i]); + } +} diff --git a/netwerk/test/unit/test_bug618835.js b/netwerk/test/unit/test_bug618835.js index 684afa0e5772..32f6109b3b32 100644 --- a/netwerk/test/unit/test_bug618835.js +++ b/netwerk/test/unit/test_bug618835.js @@ -31,7 +31,7 @@ InitialListener.prototype = { do_execute_soon(function() { var channel = setupChannel("http://localhost:" + httpserv.identity.primaryPort + "/post"); - channel.requestMethod = "post"; + channel.requestMethod = "POST"; channel.asyncOpen(new RedirectingListener(), null); }); } @@ -46,7 +46,7 @@ RedirectingListener.prototype = { do_execute_soon(function() { var channel = setupChannel("http://localhost:" + httpserv.identity.primaryPort + "/post"); - channel.requestMethod = "post"; + channel.requestMethod = "POST"; channel.asyncOpen(new VerifyingListener(), null); }); } diff --git a/netwerk/test/unit/xpcshell.ini b/netwerk/test/unit/xpcshell.ini index 89f5e6427598..e85172e337ac 100644 --- a/netwerk/test/unit/xpcshell.ini +++ b/netwerk/test/unit/xpcshell.ini @@ -111,6 +111,7 @@ skip-if = os == "android" # Bug 675039: intermittent fail on Android-armv6 skip-if = os == "android" [test_bug470716.js] +[test_bug477578.js] [test_bug479413.js] [test_bug479485.js] [test_bug482601.js]