Commit 8d2644cc authored by Kenton Varda's avatar Kenton Varda

Deal with Set-Cookie not being comma-concatenation-friendly.

This is needed now because http-over-capnp will index the Set-Cookie header.

This change should make it relatively safe to index Set-Cookie when using KJ HTTP.
parent f5190d24
......@@ -694,9 +694,18 @@ kj::HttpHeaders HttpOverCapnpFactory::headersToKj(
result.set(nameCapnpToKj[nameInt], valueCapnpToKj[cvInt]);
break;
}
case capnp::HttpHeader::Common::VALUE:
result.set(nameCapnpToKj[nameInt], nv.getValue());
case capnp::HttpHeader::Common::VALUE: {
auto headerId = nameCapnpToKj[nameInt];
if (result.get(headerId) == nullptr) {
result.set(headerId, nv.getValue());
} else {
// Unusual: This is a duplicate header, so fall back to add(), which may trigger
// comma-concatenation, except in certain cases where comma-concatentaion would
// be problematic.
result.add(headerId.toString(), nv.getValue());
}
break;
}
}
break;
}
......
......@@ -307,6 +307,29 @@ KJ_TEST("HttpHeaders validation") {
KJ_EXPECT_THROW_MESSAGE("invalid header value", headers.add("Valid-Name", "in\nvalid"));
}
KJ_TEST("HttpHeaders Set-Cookie handling") {
HttpHeaderTable::Builder builder;
auto hCookie = builder.add("Cookie");
auto hSetCookie = builder.add("Set-Cookie");
auto table = builder.build();
HttpHeaders headers(*table);
headers.set(hCookie, "Foo");
headers.add("Cookie", "Bar");
headers.add("Cookie", "Baz");
headers.set(hSetCookie, "Foo");
headers.add("Set-Cookie", "Bar");
headers.add("Set-Cookie", "Baz");
auto text = headers.toString();
KJ_EXPECT(text ==
"Cookie: Foo, Bar, Baz\r\n"
"Set-Cookie: Foo\r\n"
"Set-Cookie: Bar\r\n"
"Set-Cookie: Baz\r\n"
"\r\n", text);
}
// =======================================================================================
class ReadFragmenter final: public kj::AsyncIoStream {
......
......@@ -679,9 +679,16 @@ void HttpHeaders::addNoCheck(kj::StringPtr name, kj::StringPtr value) {
indexedHeaders[id->id] = value;
} else {
// Duplicate HTTP headers are equivalent to the values being separated by a comma.
auto concat = kj::str(indexedHeaders[id->id], ", ", value);
indexedHeaders[id->id] = concat;
ownedStrings.add(concat.releaseArray());
if (strcasecmp(name.cStr(), "set-cookie") == 0) {
// Uh-oh, Set-Cookie will be corrupted if we try to concatenate it. We'll make it an
// unindexed header, which is weird, but the alternative is guaranteed corruption, so...
// TODO(cleanup): Maybe HttpHeaders should just special-case set-cookie in general?
unindexedHeaders.add(Header {name, value});
} else {
auto concat = kj::str(indexedHeaders[id->id], ", ", value);
indexedHeaders[id->id] = concat;
ownedStrings.add(concat.releaseArray());
}
}
} else {
unindexedHeaders.add(Header {name, value});
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment