Skip to content

Commit 4227165

Browse files
committed
Avoid mutating the api::Headers immediately after creation
Modify the kj::HttpHeaders instead and then construct the api::Headers from that.
1 parent 929c41b commit 4227165

File tree

1 file changed

+9
-13
lines changed

1 file changed

+9
-13
lines changed

src/workerd/api/global-scope.c++

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,6 @@ kj::Promise<DeferredProxy<void>> ServiceWorkerGlobalScope::request(kj::HttpMetho
165165

166166
CfProperty cf(cfBlobJson);
167167

168-
auto jsHeaders = js.alloc<Headers>(js, headers, Headers::Guard::REQUEST);
169-
170168
// We only create the body stream if there is a body to read.
171169
kj::Maybe<jsg::Ref<ReadableStream>> maybeJsStream = kj::none;
172170

@@ -190,9 +188,10 @@ kj::Promise<DeferredProxy<void>> ServiceWorkerGlobalScope::request(kj::HttpMetho
190188
//
191189
// TODO(cleanup): Should KJ HTTP interfaces explicitly communicate the difference between a
192190
// missing body and an empty one?
191+
auto newHeaders = headers.cloneShallow();
193192
kj::Maybe<Body::ExtractedBody> body;
194-
if (headers.get(kj::HttpHeaderId::CONTENT_LENGTH) != kj::none ||
195-
headers.get(kj::HttpHeaderId::TRANSFER_ENCODING) != kj::none ||
193+
if (newHeaders.get(kj::HttpHeaderId::CONTENT_LENGTH) != kj::none ||
194+
newHeaders.get(kj::HttpHeaderId::TRANSFER_ENCODING) != kj::none ||
196195
requestBody.tryGetLength().orDefault(1) > 0) {
197196
// We do not automatically decode gzipped request bodies because the fetch() standard doesn't
198197
// specify any automatic encoding of requests. https://github.com/whatwg/fetch/issues/589
@@ -205,16 +204,12 @@ kj::Promise<DeferredProxy<void>> ServiceWorkerGlobalScope::request(kj::HttpMetho
205204
// If the request doesn't specify "Content-Length" or "Transfer-Encoding", set "Content-Length"
206205
// to the body length if it's known. This ensures handlers for worker-to-worker requests can
207206
// access known body lengths if they're set, without buffering bodies.
208-
if (body != kj::none && headers.get(kj::HttpHeaderId::CONTENT_LENGTH) == kj::none &&
209-
headers.get(kj::HttpHeaderId::TRANSFER_ENCODING) == kj::none) {
210-
// We can't use headers.set() here as headers is marked const. Instead, we call set() on the
211-
// JavaScript headers object, ignoring the REQUEST guard that usually makes them immutable.
207+
if (body != kj::none && newHeaders.get(kj::HttpHeaderId::CONTENT_LENGTH) == kj::none &&
208+
newHeaders.get(kj::HttpHeaderId::TRANSFER_ENCODING) == kj::none) {
212209
KJ_IF_SOME(l, requestBody.tryGetLength()) {
213-
jsHeaders->setUnguarded(
214-
js, jsg::ByteString(kj::str("Content-Length")), jsg::ByteString(kj::str(l)));
210+
newHeaders.set(kj::HttpHeaderId::CONTENT_LENGTH, kj::str(l));
215211
} else {
216-
jsHeaders->setUnguarded(
217-
js, jsg::ByteString(kj::str("Transfer-Encoding")), jsg::ByteString(kj::str("chunked")));
212+
newHeaders.setPtr(kj::HttpHeaderId::TRANSFER_ENCODING, "chunked");
218213
}
219214
}
220215

@@ -228,7 +223,8 @@ kj::Promise<DeferredProxy<void>> ServiceWorkerGlobalScope::request(kj::HttpMetho
228223
js.alloc<Fetcher>(IoContext::NEXT_CLIENT_CHANNEL, Fetcher::RequiresHostAndProtocol::YES);
229224
}
230225

231-
auto jsRequest = js.alloc<Request>(js, method, url, Request::Redirect::MANUAL, kj::mv(jsHeaders),
226+
auto jsRequest = js.alloc<Request>(js, method, url, Request::Redirect::MANUAL,
227+
js.alloc<Headers>(js, newHeaders, Headers::Guard::REQUEST),
232228
KJ_ASSERT_NONNULL(defaultFetcher).addRef(),
233229
/* signal */ kj::mv(abortSignal), kj::mv(cf), kj::mv(body),
234230
/* thisSignal */ kj::none, Request::CacheMode::NONE);

0 commit comments

Comments
 (0)