Fix invalid headers of multiple cookie and set-cookie (#2577)

* Fix invalid headers of multiple cookie and set-cookie * Support MultiFlatMap * Remove unused header
apache · Jun 3, 2024 · 4f85335 · 4f85335
1 parent 122355a
commit 4f85335
Show file tree

Hide file tree

Showing 10 changed files with 713 additions and 172 deletions.
diff --git a/src/brpc/details/http_message.cpp b/src/brpc/details/http_message.cpp
@@ -99,10 +99,17 @@ int HttpMessage::on_header_value(http_parser *parser,
             LOG(ERROR) << "Header name is empty";
             return -1;
         }
-        http_message->_cur_value =
-            &http_message->header().GetOrAddHeader(http_message->_cur_header);
+        HttpHeader& header = http_message->header();
+        if (header.CanFoldedInLine(http_message->_cur_header)) {
+            http_message->_cur_value =
+                &header.GetOrAddHeader(http_message->_cur_header);
+        } else {
+            http_message->_cur_value =
+                &header.AddHeader(http_message->_cur_header);
+        }
         if (http_message->_cur_value && !http_message->_cur_value->empty()) {
-            http_message->_cur_value->push_back(',');
+            http_message->_cur_value->append(
+                header.HeaderValueDelimiter(http_message->_cur_header));
         }
     }
     if (http_message->_cur_value) {

diff --git a/src/brpc/details/http_message.h b/src/brpc/details/http_message.h
@@ -68,8 +68,8 @@ class HttpMessage {
 
     HttpMethod request_method() const { return _request_method; }
 
-    HttpHeader &header() { return _header; }
-    const HttpHeader &header() const { return _header; }
+    HttpHeader& header() { return _header; }
+    const HttpHeader& header() const { return _header; }
     size_t parsed_length() const { return _parsed_length; }
 
     // Http parser callback functions

diff --git a/src/brpc/http_header.cpp b/src/brpc/http_header.cpp
@@ -22,33 +22,19 @@
 
 namespace brpc {
 
+const char* HttpHeader::SET_COOKIE = "set-cookie";
+const char* HttpHeader::COOKIE = "cookie";
+const char* HttpHeader::CONTENT_TYPE = "content-type";
+
 HttpHeader::HttpHeader() 
     : _status_code(HTTP_STATUS_OK)
     , _method(HTTP_METHOD_GET)
-    , _version(1, 1) {
+    , _version(1, 1)
+    , _first_set_cookie(NULL) {
+    CHECK_EQ(0, _headers.init(29));
     // NOTE: don't forget to clear the field in Clear() as well.
 }
 
-void HttpHeader::RemoveHeader(const char* key) {
-    if (IsContentType(key)) {
-        _content_type.clear();
-    } else {
-        _headers.erase(key);
-    }
-}
-
-void HttpHeader::AppendHeader(const std::string& key,
-                              const butil::StringPiece& value) {
-    std::string& slot = GetOrAddHeader(key);
-    if (slot.empty()) {
-        slot.assign(value.data(), value.size());
-    } else {
-        slot.reserve(slot.size() + 1 + value.size());
-        slot.push_back(',');
-        slot.append(value.data(), value.size());
-    }
-}
-
 void HttpHeader::Swap(HttpHeader &rhs) {
     _headers.swap(rhs._headers);
     _uri.Swap(rhs._uri);
@@ -69,6 +55,67 @@ void HttpHeader::Clear() {
     _version = std::make_pair(1, 1);
 }
 
+const std::string* HttpHeader::GetHeader(const char* key) const {
+    return GetHeader(std::string(key));
+}
+
+const std::string* HttpHeader::GetHeader(const std::string& key) const {
+    if (IsSetCookie(key)) {
+        return _first_set_cookie;
+    }
+    std::string* val = _headers.seek(key);
+    return val;
+}
+
+std::vector<const std::string*> HttpHeader::GetAllSetCookieHeader() const {
+    return GetMultiLineHeaders(SET_COOKIE);
+}
+
+std::vector<const std::string*>
+HttpHeader::GetMultiLineHeaders(const std::string& key) const {
+    std::vector<const std::string*> headers;
+    for (const auto& iter : _headers) {
+        if (_header_key_equal(iter.first, key)) {
+            headers.push_back(&iter.second);
+        }
+    }
+    return headers;
+}
+
+void HttpHeader::SetHeader(const std::string& key,
+                           const std::string& value) {
+    GetOrAddHeader(key) = value;
+}
+
+void HttpHeader::RemoveHeader(const char* key) {
+    if (IsContentType(key)) {
+        _content_type.clear();
+    } else {
+        _headers.erase(key);
+        if (IsSetCookie(key)) {
+            _first_set_cookie = NULL;
+        }
+    }
+}
+
+void HttpHeader::AppendHeader(const std::string& key,
+    const butil::StringPiece& value) {
+    if (!CanFoldedInLine(key)) {
+        // Add a new Set-Cookie header field.
+        std::string& slot = AddHeader(key);
+        slot.assign(value.data(), value.size());
+    } else {
+        std::string& slot = GetOrAddHeader(key);
+        if (slot.empty()) {
+            slot.assign(value.data(), value.size());
+        } else {
+            slot.reserve(slot.size() + 1 + value.size());
+            slot.append(HeaderValueDelimiter(key));
+            slot.append(value.data(), value.size());
+        }
+    }
+}
+
 const char* HttpHeader::reason_phrase() const {
     return HttpReasonPhrase(_status_code);
 }
@@ -82,10 +129,28 @@ std::string& HttpHeader::GetOrAddHeader(const std::string& key) {
         return _content_type;
     }
 
-    if (!_headers.initialized()) {
-        _headers.init(29);
+    bool is_set_cookie = IsSetCookie(key);
+    // Only returns the first Set-Cookie header field for compatibility.
+    if (is_set_cookie && NULL != _first_set_cookie) {
+        return *_first_set_cookie;
+    }
+
+    std::string* val = _headers.seek(key);
+    if (NULL == val) {
+        val = _headers.insert({ key, "" });
+        if (is_set_cookie) {
+            _first_set_cookie = val;
+        }
+    }
+    return *val;
+}
+
+std::string& HttpHeader::AddHeader(const std::string& key) {
+    std::string* val = _headers.insert({ key, "" });
+    if (IsSetCookie(key) && NULL == _first_set_cookie) {
+        _first_set_cookie = val;
     }
-    return _headers[key];
+    return *val;
 }
 
 const HttpHeader& DefaultHttpHeader() {

diff --git a/src/brpc/http_header.h b/src/brpc/http_header.h
@@ -19,6 +19,7 @@
 #ifndef  BRPC_HTTP_HEADER_H
 #define  BRPC_HTTP_HEADER_H
 
+#include <vector>
 #include "butil/strings/string_piece.h"  // StringPiece
 #include "butil/containers/case_ignored_flat_map.h"
 #include "brpc/uri.h"              // URI
@@ -39,7 +40,7 @@ class H2StreamContext;
 // Non-body part of a HTTP message.
 class HttpHeader {
 public:
-    typedef butil::CaseIgnoredFlatMap<std::string> HeaderMap;
+    typedef butil::CaseIgnoredMultiFlatMap<std::string> HeaderMap;
     typedef HeaderMap::const_iterator HeaderIterator;
     typedef HeaderMap::key_equal HeaderKeyEqual;
 
@@ -80,24 +81,39 @@ class HttpHeader {
     // Return pointer to the value, NULL on not found.
     // NOTE: If the key is "Content-Type", `GetHeader("Content-Type")'
     // (case-insensitive) is equal to `content_type()'.
-    const std::string* GetHeader(const char* key) const
-    { return _headers.seek(key); }
-    const std::string* GetHeader(const std::string& key) const
-    { return _headers.seek(key); }
+    const std::string* GetHeader(const char* key) const;
+    const std::string* GetHeader(const std::string& key) const;
+
+    std::vector<const std::string*> GetAllSetCookieHeader() const;
 
     // Set value of a header.
     // NOTE: If the key is "Content-Type", `SetHeader("Content-Type", ...)'
     // (case-insensitive) is equal to `set_content_type(...)'.
-    void SetHeader(const std::string& key, const std::string& value)
-    { GetOrAddHeader(key) = value; }
+    void SetHeader(const std::string& key, const std::string& value);
 
-    // Remove a header.
+    // Remove all headers of key.
     void RemoveHeader(const char* key);
     void RemoveHeader(const std::string& key) { RemoveHeader(key.c_str()); }
 
     // Append value to a header. If the header already exists, separate
-    // old value and new value with comma(,) according to:
-    //   https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
+    // old value and new value with comma(,), two-byte delimiter of "; "
+    // or into a new header field, according to:
+    //
+    // https://datatracker.ietf.org/doc/html/rfc2616#section-4.2
+    // Multiple message-header fields with the same field-name MAY be
+    // present in a message if and only if the entire field-value for that
+    // header field is defined as a comma-separated list [i.e., #(values)].
+    //
+    // https://datatracker.ietf.org/doc/html/rfc9114#section-4.2.1
+    // If a decompressed field section contains multiple cookie field lines,
+    // these MUST be concatenated into a single byte string using the two-byte
+    // delimiter of "; " (ASCII 0x3b, 0x20) before being passed into a context
+    // other than HTTP/2 or HTTP/3, such as an HTTP/1.1 connection, or a generic
+    // HTTP server application.
+    //
+    // https://datatracker.ietf.org/doc/html/rfc6265#section-3
+    // Origin servers SHOULD NOT fold multiple Set-Cookie header
+    // fields into a single header field.
     void AppendHeader(const std::string& key, const butil::StringPiece& value);
 
     // Get header iterators which are invalidated after calling AppendHeader()
@@ -145,19 +161,48 @@ friend class HttpMessageSerializer;
 friend class policy::H2StreamContext;
 friend void policy::ProcessHttpRequest(InputMessageBase *msg);
 
+    static const char* SET_COOKIE;
+    static const char* COOKIE;
+    static const char* CONTENT_TYPE;
+
+    std::vector<const std::string*> GetMultiLineHeaders(const std::string& key) const;
+
     std::string& GetOrAddHeader(const std::string& key);
 
-    static bool IsContentType(const std::string& key) {
-        return HeaderKeyEqual()(key, "content-type");
+    std::string& AddHeader(const std::string& key);
+
+    bool IsSetCookie(const std::string& key) const {
+        return _header_key_equal(key, SET_COOKIE);
+    }
+
+    bool IsCookie(const std::string& key) const {
+        return _header_key_equal(key, COOKIE);
+    }
+
+    bool IsContentType(const std::string& key) const {
+        return _header_key_equal(key, CONTENT_TYPE);
+    }
+
+    // Return true if the header can be folded in line,
+    // otherwise, returns false, i.e., Set-Cookie header.
+    // See comments of `AppendHeader'.
+    bool CanFoldedInLine(const std::string& key) {
+        return !IsSetCookie(key);
+    }
+
+    const char* HeaderValueDelimiter(const std::string& key) {
+        return IsCookie(key) ? "; " : ",";
     }
 
+    HeaderKeyEqual _header_key_equal;
     HeaderMap _headers;
     URI _uri;
     int _status_code;
     HttpMethod _method;
     std::string _content_type;
     std::string _unresolved_path;
     std::pair<int, int> _version;
+    std::string* _first_set_cookie;
 };
 
 const HttpHeader& DefaultHttpHeader();

diff --git a/src/butil/containers/case_ignored_flat_map.h b/src/butil/containers/case_ignored_flat_map.h
@@ -68,6 +68,10 @@ class CaseIgnoredFlatMap : public butil::FlatMap<
 class CaseIgnoredFlatSet : public butil::FlatSet<
     std::string, CaseIgnoredHasher, CaseIgnoredEqual> {};
 
+template <typename T>
+class CaseIgnoredMultiFlatMap : public butil::FlatMap<
+    std::string, T, CaseIgnoredHasher, CaseIgnoredEqual, false, PtAllocator, true> {};
+
 } // namespace butil
 
 #endif  // BUTIL_CASE_IGNORED_FLAT_MAP_H