bug 655389 - CRLF Injection and the parsing of HTTP headers. r=bz
authorJason Duell <jduell.mcbugs@gmail.com>
Mon, 04 Jul 2011 23:12:32 -0700
changeset 72833 cc405052ac0a1b6a4cab8a8d430fa90349f987ec
parent 72832 a733e437f95aac6980f5307eb5fc8782066217e9
child 72834 4aaf3b77a6f3df4799d427fcfb905fdd591dd2b8
push idunknown
push userunknown
push dateunknown
reviewersbz
bugs655389
milestone7.0a1
bug 655389 - CRLF Injection and the parsing of HTTP headers. r=bz
netwerk/protocol/http/HttpChannelParent.cpp
netwerk/protocol/http/HttpChannelParent.h
netwerk/protocol/http/PHttpChannelParams.h
netwerk/protocol/http/nsHttpChunkedDecoder.cpp
netwerk/protocol/http/nsHttpHeaderArray.cpp
netwerk/protocol/http/nsHttpHeaderArray.h
netwerk/protocol/http/nsHttpRequestHead.h
netwerk/protocol/http/nsHttpResponseHead.cpp
netwerk/protocol/http/nsHttpResponseHead.h
--- a/netwerk/protocol/http/HttpChannelParent.cpp
+++ b/netwerk/protocol/http/HttpChannelParent.cpp
@@ -61,16 +61,17 @@
 namespace mozilla {
 namespace net {
 
 HttpChannelParent::HttpChannelParent(PBrowserParent* iframeEmbedding)
   : mIPCClosed(false)
   , mStoredStatus(0)
   , mStoredProgress(0)
   , mStoredProgressMax(0)
+  , mHeadersToSyncToChild(nsnull)
 {
   // Ensure gHttpHandler is initialized: we need the atom table up and running.
   nsIHttpProtocolHandler* handler;
   CallGetService(NS_NETWORK_PROTOCOL_CONTRACTID_PREFIX "http", &handler);
   NS_ASSERTION(handler, "no http handler");
 
   mTabParent = do_QueryInterface(static_cast<nsITabParent*>(
       static_cast<TabParent*>(iframeEmbedding)));
@@ -88,23 +89,24 @@ HttpChannelParent::ActorDestroy(ActorDes
   // yet, but we must not send any more msgs to child.
   mIPCClosed = true;
 }
 
 //-----------------------------------------------------------------------------
 // HttpChannelParent::nsISupports
 //-----------------------------------------------------------------------------
 
-NS_IMPL_ISUPPORTS6(HttpChannelParent,
+NS_IMPL_ISUPPORTS7(HttpChannelParent,
                    nsIInterfaceRequestor,
                    nsIProgressEventSink,
                    nsIRequestObserver,
                    nsIStreamListener,
                    nsIParentChannel,
-                   nsIParentRedirectingChannel)
+                   nsIParentRedirectingChannel,
+                   nsIHttpHeaderVisitor)
 
 //-----------------------------------------------------------------------------
 // HttpChannelParent::nsIInterfaceRequestor
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 HttpChannelParent::GetInterface(const nsIID& aIID, void **result)
 {
@@ -420,25 +422,21 @@ HttpChannelParent::OnStartRequest(nsIReq
   nsCOMPtr<nsISupports> secInfoSupp;
   chan->GetSecurityInfo(getter_AddRefs(secInfoSupp));
   if (secInfoSupp) {
     nsCOMPtr<nsISerializable> secInfoSer = do_QueryInterface(secInfoSupp);
     if (secInfoSer)
       NS_SerializeToString(secInfoSer, secInfoSerialization);
   }
 
+  // sync request headers to child, in case they've changed
   RequestHeaderTuples headers;
-  nsHttpHeaderArray harray = requestHead->Headers();
-
-  for (PRUint32 i = 0; i < harray.Count(); i++) {
-    RequestHeaderTuple* tuple = headers.AppendElement();
-    tuple->mHeader = harray.Headers()[i].header;
-    tuple->mValue  = harray.Headers()[i].value;
-    tuple->mMerge  = false;
-  }
+  mHeadersToSyncToChild = &headers;
+  requestHead->Headers().VisitHeaders(this);
+  mHeadersToSyncToChild = 0;
 
   nsHttpChannel *httpChan = static_cast<nsHttpChannel *>(mChannel.get());
   if (mIPCClosed || 
       !SendOnStartRequest(responseHead ? *responseHead : nsHttpResponseHead(), 
                           !!responseHead,
                           headers,
                           isFromCache,
                           mCacheDescriptor ? PR_TRUE : PR_FALSE,
@@ -593,9 +591,26 @@ HttpChannelParent::CompleteRedirect(PRBo
     // TODO: check return value: assume child dead if failed
     unused << SendRedirect3Complete();
   }
 
   mRedirectChannel = nsnull;
   return NS_OK;
 }
 
+//-----------------------------------------------------------------------------
+// HttpChannelParent::nsIHttpHeaderVisitor
+//-----------------------------------------------------------------------------
+
+nsresult
+HttpChannelParent::VisitHeader(const nsACString &header, const nsACString &value)
+{
+  // Will be set unless some random code QI's us to nsIHttpHeaderVisitor
+  NS_ENSURE_STATE(mHeadersToSyncToChild);
+
+  RequestHeaderTuple* tuple = mHeadersToSyncToChild->AppendElement();
+  tuple->mHeader = header;
+  tuple->mValue  = value;
+  tuple->mMerge  = false;  // headers already merged:
+  return NS_OK;
+}
+
 }} // mozilla::net
--- a/netwerk/protocol/http/HttpChannelParent.h
+++ b/netwerk/protocol/http/HttpChannelParent.h
@@ -58,25 +58,27 @@ namespace mozilla {
 namespace net {
 
 class HttpChannelParentListener;
 
 class HttpChannelParent : public PHttpChannelParent
                         , public nsIParentRedirectingChannel
                         , public nsIProgressEventSink
                         , public nsIInterfaceRequestor
+                        , public nsIHttpHeaderVisitor
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSIPARENTCHANNEL
   NS_DECL_NSIPARENTREDIRECTINGCHANNEL
   NS_DECL_NSIPROGRESSEVENTSINK
   NS_DECL_NSIINTERFACEREQUESTOR
+  NS_DECL_NSIHTTPHEADERVISITOR
 
   HttpChannelParent(PBrowserParent* iframeEmbedding);
   virtual ~HttpChannelParent();
 
 protected:
   virtual bool RecvAsyncOpen(const IPC::URI&            uri,
                              const IPC::URI&            originalUri,
                              const IPC::URI&            docUri,
@@ -125,14 +127,17 @@ private:
   nsCOMPtr<nsIChannel> mRedirectChannel;
   nsCOMPtr<nsIAsyncVerifyRedirectCallback> mRedirectCallback;
 
   // state for combining OnStatus/OnProgress with OnDataAvailable
   // into one IPDL call to child.
   nsresult mStoredStatus;
   PRUint64 mStoredProgress;
   PRUint64 mStoredProgressMax;
+
+  // used while visiting headers, to send them to child: else null
+  RequestHeaderTuples *mHeadersToSyncToChild;
 };
 
 } // namespace net
 } // namespace mozilla
 
 #endif // mozilla_net_HttpChannelParent_h
--- a/netwerk/protocol/http/PHttpChannelParams.h
+++ b/netwerk/protocol/http/PHttpChannelParams.h
@@ -78,19 +78,19 @@ struct ParamTraits<mozilla::net::Request
   {
     WriteParam(aMsg, aParam.mHeader);
     WriteParam(aMsg, aParam.mValue);
     WriteParam(aMsg, aParam.mMerge);
   }
 
   static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
   {
-    if (!ReadParam(aMsg, aIter, &(aResult->mHeader)) ||
-        !ReadParam(aMsg, aIter, &(aResult->mValue))  ||
-        !ReadParam(aMsg, aIter, &(aResult->mMerge)))
+    if (!ReadParam(aMsg, aIter, &aResult->mHeader) ||
+        !ReadParam(aMsg, aIter, &aResult->mValue)  ||
+        !ReadParam(aMsg, aIter, &aResult->mMerge))
       return false;
 
     return true;
   }
 };
 
 template<>
 struct ParamTraits<nsHttpAtom>
@@ -125,39 +125,39 @@ struct ParamTraits<nsHttpHeaderArray::ns
   static void Write(Message* aMsg, const paramType& aParam)
   {
     WriteParam(aMsg, aParam.header);
     WriteParam(aMsg, aParam.value);
   }
 
   static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
   {
-    if (!ReadParam(aMsg, aIter, &(aResult->header)) ||
-        !ReadParam(aMsg, aIter, &(aResult->value)))
+    if (!ReadParam(aMsg, aIter, &aResult->header) ||
+        !ReadParam(aMsg, aIter, &aResult->value))
       return false;
 
     return true;
   }
 };
 
 template<>
 struct ParamTraits<nsHttpHeaderArray>
 {
   typedef nsHttpHeaderArray paramType;
 
   static void Write(Message* aMsg, const paramType& aParam)
   {
     paramType& p = const_cast<paramType&>(aParam);
 
-    WriteParam(aMsg, p.Headers());
+    WriteParam(aMsg, p.mHeaders);
   }
 
   static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
   {
-    if (!ReadParam(aMsg, aIter, &(aResult->Headers())))
+    if (!ReadParam(aMsg, aIter, &aResult->mHeaders))
       return false;
 
     return true;
   }
 };
 
 template<>
 struct ParamTraits<nsHttpResponseHead>
@@ -175,26 +175,26 @@ struct ParamTraits<nsHttpResponseHead>
     WriteParam(aMsg, aParam.mContentCharset);
     WriteParam(aMsg, aParam.mCacheControlNoStore);
     WriteParam(aMsg, aParam.mCacheControlNoCache);
     WriteParam(aMsg, aParam.mPragmaNoCache);
   }
 
   static bool Read(const Message* aMsg, void** aIter, paramType* aResult)
   {
-    if (!ReadParam(aMsg, aIter, &(aResult->mHeaders))             ||
-        !ReadParam(aMsg, aIter, &(aResult->mVersion))             ||
-        !ReadParam(aMsg, aIter, &(aResult->mStatus))              ||
-        !ReadParam(aMsg, aIter, &(aResult->mStatusText))          ||
-        !ReadParam(aMsg, aIter, &(aResult->mContentLength))       ||
-        !ReadParam(aMsg, aIter, &(aResult->mContentType))         ||
-        !ReadParam(aMsg, aIter, &(aResult->mContentCharset))      ||
-        !ReadParam(aMsg, aIter, &(aResult->mCacheControlNoStore)) ||
-        !ReadParam(aMsg, aIter, &(aResult->mCacheControlNoCache)) ||
-        !ReadParam(aMsg, aIter, &(aResult->mPragmaNoCache)))
+    if (!ReadParam(aMsg, aIter, &aResult->mHeaders)             ||
+        !ReadParam(aMsg, aIter, &aResult->mVersion)             ||
+        !ReadParam(aMsg, aIter, &aResult->mStatus)              ||
+        !ReadParam(aMsg, aIter, &aResult->mStatusText)          ||
+        !ReadParam(aMsg, aIter, &aResult->mContentLength)       ||
+        !ReadParam(aMsg, aIter, &aResult->mContentType)         ||
+        !ReadParam(aMsg, aIter, &aResult->mContentCharset)      ||
+        !ReadParam(aMsg, aIter, &aResult->mCacheControlNoStore) ||
+        !ReadParam(aMsg, aIter, &aResult->mCacheControlNoCache) ||
+        !ReadParam(aMsg, aIter, &aResult->mPragmaNoCache))
       return false;
 
     return true;
   }
 };
 
 } // namespace IPC
 
--- a/netwerk/protocol/http/nsHttpChunkedDecoder.cpp
+++ b/netwerk/protocol/http/nsHttpChunkedDecoder.cpp
@@ -132,20 +132,17 @@ nsHttpChunkedDecoder::ParseChunkRemainin
             buf = (char *) mLineBuf.get();
         }
 
         if (mWaitEOF) {
             if (*buf) {
                 LOG(("got trailer: %s\n", buf));
                 // allocate a header array for the trailers on demand
                 if (!mTrailers) {
-                    mTrailers = new nsHttpHeaderArray
-                        (nsHttpHeaderArray::HTTP_RESPONSE_HEADERS);
-                    if (!mTrailers)
-                        return NS_ERROR_OUT_OF_MEMORY;
+                    mTrailers = new nsHttpHeaderArray();
                 }
                 mTrailers->ParseHeaderLine(buf);
             }
             else {
                 mWaitEOF = PR_FALSE;
                 mReachedEOF = PR_TRUE;
                 LOG(("reached end of chunked-body\n"));
             }
--- a/netwerk/protocol/http/nsHttpHeaderArray.cpp
+++ b/netwerk/protocol/http/nsHttpHeaderArray.cpp
@@ -17,16 +17,18 @@
  *
  * The Initial Developer of the Original Code is
  * Netscape Communications.
  * Portions created by the Initial Developer are Copyright (C) 2001
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
  *   Darin Fisher <darin@netscape.com> (original author)
+ *   Patrick McManus <mcmanus@ducksong.com>
+ *   Jason Duell <jduell.mcbugs@gmail.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
  * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
  * use your version of this file under the terms of the MPL, indicate your
@@ -38,17 +40,16 @@
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsHttpHeaderArray.h"
 #include "nsHttp.h"
 
 //-----------------------------------------------------------------------------
 // nsHttpHeaderArray <public>
 //-----------------------------------------------------------------------------
-
 nsresult
 nsHttpHeaderArray::SetHeader(nsHttpAtom header,
                              const nsACString &value,
                              PRBool merge)
 {
     nsEntry *entry = nsnull;
     PRInt32 index;
 
@@ -57,43 +58,60 @@ nsHttpHeaderArray::SetHeader(nsHttpAtom 
     // If an empty value is passed in, then delete the header entry...
     // unless we are merging, in which case this function becomes a NOP.
     if (value.IsEmpty()) {
         if (!merge && entry)
             mHeaders.RemoveElementAt(index);
         return NS_OK;
     }
 
-    // Create a new entry, or...
     if (!entry) {
+        entry = mHeaders.AppendElement(); // new nsEntry()
+        if (!entry)
+            return NS_ERROR_OUT_OF_MEMORY;
+        entry->header = header;
+        entry->value = value;
+    } else if (merge && !IsSingletonHeader(header)) {
+        MergeHeader(header, entry, value);
+    } else {
+        // Replace the existing string with the new value
+        entry->value = value;
+    }
+
+    return NS_OK;
+}
+
+nsresult
+nsHttpHeaderArray::SetHeaderFromNet(nsHttpAtom header, const nsACString &value)
+{
+    nsEntry *entry = nsnull;
+    PRInt32 index;
+
+    index = LookupEntry(header, &entry);
+
+    if (!entry) {
+        if (value.IsEmpty())
+            return NS_OK; // ignore empty headers
         entry = mHeaders.AppendElement(); //new nsEntry(header, value);
         if (!entry)
             return NS_ERROR_OUT_OF_MEMORY;
         entry->header = header;
         entry->value = value;
+    } else if (!IsSingletonHeader(header)) {
+        MergeHeader(header, entry, value);
+    } else {
+        // Multiple instances of non-mergeable header received from network
+        // - ignore if same value
+        if (!entry->value.Equals(value)) {
+            if (IsSuspectDuplicateHeader(header)) {
+                // reply may be corrupt/hacked (ex: CLRF injection attacks)
+                return NS_ERROR_CORRUPTED_CONTENT;
+            } // else silently drop value: keep value from 1st header seen
+        }
     }
-    // Append the new value to the existing value iff...
-    else if (merge && CanAppendToHeader(header)) {
-        if (header == nsHttp::Set_Cookie ||
-            header == nsHttp::WWW_Authenticate ||
-            header == nsHttp::Proxy_Authenticate)
-            // Special case these headers and use a newline delimiter to
-            // delimit the values from one another as commas may appear
-            // in the values of these headers contrary to what the spec says.
-            entry->value.Append('\n');
-        else
-            // Delimit each value from the others using a comma (per HTTP spec)
-            entry->value.AppendLiteral(", ");
-        entry->value.Append(value);
-    }
-    // Replace the existing string with the new value
-    else if (CanOverwriteHeader(header))
-        entry->value = value;
-    else if (!entry->value.Equals(value))
-        return NS_ERROR_CORRUPTED_CONTENT;
 
     return NS_OK;
 }
 
 void
 nsHttpHeaderArray::ClearHeader(nsHttpAtom header)
 {
     mHeaders.RemoveElement(header, nsEntry::MatchHeader());
@@ -181,17 +199,17 @@ nsHttpHeaderArray::ParseHeaderLine(const
                // consisted of LWS, then p2 would have pointed at |p-1|, so
                // the prefix increment is always valid.
 
     // assign return values
     if (hdr) *hdr = atom;
     if (val) *val = p;
 
     // assign response header
-    return SetHeader(atom, nsDependentCString(p, p2 - p), PR_TRUE);
+    return SetHeaderFromNet(atom, nsDependentCString(p, p2 - p));
 }
 
 void
 nsHttpHeaderArray::Flatten(nsACString &buf, PRBool pruneProxyHeaders)
 {
     PRUint32 i, count = mHeaders.Length();
     for (i = 0; i < count; ++i) {
         const nsEntry &entry = mHeaders[i];
@@ -215,46 +233,8 @@ nsHttpHeaderArray::PeekHeaderAt(PRUint32
     return entry.value.get();
 }
 
 void
 nsHttpHeaderArray::Clear()
 {
     mHeaders.Clear();
 }
-
-//-----------------------------------------------------------------------------
-// nsHttpHeaderArray <private>
-//-----------------------------------------------------------------------------
-
-PRInt32
-nsHttpHeaderArray::LookupEntry(nsHttpAtom header, nsEntry **entry)
-{
-    PRUint32 index = mHeaders.IndexOf(header, 0, nsEntry::MatchHeader());
-    if (index != PR_UINT32_MAX)
-      *entry = &mHeaders[index];
-    return index;
-}
-
-PRBool
-nsHttpHeaderArray::CanAppendToHeader(nsHttpAtom header)
-{
-    return header != nsHttp::Content_Type        &&
-           header != nsHttp::Content_Length      &&
-           header != nsHttp::User_Agent          &&
-           header != nsHttp::Referer             &&
-           header != nsHttp::Host                &&
-           header != nsHttp::Authorization       &&
-           header != nsHttp::Proxy_Authorization &&
-           header != nsHttp::If_Modified_Since   &&
-           header != nsHttp::If_Unmodified_Since &&
-           header != nsHttp::From                &&
-           header != nsHttp::Location            &&
-           header != nsHttp::Max_Forwards;
-}
-
-PRBool
-nsHttpHeaderArray::CanOverwriteHeader(nsHttpAtom header)
-{
-    if (mType != HTTP_RESPONSE_HEADERS)
-        return PR_TRUE;
-    return header != nsHttp::Content_Length;
-}
--- a/netwerk/protocol/http/nsHttpHeaderArray.h
+++ b/netwerk/protocol/http/nsHttpHeaderArray.h
@@ -1,9 +1,10 @@
 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/* vim: set sw=4 ts=8 et tw=80 : */
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
  *
@@ -44,27 +45,29 @@
 #include "nsIHttpChannel.h"
 #include "nsIHttpHeaderVisitor.h"
 #include "nsCOMPtr.h"
 #include "nsString.h"
 
 class nsHttpHeaderArray
 {
 public:
-    enum nsHttpHeaderType {
-        HTTP_REQUEST_HEADERS,
-        HTTP_RESPONSE_HEADERS
-    };
-
-    nsHttpHeaderArray(nsHttpHeaderType headerType) : mType(headerType) {}
+    nsHttpHeaderArray() {}
    ~nsHttpHeaderArray() { Clear(); }
 
     const char *PeekHeader(nsHttpAtom header);
 
-    nsresult SetHeader(nsHttpAtom header, const nsACString &value, PRBool merge = PR_FALSE);
+    // Used by internal setters: to set header from network use SetHeaderFromNet
+    nsresult SetHeader(nsHttpAtom header, const nsACString &value,
+                       PRBool merge = PR_FALSE);
+
+    // Merges supported headers. For other duplicate values, determines if error
+    // needs to be thrown or 1st value kept.
+    nsresult SetHeaderFromNet(nsHttpAtom header, const nsACString &value);
+
     nsresult GetHeader(nsHttpAtom header, nsACString &value);
     void     ClearHeader(nsHttpAtom h);
 
     // Find the location of the given header value, or null if none exists.
     const char *FindHeaderValue(nsHttpAtom header, const char *value) {
         return nsHttp::FindToken(PeekHeader(header), value,
                                  HTTP_HEADER_VALUE_SEPS);
     }
@@ -99,20 +102,95 @@ public:
 
         struct MatchHeader {
           PRBool Equals(const nsEntry &entry, const nsHttpAtom &header) const {
             return entry.header == header;
           }
         };
     };
 
-    nsTArray<nsEntry> &Headers() { return mHeaders; }
-
 private:
     PRInt32 LookupEntry(nsHttpAtom header, nsEntry **);
-    PRBool  CanAppendToHeader(nsHttpAtom header);
-    PRBool  CanOverwriteHeader(nsHttpAtom header);
+    void MergeHeader(nsHttpAtom header, nsEntry *entry, const nsACString &value);
+
+    // Header cannot be merged: only one value possible
+    PRBool  IsSingletonHeader(nsHttpAtom header);
+
+    // Subset of singleton headers: should never see multiple, different
+    // instances of these, else something fishy may be going on (like CLRF
+    // injection)
+    PRBool  IsSuspectDuplicateHeader(nsHttpAtom header);
 
     nsTArray<nsEntry> mHeaders;
-    nsHttpHeaderType  mType;
+
+    friend struct IPC::ParamTraits<nsHttpHeaderArray>;
 };
 
+
+//-----------------------------------------------------------------------------
+// nsHttpHeaderArray <private>: inline functions
+//-----------------------------------------------------------------------------
+
+inline PRInt32
+nsHttpHeaderArray::LookupEntry(nsHttpAtom header, nsEntry **entry)
+{
+    PRUint32 index = mHeaders.IndexOf(header, 0, nsEntry::MatchHeader());
+    if (index != PR_UINT32_MAX)
+        *entry = &mHeaders[index];
+    return index;
+}
+
+inline PRBool
+nsHttpHeaderArray::IsSingletonHeader(nsHttpAtom header)
+{
+    return header == nsHttp::Content_Type        ||
+           header == nsHttp::Content_Disposition ||
+           header == nsHttp::Content_Length      ||
+           header == nsHttp::User_Agent          ||
+           header == nsHttp::Referer             ||
+           header == nsHttp::Host                ||
+           header == nsHttp::Authorization       ||
+           header == nsHttp::Proxy_Authorization ||
+           header == nsHttp::If_Modified_Since   ||
+           header == nsHttp::If_Unmodified_Since ||
+           header == nsHttp::From                ||
+           header == nsHttp::Location            ||
+           header == nsHttp::Max_Forwards;
+}
+
+inline void
+nsHttpHeaderArray::MergeHeader(nsHttpAtom header,
+                               nsEntry *entry,
+                               const nsACString &value)
+{
+    if (value.IsEmpty())
+        return;   // merge of empty header = no-op
+
+    // Append the new value to the existing value
+    if (header == nsHttp::Set_Cookie ||
+        header == nsHttp::WWW_Authenticate ||
+        header == nsHttp::Proxy_Authenticate)
+    {
+        // Special case these headers and use a newline delimiter to
+        // delimit the values from one another as commas may appear
+        // in the values of these headers contrary to what the spec says.
+        entry->value.Append('\n');
+    } else {
+        // Delimit each value from the others using a comma (per HTTP spec)
+        entry->value.AppendLiteral(", ");
+    }
+    entry->value.Append(value);
+}
+
+inline PRBool
+nsHttpHeaderArray::IsSuspectDuplicateHeader(nsHttpAtom header)
+{
+    PRBool retval =  header == nsHttp::Content_Length         ||
+                     header == nsHttp::Content_Disposition    ||
+                     header == nsHttp::Location;
+
+    NS_ASSERTION(!retval || IsSingletonHeader(header),
+                 "Only non-mergeable headers should be in this list\n");
+
+    return retval;
+}
+
 #endif
--- a/netwerk/protocol/http/nsHttpRequestHead.h
+++ b/netwerk/protocol/http/nsHttpRequestHead.h
@@ -47,19 +47,17 @@
 //-----------------------------------------------------------------------------
 // nsHttpRequestHead represents the request line and headers from an HTTP
 // request.
 //-----------------------------------------------------------------------------
 
 class nsHttpRequestHead
 {
 public:
-    nsHttpRequestHead() : mHeaders(nsHttpHeaderArray::HTTP_REQUEST_HEADERS)
-                        , mMethod(nsHttp::Get)
-                        , mVersion(NS_HTTP_VERSION_1_1) {}
+    nsHttpRequestHead() : mMethod(nsHttp::Get), mVersion(NS_HTTP_VERSION_1_1) {}
     ~nsHttpRequestHead() {}
 
     void SetMethod(nsHttpAtom method) { mMethod = method; }
     void SetVersion(nsHttpVersion version) { mVersion = version; }
     void SetRequestURI(const nsCSubstring &s) { mRequestURI = s; }
 
     nsHttpHeaderArray  &Headers()    { return mHeaders; }
     nsHttpAtom          Method()     { return mMethod; }
--- a/netwerk/protocol/http/nsHttpResponseHead.cpp
+++ b/netwerk/protocol/http/nsHttpResponseHead.cpp
@@ -440,17 +440,16 @@ nsHttpResponseHead::UpdateHeaders(nsHttp
     LOG(("nsHttpResponseHead::UpdateHeaders [this=%x]\n", this));
 
     PRUint32 i, count = headers.Count();
     for (i=0; i<count; ++i) {
         nsHttpAtom header;
         const char *val = headers.PeekHeaderAt(i, header);
 
         if (!val) {
-            NS_NOTREACHED("null header value");
             continue;
         }
 
         // Ignore any hop-by-hop headers...
         if (header == nsHttp::Connection          ||
             header == nsHttp::Proxy_Connection    ||
             header == nsHttp::Keep_Alive          ||
             header == nsHttp::Proxy_Authenticate  ||
--- a/netwerk/protocol/http/nsHttpResponseHead.h
+++ b/netwerk/protocol/http/nsHttpResponseHead.h
@@ -46,18 +46,17 @@
 //-----------------------------------------------------------------------------
 // nsHttpResponseHead represents the status line and headers from an HTTP
 // response.
 //-----------------------------------------------------------------------------
 
 class nsHttpResponseHead
 {
 public:
-    nsHttpResponseHead() : mHeaders(nsHttpHeaderArray::HTTP_RESPONSE_HEADERS)
-                         , mVersion(NS_HTTP_VERSION_1_1)
+    nsHttpResponseHead() : mVersion(NS_HTTP_VERSION_1_1)
                          , mStatus(200)
                          , mContentLength(LL_MAXUINT)
                          , mCacheControlNoStore(PR_FALSE)
                          , mCacheControlNoCache(PR_FALSE)
                          , mPragmaNoCache(PR_FALSE) {}
     ~nsHttpResponseHead() 
     {
         Reset();