Bug 477578 - uppercase selected HTTP methods (CONNECT, DELETE, GET, HEAD, OPTIONS, POST, PUT, TRACE and TRACK) and make other methods case sensitive

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -776,7 +776,7 @@ HttpBaseChannel::nsContentEncodings::PrepareForNext(void)
 NS_IMETHODIMP
 HttpBaseChannel::GetRequestMethod(nsACString& aMethod)
 {
-  aMethod = mRequestHead.Method();
+  mRequestHead.Method()->ToUTF8String(aMethod);
   return NS_OK;
 }
 
@@ -785,16 +785,27 @@ HttpBaseChannel::SetRequestMethod(const nsACString& aMethod)
 {
   ENSURE_CALLED_BEFORE_ASYNC_OPEN();
 
-  const nsCString& flatMethod = PromiseFlatCString(aMethod);
+  nsCAutoString upperCaseMethod;
+  ToUpperCase(aMethod, upperCaseMethod);
 
   // Method names are restricted to valid HTTP tokens.
-  if (!nsHttp::IsValidToken(flatMethod))
+  if (!nsHttp::IsValidToken(upperCaseMethod))
     return NS_ERROR_INVALID_ARG;
 
-  nsHttpAtom atom = nsHttp::ResolveAtom(flatMethod.get());
+  nsCOMPtr<nsIAtom> atom = do_GetAtom(upperCaseMethod);
-  if (!atom)
-    return NS_ERROR_FAILURE;
 
+  // We've changed method names to case sensitive in bug 477578. Some
+  // methods are kept case insensitive to keep backward compatibility and
+  // to satisfy XMLHttpRequest specification which demands it.
+#define HTTP_METHOD_ATOM(name_, value_)
+#define HTTP_CASE_INSENSITIVE_METHOD_ATOM(name_, value_) \
+  if (nsHttp::name_ == atom) {} else
+#include "nsHttpAtomList.h"
+#undef HTTP_CASE_INSENSITIVE_METHOD_ATOM
+#undef HTTP_METHOD_ATOM
+  { // upper case atom doesn't match any case insensitive atom
+    atom = do_GetAtom(aMethod);
+  }
   mRequestHead.SetMethod(atom);
   return NS_OK;
 }
@@ -1543,7 +1554,7 @@ HttpBaseChannel::SetupReplacementChannel(nsIURI       *newURI,
         PRInt64 len = clen ? nsCRT::atoll(clen) : -1;
         uploadChannel2->ExplicitSetUploadStream(
                                   mUploadStream, nsDependentCString(ctype), len,
-                                  nsDependentCString(mRequestHead.Method()),
+                                  nsAtomCString(mRequestHead.Method()),
                                   mUploadStreamHasHeaders);
       } else {
         if (mUploadStreamHasHeaders) {
@@ -1570,7 +1581,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(nsAtomCString(mRequestHead.Method()));
   }
   // convey the referrer if one was used for this channel to the next one
   if (mReferrer)

netwerk/protocol/http/HttpChannelChild.cpp

@@ -1072,7 +1072,7 @@ HttpChannelChild::AsyncOpen(nsIStreamListener *listener, nsISupports *aContext)
 
   SendAsyncOpen(IPC::URI(mURI), IPC::URI(mOriginalURI),
                 IPC::URI(mDocumentURI), IPC::URI(mReferrer), mLoadFlags,
-                mRequestHeaders, mRequestHead.Method(),
+                mRequestHeaders, nsAtomCString(mRequestHead.Method()),
                 IPC::InputStream(mUploadStream), mUploadStreamHasHeaders,
                 mPriority, mRedirectionLimit, mAllowPipelining,
                 mForceAllowThirdPartyCookie, mSendResumeAt,

netwerk/protocol/http/HttpChannelParent.cpp

@@ -132,7 +132,7 @@ HttpChannelParent::RecvAsyncOpen(const IPC::URI&            aURI,
                                  const IPC::URI&            aReferrerURI,
                                  const PRUint32&            loadFlags,
                                  const RequestHeaderTuples& requestHeaders,
-                                 const nsHttpAtom&          requestMethod,
+                                 const nsCString&           requestMethod,
                                  const IPC::InputStream&    uploadStream,
                                  const PRBool&              uploadStreamHasHeaders,
                                  const PRUint16&            priority,
@@ -190,7 +190,7 @@ HttpChannelParent::RecvAsyncOpen(const IPC::URI&            aURI,
 
   httpChan->SetNotificationCallbacks(channelListener);
 
-  httpChan->SetRequestMethod(nsDependentCString(requestMethod.get()));
+  httpChan->SetRequestMethod(requestMethod);
 
   nsCOMPtr<nsIInputStream> stream(uploadStream);
   if (stream) {

netwerk/protocol/http/HttpChannelParent.h

@@ -86,7 +86,7 @@ protected:
                              const IPC::URI&            referrerUri,
                              const PRUint32&            loadFlags,
                              const RequestHeaderTuples& requestHeaders,
-                             const nsHttpAtom&          requestMethod,
+                             const nsCString&           requestMethod,
                              const IPC::InputStream&    uploadStream,
                              const PRBool&              uploadStreamHasHeaders,
                              const PRUint16&            priority,

netwerk/protocol/http/PHttpChannel.ipdl

@@ -47,7 +47,6 @@ include "prio.h";
 
 using RequestHeaderTuples;
 using nsHttpResponseHead;
-using nsHttpAtom;
 using IPC::URI;
 using IPC::InputStream;
 using PRNetAddr;
@@ -70,7 +69,7 @@ parent:
             URI                 referrer,
             PRUint32            loadFlags,
             RequestHeaderTuples requestHeaders,
-            nsHttpAtom          requestMethod,
+            nsCString           requestMethod,
             InputStream         uploadStream,
             PRBool              uploadStreamHasHeaders,
             PRUint16            priority,

netwerk/protocol/http/nsHttp.cpp

@@ -42,6 +42,7 @@
 #include "mozilla/Mutex.h"
 #include "nsCRT.h"
 #include "prbit.h"
+#include "nsStaticAtom.h"
 
 #if defined(PR_LOGGING)
 PRLogModuleInfo *gHttpLog = nsnull;
@@ -60,6 +61,23 @@ enum {
 };
 #undef HTTP_ATOM
 
+// define all method atoms
+#define HTTP_METHOD_ATOM(name_, value_) nsIAtom* nsHttp::name_;
+#include "nsHttpAtomList.h"
+#undef HTTP_METHOD_ATOM
+
+#define HTTP_METHOD_ATOM(name_, value_) \
+    NS_STATIC_ATOM_BUFFER(name_##_buffer, value_)
+#include "nsHttpAtomList.h"
+#undef HTTP_METHOD_ATOM
+
+static const nsStaticAtom methodAtomsInfo[] = {
+#define HTTP_METHOD_ATOM(name_, value_) \
+    NS_STATIC_ATOM(name_##_buffer, &nsHttp::name_),
+#include "nsHttpAtomList.h"
+#undef HTTP_METHOD_ATOM
+};
+
 using namespace mozilla;
 
 // we keep a linked list of atoms allocated on the heap for easy clean up when
@@ -184,6 +202,12 @@ nsHttp::DestroyAtomTable()
     }
 }
 
+void
+nsHttp::CreateMethodAtoms()
+{
+    NS_RegisterStaticAtoms(methodAtomsInfo, NS_ARRAY_LENGTH(methodAtomsInfo));
+}
+
 // this function may be called from multiple threads
 nsHttpAtom
 nsHttp::ResolveAtom(const char *str)

netwerk/protocol/http/nsHttp.h

@@ -67,6 +67,7 @@
 #include "nsPromiseFlatString.h"
 #include "nsURLHelper.h"
 #include "netCore.h"
+#include "nsIAtom.h"
 
 #if defined(PR_LOGGING)
 //
@@ -164,6 +165,8 @@ struct nsHttp
     static nsresult CreateAtomTable();
     static void DestroyAtomTable();
 
+    static void CreateMethodAtoms();
+
     // will dynamically add atoms to the table if they don't already exist
     static nsHttpAtom ResolveAtom(const char *);
     static nsHttpAtom ResolveAtom(const nsACString &s)
@@ -215,6 +218,10 @@ struct nsHttp
 #define HTTP_ATOM(_name, _value) static nsHttpAtom _name;
 #include "nsHttpAtomList.h"
 #undef HTTP_ATOM
+
+#define HTTP_METHOD_ATOM(_name, _value) static nsIAtom* _name;
+#include "nsHttpAtomList.h"
+#undef HTTP_METHOD_ATOM
 };
 
 //-----------------------------------------------------------------------------

netwerk/protocol/http/nsHttpAtomList.h

@@ -50,6 +50,7 @@
   The second argument to HTTP_ATOM is the string value of the atom.
  ******/
 
+#if defined(HTTP_ATOM)
 HTTP_ATOM(Accept,                    "Accept")
 HTTP_ATOM(Accept_Encoding,           "Accept-Encoding")
 HTTP_ATOM(Accept_Language,           "Accept-Language")
@@ -123,26 +124,40 @@ HTTP_ATOM(Vary,                      "Vary")
 HTTP_ATOM(Version,                   "Version")
 HTTP_ATOM(WWW_Authenticate,          "WWW-Authenticate")
 HTTP_ATOM(Warning,                   "Warning")
+#endif
 
 // 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")
+#if defined(HTTP_METHOD_ATOM)
-HTTP_ATOM(Copy,                      "COPY")
+
-HTTP_ATOM(Delete,                    "DELETE")
+#if !defined(HTTP_CASE_INSENSITIVE_METHOD_ATOM)
-HTTP_ATOM(Get,                       "GET")
+#define HTTP_CASE_INSENSITIVE_METHOD_ATOM HTTP_METHOD_ATOM
-HTTP_ATOM(Head,                      "HEAD")
+#define UNDEF_HTTP_CASE_INSENSITIVE_METHOD_ATOM
-HTTP_ATOM(Index,                     "INDEX")
+#endif
-HTTP_ATOM(Lock,                      "LOCK")
+HTTP_CASE_INSENSITIVE_METHOD_ATOM(Connect,   "CONNECT")
-HTTP_ATOM(M_Post,                    "M-POST")
+HTTP_METHOD_ATOM                 (Copy,      "COPY")
-HTTP_ATOM(Mkcol,                     "MKCOL")
+HTTP_CASE_INSENSITIVE_METHOD_ATOM(Delete,    "DELETE")
-HTTP_ATOM(Move,                      "MOVE")
+HTTP_CASE_INSENSITIVE_METHOD_ATOM(Get,       "GET")
-HTTP_ATOM(Options,                   "OPTIONS")
+HTTP_CASE_INSENSITIVE_METHOD_ATOM(Head,      "HEAD")
-HTTP_ATOM(Post,                      "POST")
+HTTP_METHOD_ATOM                 (Index,     "INDEX")
-HTTP_ATOM(Propfind,                  "PROPFIND")
+HTTP_METHOD_ATOM                 (Lock,      "LOCK")
-HTTP_ATOM(Proppatch,                 "PROPPATCH")
+HTTP_METHOD_ATOM                 (M_Post,    "M-POST")
-HTTP_ATOM(Put,                       "PUT")
+HTTP_METHOD_ATOM                 (Mkcol,     "MKCOL")
-HTTP_ATOM(Trace,                     "TRACE")
+HTTP_METHOD_ATOM                 (Move,      "MOVE")
-HTTP_ATOM(Unlock,                    "UNLOCK")
+HTTP_CASE_INSENSITIVE_METHOD_ATOM(Options,   "OPTIONS")
+HTTP_CASE_INSENSITIVE_METHOD_ATOM(Post,      "POST")
+HTTP_METHOD_ATOM                 (Propfind,  "PROPFIND")
+HTTP_METHOD_ATOM                 (Proppatch, "PROPPATCH")
+HTTP_CASE_INSENSITIVE_METHOD_ATOM(Put,       "PUT")
+HTTP_CASE_INSENSITIVE_METHOD_ATOM(Trace,     "TRACE")
+HTTP_CASE_INSENSITIVE_METHOD_ATOM(Track,     "TRACK")
+HTTP_METHOD_ATOM                 (Unlock,    "UNLOCK")
+#if defined(UNDEF_HTTP_CASE_INSENSITIVE_METHOD_ATOM)
+#undef UNDEF_HTTP_CASE_INSENSITIVE_METHOD_ATOM
+#undef HTTP_CASE_INSENSITIVE_METHOD_ATOM
+#endif
+
+#endif

netwerk/protocol/http/nsHttpChannel.cpp

@@ -2412,7 +2412,9 @@ nsHttpChannel::CheckCache()
     rv = mCacheEntry->GetMetaDataElement("request-method", getter_Copies(buf));
     NS_ENSURE_SUCCESS(rv, rv);
 
-    nsHttpAtom method = nsHttp::ResolveAtom(buf);
+    nsCOMPtr<nsIAtom> method = do_GetAtom(buf);
+    NS_ENSURE_TRUE(method, NS_ERROR_OUT_OF_MEMORY);
+
     if (method == nsHttp::Head) {
         // The cached response does not contain an entity.  We can only reuse
         // the response if the current request is also HEAD.
@@ -2981,7 +2983,7 @@ nsHttpChannel::AddCacheEntryHeaders(nsICacheEntryDescriptor *entry)
     // Store the HTTP request method with the cache entry so we can distinguish
     // for example GET and HEAD responses.
     rv = entry->SetMetaDataElement("request-method",
-                                   mRequestHead.Method().get());
+                                   nsAtomCString(mRequestHead.Method()).get());
     if (NS_FAILED(rv)) return rv;
 
     // Store the HTTP authorization scheme used if any...

netwerk/protocol/http/nsHttpHandler.cpp

@@ -238,6 +238,8 @@ nsHttpHandler::Init()
     if (NS_FAILED(rv))
         return rv;
 
+    nsHttp::CreateMethodAtoms();
+
     mIOService = do_GetService(NS_IOSERVICE_CONTRACTID, &rv);
     if (NS_FAILED(rv)) {
         NS_WARNING("unable to continue without io service");

netwerk/protocol/http/nsHttpRequestHead.cpp

@@ -47,7 +47,7 @@ nsHttpRequestHead::Flatten(nsACString &buf, PRBool pruneProxyHeaders)
 {
     // note: the first append is intentional.
  
-    buf.Append(mMethod.get());
+    buf.Append(nsAtomCString(mMethod));
     buf.Append(' ');
     buf.Append(mRequestURI);
     buf.AppendLiteral(" HTTP/");

netwerk/protocol/http/nsHttpRequestHead.h

@@ -55,12 +55,12 @@ public:
     nsHttpRequestHead() : mMethod(nsHttp::Get), mVersion(NS_HTTP_VERSION_1_1) {}
     ~nsHttpRequestHead() {}
 
-    void SetMethod(nsHttpAtom method) { mMethod = method; }
+    void SetMethod(nsIAtom *method) { mMethod = method; }
     void SetVersion(nsHttpVersion version) { mVersion = version; }
     void SetRequestURI(const nsCSubstring &s) { mRequestURI = s; }
 
     nsHttpHeaderArray  &Headers()    { return mHeaders; }
-    nsHttpAtom          Method()     { return mMethod; }
+    nsIAtom            *Method()     { return mMethod; }
     nsHttpVersion       Version()    { return mVersion; }
     const nsCSubstring &RequestURI() { return mRequestURI; }
 
@@ -77,7 +77,7 @@ public:
 
 private:
     nsHttpHeaderArray mHeaders;
-    nsHttpAtom        mMethod;
+    nsCOMPtr<nsIAtom> mMethod;
     nsHttpVersion     mVersion;
     nsCString         mRequestURI;
 };

netwerk/test/unit/test_bug477578.js

@@ -0,0 +1,42 @@
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+
+const testMethods = [
+  ["get", "GET"],
+  ["post", "POST"],
+  ["head", "HEAD"],
+  ["put", "PUT"],
+  ["delete", "DELETE"],
+  ["connect", "CONNECT"],
+  ["options", "OPTIONS"],
+  ["trace", "TRACE"],
+  ["track", "TRACK"],
+  ["copy", "copy"],
+  ["index", "index"],
+  ["lock", "lock"],
+  ["m-post", "m-post"],
+  ["mkcol", "mkcol"],
+  ["move", "move"],
+  ["propfind", "propfind"],
+  ["proppatch", "proppatch"],
+  ["unlock", "unlock"],
+  ["link", "link"],
+  ["foo", "foo"],
+  ["foO", "foO"],
+  ["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][0];
+    do_check_eq(chan.requestMethod, testMethods[i][1]);
+  }
+}

netwerk/test/unit/xpcshell.ini

@@ -44,6 +44,7 @@ skip-if = os == "android"
 skip-if = os == "android"
 [test_bug468594.js]
 [test_bug470716.js]
+[test_bug477578.js]
 [test_bug479413.js]
 [test_bug479485.js]
 [test_bug482601.js]