Skip to content

Commit 66c9b44

Browse files
dylan-conwayJarred-Sumner
authored and
paperclover
committed
fix http set cookie headers (oven-sh#5428)
* allow multiple set-cookie values * make it work for `getHeader` * move `getHeader` to cpp * remove set-cookie check * move `setHeader` to cpp --------- Co-authored-by: Jarred Sumner <[email protected]>
1 parent 23b7bdf commit 66c9b44

File tree

6 files changed

+139
-27
lines changed

6 files changed

+139
-27
lines changed

‎src/bun.js/bindings/ZigGlobalObject.cpp

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1552,6 +1552,89 @@ JSC_DEFINE_HOST_FUNCTION(jsTTYSetMode, (JSC::JSGlobalObject * globalObject, Call
15521552
return JSValue::encode(jsNumber(err));
15531553
}
15541554

1555+
JSC_DEFINE_HOST_FUNCTION(jsHTTPGetHeader, (JSGlobalObject * globalObject, CallFrame* callFrame))
1556+
{
1557+
auto& vm = globalObject->vm();
1558+
auto scope = DECLARE_THROW_SCOPE(vm);
1559+
1560+
JSValue headersValue = callFrame->argument(0);
1561+
1562+
if (auto* headers = jsDynamicCast<WebCore::JSFetchHeaders*>(headersValue)) {
1563+
JSValue nameValue = callFrame->argument(1);
1564+
if (nameValue.isString()) {
1565+
FetchHeaders* impl = &headers->wrapped();
1566+
String name = nameValue.toWTFString(globalObject);
1567+
if (WTF::equalIgnoringASCIICase(name, "set-cookie"_s)) {
1568+
return fetchHeadersGetSetCookie(globalObject, vm, impl);
1569+
}
1570+
1571+
WebCore::ExceptionOr<String> res = impl->get(name);
1572+
if (res.hasException()) {
1573+
WebCore::propagateException(globalObject, scope, res.releaseException());
1574+
return JSValue::encode(jsUndefined());
1575+
}
1576+
1577+
String value = res.returnValue();
1578+
if (value.isEmpty()) {
1579+
return JSValue::encode(jsUndefined());
1580+
}
1581+
1582+
return JSC::JSValue::encode(jsString(vm, value));
1583+
}
1584+
}
1585+
1586+
return JSValue::encode(jsUndefined());
1587+
}
1588+
1589+
JSC_DEFINE_HOST_FUNCTION(jsHTTPSetHeader, (JSGlobalObject * globalObject, CallFrame* callFrame))
1590+
{
1591+
auto& vm = globalObject->vm();
1592+
auto scope = DECLARE_THROW_SCOPE(vm);
1593+
1594+
JSValue headersValue = callFrame->argument(0);
1595+
1596+
if (auto* headers = jsDynamicCast<WebCore::JSFetchHeaders*>(headersValue)) {
1597+
JSValue nameValue = callFrame->argument(1);
1598+
if (nameValue.isString()) {
1599+
String name = nameValue.toWTFString(globalObject).convertToASCIILowercase();
1600+
FetchHeaders* impl = &headers->wrapped();
1601+
1602+
JSValue valueValue = callFrame->argument(2);
1603+
if (valueValue.isUndefined())
1604+
return JSValue::encode(jsUndefined());
1605+
1606+
if (isArray(globalObject, valueValue)) {
1607+
auto* array = jsCast<JSArray*>(valueValue);
1608+
unsigned length = array->length();
1609+
if (length > 0) {
1610+
JSValue item = array->getIndex(globalObject, 0);
1611+
if (UNLIKELY(scope.exception()))
1612+
return JSValue::encode(jsUndefined());
1613+
impl->set(name, item.getString(globalObject));
1614+
RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined()));
1615+
}
1616+
for (unsigned i = 1; i < length; ++i) {
1617+
JSValue value = array->getIndex(globalObject, i);
1618+
if (UNLIKELY(scope.exception()))
1619+
return JSValue::encode(jsUndefined());
1620+
if (!value.isString())
1621+
continue;
1622+
impl->append(name, value.getString(globalObject));
1623+
RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined()));
1624+
}
1625+
RELEASE_AND_RETURN(scope, JSValue::encode(jsUndefined()));
1626+
return JSValue::encode(jsUndefined());
1627+
}
1628+
1629+
impl->set(name, valueValue.getString(globalObject));
1630+
RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined()));
1631+
return JSValue::encode(jsUndefined());
1632+
}
1633+
}
1634+
1635+
return JSValue::encode(jsUndefined());
1636+
}
1637+
15551638
JSC_DEFINE_CUSTOM_GETTER(noop_getter, (JSGlobalObject*, EncodedJSValue, PropertyName))
15561639
{
15571640
return JSC::JSValue::encode(JSC::jsUndefined());
@@ -1686,6 +1769,17 @@ static JSC_DEFINE_HOST_FUNCTION(functionLazyLoad,
16861769
return JSC::JSValue::encode(JSSQLStatementConstructor::create(vm, globalObject, JSSQLStatementConstructor::createStructure(vm, globalObject, globalObject->m_functionPrototype.get())));
16871770
}
16881771

1772+
if (string == "http"_s) {
1773+
auto* obj = constructEmptyObject(globalObject);
1774+
obj->putDirect(
1775+
vm, JSC::PropertyName(JSC::Identifier::fromString(vm, "setHeader"_s)),
1776+
JSC::JSFunction::create(vm, globalObject, 3, "setHeader"_s, jsHTTPSetHeader, ImplementationVisibility::Public), NoIntrinsic);
1777+
obj->putDirect(
1778+
vm, JSC::PropertyName(JSC::Identifier::fromString(vm, "getHeader"_s)),
1779+
JSC::JSFunction::create(vm, globalObject, 2, "getHeader"_s, jsHTTPGetHeader, ImplementationVisibility::Public), NoIntrinsic);
1780+
return JSC::JSValue::encode(obj);
1781+
}
1782+
16891783
if (string == "worker_threads"_s) {
16901784

16911785
JSValue workerData = jsUndefined();

‎src/bun.js/bindings/webcore/JSFetchHeaders.cpp

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -245,38 +245,43 @@ JSC_DEFINE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_getAll, (JSGlobalObject
245245
return JSValue::encode(array);
246246
}
247247

248-
JSC_DEFINE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_getSetCookie, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame))
248+
JSC_DEFINE_HOST_FUNCTION(fetchHeadersGetSetCookie, (JSC::JSGlobalObject * lexicalGlobalObject, VM& vm, WebCore::FetchHeaders* impl))
249249
{
250-
auto& vm = JSC::getVM(lexicalGlobalObject);
251250
auto scope = DECLARE_THROW_SCOPE(vm);
252-
JSFetchHeaders* castedThis = jsDynamicCast<JSFetchHeaders*>(callFrame->thisValue());
253-
if (UNLIKELY(!castedThis)) {
254-
return JSValue::encode(jsUndefined());
255-
}
256251

257-
auto& impl = castedThis->wrapped();
258-
auto values = impl.getSetCookieHeaders();
252+
auto values = impl->getSetCookieHeaders();
259253
unsigned count = values.size();
260254

261255
if (!count) {
262256
return JSValue::encode(JSC::constructEmptyArray(lexicalGlobalObject, nullptr, 0));
263257
}
264258

265259
JSC::JSArray* array = constructEmptyArray(lexicalGlobalObject, nullptr, count);
266-
RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined()));
260+
RETURN_IF_EXCEPTION(scope, encodedJSValue());
267261
if (UNLIKELY(!array)) {
268262
throwOutOfMemoryError(lexicalGlobalObject, scope);
269-
return JSValue::encode(jsUndefined());
263+
return encodedJSValue();
270264
}
271265

272266
for (unsigned i = 0; i < count; ++i) {
273267
array->putDirectIndex(lexicalGlobalObject, i, jsString(vm, values[i]));
274-
RETURN_IF_EXCEPTION(scope, JSValue::encode(jsUndefined()));
268+
RETURN_IF_EXCEPTION(scope, encodedJSValue());
275269
}
276270

277271
return JSValue::encode(array);
278272
}
279273

274+
JSC_DEFINE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_getSetCookie, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame))
275+
{
276+
auto& vm = JSC::getVM(lexicalGlobalObject);
277+
JSFetchHeaders* castedThis = jsDynamicCast<JSFetchHeaders*>(callFrame->thisValue());
278+
if (UNLIKELY(!castedThis)) {
279+
return JSValue::encode(jsUndefined());
280+
}
281+
auto& impl = castedThis->wrapped();
282+
return fetchHeadersGetSetCookie(lexicalGlobalObject, vm, &impl);
283+
}
284+
280285
/* Hash table for prototype */
281286

282287
static const HashTableValue JSFetchHeadersPrototypeTableValues[] = {
@@ -688,7 +693,6 @@ extern void* _ZTVN7WebCore12FetchHeadersE[];
688693

689694
JSC::JSValue toJSNewlyCreated(JSC::JSGlobalObject*, JSDOMGlobalObject* globalObject, Ref<FetchHeaders>&& impl)
690695
{
691-
692696
if constexpr (std::is_polymorphic_v<FetchHeaders>) {
693697
#if ENABLE(BINDING_INTEGRITY)
694698
const void* actualVTablePointer = getVTablePointer(impl.ptr());

‎src/bun.js/bindings/webcore/JSFetchHeaders.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,6 @@ template<> struct JSDOMWrapperConverterTraits<FetchHeaders> {
9797
using ToWrappedReturnType = FetchHeaders*;
9898
};
9999

100+
JSC::EncodedJSValue fetchHeadersGetSetCookie(JSC::JSGlobalObject* lexicalGlobalObject, VM& vm, WebCore::FetchHeaders* impl);
101+
100102
} // namespace WebCore

‎src/js/node/http.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
const EventEmitter = require("node:events");
33
const { isTypedArray } = require("node:util/types");
44
const { Duplex, Readable, Writable } = require("node:stream");
5+
const { getHeader, setHeader } = $lazy("http");
56

67
const headerCharRegex = /[^\t\x20-\x7e\x80-\xff]/;
78
/**
@@ -75,13 +76,9 @@ const searchParamsSymbol = Symbol.for("query"); // This is the symbol used in No
7576
const StringPrototypeSlice = String.prototype.slice;
7677
const StringPrototypeStartsWith = String.prototype.startsWith;
7778
const StringPrototypeToUpperCase = String.prototype.toUpperCase;
78-
const StringPrototypeIncludes = String.prototype.includes;
79-
const StringPrototypeCharCodeAt = String.prototype.charCodeAt;
80-
const StringPrototypeIndexOf = String.prototype.indexOf;
8179
const ArrayIsArray = Array.isArray;
8280
const RegExpPrototypeExec = RegExp.prototype.exec;
8381
const ObjectAssign = Object.assign;
84-
const ObjectPrototypeHasOwnProperty = Object.prototype.hasOwnProperty;
8582

8683
const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;
8784
const NODE_HTTP_WARNING =
@@ -126,12 +123,6 @@ function validateFunction(callable: any, field: string) {
126123
return callable;
127124
}
128125

129-
function getHeader(headers, name) {
130-
if (!headers) return;
131-
const result = headers.get(name);
132-
return result == null ? undefined : result;
133-
}
134-
135126
type FakeSocket = InstanceType<typeof FakeSocket>;
136127
var FakeSocket = class Socket extends Duplex {
137128
[kInternalSocketData]: any;
@@ -958,8 +949,11 @@ let OriginalWriteHeadFn, OriginalImplicitHeadFn;
958949
class ServerResponse extends Writable {
959950
declare _writableState: any;
960951

961-
constructor({ req, reply }) {
952+
constructor(c) {
962953
super();
954+
if (!c) c = {};
955+
var req = c.req || {};
956+
var reply = c.reply;
963957
this.req = req;
964958
this._reply = reply;
965959
this.sendDate = true;
@@ -1174,7 +1168,7 @@ class ServerResponse extends Writable {
11741168

11751169
setHeader(name, value) {
11761170
var headers = (this.#headers ??= new Headers());
1177-
headers.set(name, value);
1171+
setHeader(headers, name, value);
11781172
return this;
11791173
}
11801174

‎src/js/out/InternalModuleRegistryConstants.h

Lines changed: 3 additions & 3 deletions
Large diffs are not rendered by default.

‎test/js/node/http/node-http.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
Server,
99
validateHeaderName,
1010
validateHeaderValue,
11+
ServerResponse,
1112
} from "node:http";
1213
import { createTest } from "node-harness";
1314
import url from "node:url";
@@ -113,6 +114,23 @@ describe("node:http", () => {
113114
});
114115
});
115116

117+
describe("response", () => {
118+
test("set-cookie works with getHeader", () => {
119+
const res = new ServerResponse({});
120+
res.setHeader("Set-Cookie", ["swag=true", "yolo=true"]);
121+
expect(res.getHeader("Set-Cookie")).toEqual(["swag=true", "yolo=true"]);
122+
});
123+
test("set-cookie works with getHeaders", () => {
124+
const res = new ServerResponse({});
125+
res.setHeader("Set-Cookie", ["swag=true", "yolo=true"]);
126+
res.setHeader("test", "test");
127+
expect(res.getHeaders()).toEqual({
128+
"Set-Cookie": ["swag=true", "yolo=true"],
129+
"test": "test",
130+
});
131+
});
132+
});
133+
116134
describe("request", () => {
117135
function runTest(done: Function, callback: (server: Server, port: number, done: (err?: Error) => void) => void) {
118136
var timer;

0 commit comments

Comments
 (0)