Skip to content

Commit 46b7629

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 f58cdfa commit 46b7629

File tree

1 file changed

+11
-12
lines changed

1 file changed

+11
-12
lines changed

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,6 @@ kj::Promise<DeferredProxy<void>> ServiceWorkerGlobalScope::request(kj::HttpMetho
166166

167167
CfProperty cf(cfBlobJson);
168168

169-
auto jsHeaders = js.alloc<Headers>(js, headers, Headers::Guard::REQUEST);
170169
// We do not automatically decode gzipped request bodies because the fetch() standard doesn't
171170
// specify any automatic encoding of requests. https://github.com/whatwg/fetch/issues/589
172171
auto b = newSystemStream(kj::addRef(*ownRequestBody), StreamEncoding::IDENTITY);
@@ -192,30 +191,30 @@ kj::Promise<DeferredProxy<void>> ServiceWorkerGlobalScope::request(kj::HttpMetho
192191
//
193192
// TODO(cleanup): Should KJ HTTP interfaces explicitly communicate the difference between a
194193
// missing body and an empty one?
194+
auto newHeaders = headers.cloneShallow();
195195
kj::Maybe<Body::ExtractedBody> body;
196-
if (headers.get(kj::HttpHeaderId::CONTENT_LENGTH) != kj::none ||
197-
headers.get(kj::HttpHeaderId::TRANSFER_ENCODING) != kj::none ||
196+
if (newHeaders.get(kj::HttpHeaderId::CONTENT_LENGTH) != kj::none ||
197+
newHeaders.get(kj::HttpHeaderId::TRANSFER_ENCODING) != kj::none ||
198198
requestBody.tryGetLength().orDefault(1) > 0) {
199199
body = Body::ExtractedBody(jsStream.addRef());
200200
}
201201

202202
// If the request doesn't specify "Content-Length" or "Transfer-Encoding", set "Content-Length"
203203
// to the body length if it's known. This ensures handlers for worker-to-worker requests can
204204
// access known body lengths if they're set, without buffering bodies.
205-
if (body != kj::none && headers.get(kj::HttpHeaderId::CONTENT_LENGTH) == kj::none &&
206-
headers.get(kj::HttpHeaderId::TRANSFER_ENCODING) == kj::none) {
207-
// We can't use headers.set() here as headers is marked const. Instead, we call set() on the
208-
// JavaScript headers object, ignoring the REQUEST guard that usually makes them immutable.
205+
if (body != kj::none && newHeaders.get(kj::HttpHeaderId::CONTENT_LENGTH) == kj::none &&
206+
newHeaders.get(kj::HttpHeaderId::TRANSFER_ENCODING) == kj::none) {
209207
KJ_IF_SOME(l, requestBody.tryGetLength()) {
210-
jsHeaders->setUnguarded(
211-
js, jsg::ByteString(kj::str("Content-Length")), jsg::ByteString(kj::str(l)));
208+
newHeaders.set(kj::HttpHeaderId::CONTENT_LENGTH, kj::str(l));
212209
} else {
213-
jsHeaders->setUnguarded(
214-
js, jsg::ByteString(kj::str("Transfer-Encoding")), jsg::ByteString(kj::str("chunked")));
210+
newHeaders.setPtr(kj::HttpHeaderId::TRANSFER_ENCODING, "chunked");
215211
}
216212
}
217213

218-
auto jsRequest = js.alloc<Request>(js, method, url, Request::Redirect::MANUAL, kj::mv(jsHeaders),
214+
auto jsHeaders = js.alloc<Headers>(js, newHeaders, Headers::Guard::REQUEST);
215+
216+
auto jsRequest = js.alloc<Request>(js, method, url, Request::Redirect::MANUAL,
217+
js.alloc<Headers>(js, newHeaders, Headers::Guard::REQUEST),
219218
js.alloc<Fetcher>(IoContext::NEXT_CLIENT_CHANNEL, Fetcher::RequiresHostAndProtocol::YES),
220219
/* signal */ kj::mv(abortSignal), kj::mv(cf), kj::mv(body),
221220
/* thisSignal */ kj::none, Request::CacheMode::NONE);

0 commit comments

Comments
 (0)