-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Add Headers.prototype.clear() for reusing instances without reallocation #34548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
8933cd4
4ce6b4c
341ddca
a1729a5
7213bc6
a833224
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,6 +69,7 @@ static JSC_DECLARE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_delete); | |
| static JSC_DECLARE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_get); | ||
| static JSC_DECLARE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_has); | ||
| static JSC_DECLARE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_set); | ||
| static JSC_DECLARE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_clear); | ||
| static JSC_DECLARE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_entries); | ||
| static JSC_DECLARE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_keys); | ||
| static JSC_DECLARE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_values); | ||
|
|
@@ -309,6 +310,7 @@ static const HashTableValue JSFetchHeadersPrototypeTableValues[] = { | |
| { "getAll"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { HashTableValue::NativeFunctionType, jsFetchHeadersPrototypeFunction_getAll, 1 } }, | ||
| { "has"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { HashTableValue::NativeFunctionType, jsFetchHeadersPrototypeFunction_has, 1 } }, | ||
| { "set"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { HashTableValue::NativeFunctionType, jsFetchHeadersPrototypeFunction_set, 2 } }, | ||
| { "clear"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { HashTableValue::NativeFunctionType, jsFetchHeadersPrototypeFunction_clear, 0 } }, | ||
| { "entries"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { HashTableValue::NativeFunctionType, jsFetchHeadersPrototypeFunction_entries, 0 } }, | ||
| { "keys"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { HashTableValue::NativeFunctionType, jsFetchHeadersPrototypeFunction_keys, 0 } }, | ||
| { "values"_s, static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { HashTableValue::NativeFunctionType, jsFetchHeadersPrototypeFunction_values, 0 } }, | ||
|
|
@@ -444,6 +446,21 @@ JSC_DEFINE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_delete, (JSGlobalObject | |
| return IDLOperation<JSFetchHeaders>::call<jsFetchHeadersPrototypeFunction_deleteBody>(*lexicalGlobalObject, *callFrame, "delete"); | ||
| } | ||
|
|
||
| static inline JSC::EncodedJSValue jsFetchHeadersPrototypeFunction_clearBody(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame, typename IDLOperation<JSFetchHeaders>::ClassParameter castedThis) | ||
| { | ||
| auto& vm = JSC::getVM(lexicalGlobalObject); | ||
| auto throwScope = DECLARE_THROW_SCOPE(vm); | ||
| UNUSED_PARAM(throwScope); | ||
| UNUSED_PARAM(callFrame); | ||
| auto& impl = castedThis->wrapped(); | ||
| RELEASE_AND_RETURN(throwScope, JSValue::encode(toJS<IDLUndefined>(*lexicalGlobalObject, throwScope, [&]() -> decltype(auto) { return impl.clear(); }))); | ||
| } | ||
|
|
||
| JSC_DEFINE_HOST_FUNCTION(jsFetchHeadersPrototypeFunction_clear, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) | ||
| { | ||
| return IDLOperation<JSFetchHeaders>::call<jsFetchHeadersPrototypeFunction_clearBody>(*lexicalGlobalObject, *callFrame, "clear"); | ||
| } | ||
|
|
||
| static inline JSC::EncodedJSValue jsFetchHeadersPrototypeFunction_getBody(JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame, typename IDLOperation<JSFetchHeaders>::ClassParameter castedThis) | ||
| { | ||
| auto& vm = JSC::getVM(lexicalGlobalObject); | ||
|
|
@@ -597,19 +614,11 @@ JSC_DEFINE_HOST_FUNCTION(jsFetchHeaders_getRawKeys, (JSC::JSGlobalObject * lexic | |
| } | ||
|
|
||
| FetchHeaders& headers = thisObject->wrapped(); | ||
| // HTTPHeaderMap's iterator covers only the common and uncommon segments; | ||
| // set-cookie values live in their own segment, so size() (which counts | ||
| // every cookie) used to leave trailing holes in the array. Size for one | ||
| // entry per unique name and append "set-cookie" explicitly. | ||
| JSArray* outArray = JSC::JSArray::create(vm, lexicalGlobalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous), headers.sizeAfterJoiningSetCookieHeader()); | ||
|
|
||
| unsigned int i = 0; | ||
| for (const auto& header : headers.internalHeaders()) { | ||
| JSArray* outArray = JSC::JSArray::create(vm, lexicalGlobalObject->arrayStructureForIndexingTypeDuringAllocation(JSC::ArrayWithContiguous), headers.size()); | ||
|
|
||
| for (unsigned int i = 0; const auto& header : headers.internalHeaders()) { | ||
| outArray->putDirectIndex(lexicalGlobalObject, i++, jsString(vm, header.name())); | ||
| } | ||
|
Comment on lines
+617
to
621
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
ast-grep outline src/jsc/bindings/webcore/JSFetchHeaders.cpp --match 'jsFetchHeaders_getRawKeys' --view expanded
rg -n -C3 'putDirectIndex\(' src/jsc/bindings/webcore/JSFetchHeaders.cppRepository: oven-sh/bun Length of output: 1390 🏁 Script executed: sed -n '600,630p' src/jsc/bindings/webcore/JSFetchHeaders.cppRepository: oven-sh/bun Length of output: 1434 Propagate exceptions after each header insert. 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| if (!headers.internalHeaders().getSetCookieHeaders().isEmpty()) { | ||
| outArray->putDirectIndex(lexicalGlobalObject, i++, jsString(vm, WTF::httpHeaderNameDefaultCaseStringImpl(HTTPHeaderName::SetCookie))); | ||
| } | ||
|
|
||
| RELEASE_AND_RETURN(scope, JSValue::encode(outArray)); | ||
| } | ||
|
|
@@ -704,7 +713,7 @@ JSC::JSValue getInternalProperties(JSC::VM& vm, JSGlobalObject* lexicalGlobalObj | |
| for (const auto& it : vec) { | ||
| const auto& name = it.key; | ||
| const auto& value = it.value; | ||
| obj->putDirectMayBeIndex(lexicalGlobalObject, Identifier::fromString(vm, lowercaseHeaderName(name)), jsString(vm, value)); | ||
| obj->putDirectMayBeIndex(lexicalGlobalObject, Identifier::fromString(vm, name.convertToASCIILowercase()), jsString(vm, value)); | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep regression coverage for the changed uncommon-header lowercase path.
src/jsc/bindings/webcore/HTTPHeaderMap.h#L104-L104: retain validation thatconvertToASCIILowercase()normalizes uncommon names during iteration.test/js/web/fetch/headers.test.ts#L549-L550: replace the removed implementation-specific tests with publicentries(),keys(), and iterator assertions using an uppercase uncommon header.As per coding guidelines, behavioral changes require automated coverage and existing safety nets must remain protected.
📍 Affects 2 files
src/jsc/bindings/webcore/HTTPHeaderMap.h#L104-L104(this comment)test/js/web/fetch/headers.test.ts#L549-L550🤖 Prompt for AI Agents
Source: Coding guidelines