Duplicate "Content-Length" field in reponse headers

[Laro88]

I initially thought Postman was messing with me, however Chrome shows the same so here goes :-)

req.reply(web::http::status_codes::OK, "");

results in a duplicate header as seen here: however

req.reply(web::http::status_codes::OK);

is fine. I haven't done any further tracing to find the place where there are duplicate assignments (not a std::map i guess), and I only stumbled upon it by accident :-)

[maxouellet]

I ran into this bug yesterday, and ended up investigating it more thoroughly

Basically, cpprest on Windows ends up calling HttpSendHttpResponse. This requires filling a HTTP_RESPONSE_HEADERS struct. Problem is that HTTP_RESPONSE_HEADERS has two separate fields:

• pUnknownHeaders
• KnownHeaders

The expectation is that standard headers should be put in KnownHeaders (full list here), whereas other arbitrary headers should be put in pUnknownHeaders.

cppRest was putting all its headers inside pUnknownHeaders. I believe this led to the Windows API thinking that Content-Length (a known header) wasn't set, and therefore it was force-adding some default value for Content-Length. Thus leading to duplicate header entries.

Here's a patch I extracted from our codebase where I made a fix, in case it helps anyone else. I did this quickly, so don't judge if it's not the most optimal code ever :)

diff --git a/src/http/listener/http_server_httpsys.cpp b/src/http/listener/http_server_httpsys.cpp
--- a/src/http/listener/http_server_httpsys.cpp
+++ b/src/http/listener/http_server_httpsys.cpp
@@ -917,23 +917,52 @@
         content_length = m_response._get_impl()->_get_content_length();
     }

-    m_headers = std::unique_ptr<HTTP_UNKNOWN_HEADER[]>(
-        new HTTP_UNKNOWN_HEADER[msl::safeint3::SafeInt<size_t>(m_response.headers().size())]);
-    m_headers_buffer.resize(msl::safeint3::SafeInt<size_t>(m_response.headers().size()) * 2);

-    win_api_response.Headers.UnknownHeaderCount = (USHORT)m_response.headers().size();
+    // We need to reconvert our headers to split them between known & unknown headers
+    // Otherwise Windows can do weird behavior in HttpSendHttpResponse where some
+    // headers like Content-Length get duplicated
+    // Note that it's critical to reserve space for the headers and store them somewhere, since
+    // the Windows HTTP headers struct stores a pointer to the strings. Therefore, the strings
+    // must be kept alive at least until HttpSendHttpResponse consumes them
+    m_known_headers_buffer.clear();
+    size_t knownHeadersPredictedSize = std::min((size_t)HttpHeaderResponseMaximum, m_response.headers().size());
+    m_known_headers_buffer.reserve(msl::safeint3::SafeInt<size_t>(knownHeadersPredictedSize));
+    m_unknown_headers_buffer.clear();
+    size_t numUnkownHeaders = 0;
+
+    for (auto iter = m_response.headers().begin(); iter != m_response.headers().end(); ++iter) {
+        bool isKnownHeader = false;
+         for (int i = 0; i < HttpHeaderResponseMaximum; ++i) {
+            if (iter->first == HttpServerAPIRequestKnownHeaders[i]) {
+                m_known_headers_buffer.push_back(utf16_to_utf8(iter->second));
+                win_api_response.Headers.KnownHeaders[i].pRawValue = m_known_headers_buffer[m_known_headers_buffer.size() - 1].c_str();
+                win_api_response.Headers.KnownHeaders[i].RawValueLength = (USHORT)(m_known_headers_buffer[m_known_headers_buffer.size() - 1].size());
+                isKnownHeader = true;
+                break;
+            }
+        }
+
+        if (!isKnownHeader) {
+            m_unknown_headers_buffer.push_back(utf16_to_utf8(iter->first));
+            m_unknown_headers_buffer.push_back(utf16_to_utf8(iter->second));
+            numUnkownHeaders++;
+        }
+    }
+
+    m_headers = std::unique_ptr<HTTP_UNKNOWN_HEADER[]>(
+        new HTTP_UNKNOWN_HEADER[msl::safeint3::SafeInt<size_t>(numUnkownHeaders)]);
+    win_api_response.Headers.UnknownHeaderCount = (USHORT)numUnkownHeaders;
     win_api_response.Headers.pUnknownHeaders = m_headers.get();
-    int headerIndex = 0;
-    for (auto iter = m_response.headers().begin(); iter != m_response.headers().end(); ++iter, ++headerIndex)
+
+    for (size_t headerIndex = 0; headerIndex < numUnkownHeaders; ++headerIndex)
     {
-        m_headers_buffer[headerIndex * 2] = utf16_to_utf8(iter->first);
-        m_headers_buffer[headerIndex * 2 + 1] = utf16_to_utf8(iter->second);
         win_api_response.Headers.pUnknownHeaders[headerIndex].NameLength =
-            (USHORT)m_headers_buffer[headerIndex * 2].size();
-        win_api_response.Headers.pUnknownHeaders[headerIndex].pName = m_headers_buffer[headerIndex * 2].c_str();
+            (USHORT)m_unknown_headers_buffer[headerIndex * 2].size();
+        win_api_response.Headers.pUnknownHeaders[headerIndex].pName = m_unknown_headers_buffer[headerIndex * 2].c_str();
         win_api_response.Headers.pUnknownHeaders[headerIndex].RawValueLength =
-            (USHORT)m_headers_buffer[headerIndex * 2 + 1].size();
-        win_api_response.Headers.pUnknownHeaders[headerIndex].pRawValue = m_headers_buffer[headerIndex * 2 + 1].c_str();
+            (USHORT)m_unknown_headers_buffer[headerIndex * 2 + 1].size();
+        win_api_response.Headers.pUnknownHeaders[headerIndex].pRawValue =
+            m_unknown_headers_buffer[headerIndex * 2 + 1].c_str();
     }

     // Send response callback function
diff --git a/src/http/listener/http_server_httpsys.h b/src/http/listener/http_server_httpsys.h
--- a/src/http/listener/http_server_httpsys.h
+++ b/src/http/listener/http_server_httpsys.h
@@ -133,7 +133,8 @@
     std::unique_ptr<unsigned char[]> m_request_buffer;

     std::unique_ptr<HTTP_UNKNOWN_HEADER[]> m_headers;
-    std::vector<std::string> m_headers_buffer;
+    std::vector<std::string> m_known_headers_buffer;
+    std::vector<std::string> m_unknown_headers_buffer;

     http_overlapped m_overlapped;