From 3cb143f5f2d9a424e49c1fe59bae6fe474cc4719 Mon Sep 17 00:00:00 2001 From: andres Date: Tue, 23 Jun 2026 13:27:00 -0500 Subject: [PATCH 1/2] fix: Handles multiple cookie headers correctly according to rfc6265 * Closes #439 --- src/httprequest.cpp | 11 +++++-- tests/testthat/test-http-parse.R | 55 ++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/httprequest.cpp b/src/httprequest.cpp index 55fee952..6dd72766 100644 --- a/src/httprequest.cpp +++ b/src/httprequest.cpp @@ -253,8 +253,15 @@ int HttpRequest::_on_header_value(http_parser* pParser, const char* pAt, size_t // ...and is already non-empty... if (value.size() > 0) { - // ...and this value is also non-empty, then combine using comma... - value = _headers[_lastHeaderField] + "," + value; + // ...and if this value is also non-empty, then combine. + // Use semicolon separator for Cookie headers per RFC 6265, comma for others. + std::string separator; + if (to_lower(_lastHeaderField) == "cookie") { + separator = "; "; + } else { + separator = ","; + } + value = _headers[_lastHeaderField] + separator + value; } else { // ...but if this value is empty, then use previous value (no-op). value = _headers[_lastHeaderField]; diff --git a/tests/testthat/test-http-parse.R b/tests/testthat/test-http-parse.R index 656120b4..1ffc0b87 100644 --- a/tests/testthat/test-http-parse.R +++ b/tests/testthat/test-http-parse.R @@ -71,6 +71,61 @@ test_that("Large HTTP header values are preserved", { }) +test_that("Multiple Cookie headers are joined with semicolon", { + # This is a test for correct Cookie header concatenation per RFC 6265. + # When multiple Cookie headers are present, they should be joined with "; " + # instead of "," which is used for other headers. + s <- httpuv::startServer( + "0.0.0.0", + randomPort(), + list( + call = function(req) { + list( + status = 200L, + headers = list('Content-Type' = 'text/plain'), + body = paste0("", req$HTTP_COOKIE) + ) + } + ) + ) + on.exit(s$stop()) + + # Test multiple Cookie headers - they should be joined with "; " + h <- new_handle() + handle_setheaders( + h, + `Cookie` = "session=abc123", + `Cookie` = "user=john" + ) + res <- fetch(local_url("/", s$getPort()), h) + content <- rawToChar(res$content) + expect_identical(content, "session=abc123; user=john") + + # Test three Cookie headers + h <- new_handle() + handle_setheaders( + h, + `Cookie` = "session=abc123", + `Cookie` = "user=john", + `Cookie` = "theme=dark" + ) + res <- fetch(local_url("/", s$getPort()), h) + content <- rawToChar(res$content) + expect_identical(content, "session=abc123; user=john; theme=dark") + + # Verify that non-Cookie headers still use comma separator + h <- new_handle() + handle_setheaders( + h, + `X-Custom` = "value1", + `X-Custom` = "value2" + ) + res <- fetch(local_url("/", s$getPort()), h) + content <- rawToChar(res$content) + # The body should be empty since we're not setting Cookie, but we test + # the X-Custom header via a separate server that returns it +}) + test_that("Large HTTP header field names are preserved", { # Also for https://github.com/rstudio/httpuv/issues/275 # This tests for field names that are split across messages. From c17b15804c714ba98e5d04db5793f708c12b5ae2 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Mon, 21 Sep 2026 12:21:40 -0400 Subject: [PATCH 2/2] test: Cover non-adjacent Cookie headers and the comma path The third sub-test had no expectation, so the non-Cookie comma separator was never actually verified. Capture req$HEADERS and assert on it, and add a case where two Cookie headers are separated by another header and differ in case. Also add a NEWS entry. --- NEWS.md | 2 ++ tests/testthat/test-http-parse.R | 22 ++++++++++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/NEWS.md b/NEWS.md index b49eeb70..90ba392a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # httpuv (development version) +* Fixed #439: Multiple `Cookie` request headers are now combined with `"; "` instead of `","`, as required by RFC 6265 (thanks to @andyquinterom, #440). + # httpuv 1.6.17 * Closed #426: Uses native symbol registration for calls into compiled code, resulting in performance gains from not having to perform a lookup on each call. (#427) diff --git a/tests/testthat/test-http-parse.R b/tests/testthat/test-http-parse.R index 1ffc0b87..be8b369e 100644 --- a/tests/testthat/test-http-parse.R +++ b/tests/testthat/test-http-parse.R @@ -75,11 +75,14 @@ test_that("Multiple Cookie headers are joined with semicolon", { # This is a test for correct Cookie header concatenation per RFC 6265. # When multiple Cookie headers are present, they should be joined with "; " # instead of "," which is used for other headers. + headers_received <- NULL s <- httpuv::startServer( "0.0.0.0", randomPort(), list( call = function(req) { + # Save the headers for examination later + headers_received <<- req$HEADERS list( status = 200L, headers = list('Content-Type' = 'text/plain'), @@ -113,17 +116,28 @@ test_that("Multiple Cookie headers are joined with semicolon", { content <- rawToChar(res$content) expect_identical(content, "session=abc123; user=john; theme=dark") - # Verify that non-Cookie headers still use comma separator + # Cookie headers are combined even when another header sits between them, + # and regardless of the case used for the field name. h <- new_handle() handle_setheaders( h, + `Cookie` = "session=abc123", `X-Custom` = "value1", - `X-Custom` = "value2" + `cookie` = "user=john" ) res <- fetch(local_url("/", s$getPort()), h) content <- rawToChar(res$content) - # The body should be empty since we're not setting Cookie, but we test - # the X-Custom header via a separate server that returns it + expect_identical(content, "session=abc123; user=john") + + # Verify that non-Cookie headers still use comma separator + h <- new_handle() + handle_setheaders( + h, + `X-Custom` = "value1", + `X-Custom` = "value2" + ) + fetch(local_url("/", s$getPort()), h) + expect_identical(headers_received[["x-custom"]], "value1,value2") }) test_that("Large HTTP header field names are preserved", {