Skip to content

Commit b2cdb63

Browse files
jasnelljasnell
committed
fixup! Update Headers implementation
1 parent e770ff6 commit b2cdb63

File tree

2 files changed

+66
-65
lines changed

2 files changed

+66
-65
lines changed

‎src/workerd/api/headers.c++‎

Lines changed: 60 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -220,12 +220,13 @@ Headers::Headers(jsg::Lock& js, jsg::Dict<kj::String, kj::String> dict): guard(G
220220

221221
Headers::Headers(jsg::Lock& js, const Headers& other): guard(Guard::NONE) {
222222
for (kj::uint i = 0; i < other.commonHeaders.size(); i++) {
223-
commonHeaders[i] = other.commonHeaders[i].map([](const Header& h) { return h.clone(); });
223+
commonHeaders[i] =
224+
other.commonHeaders[i].map([](const kj::Own<Header>& h) { return h->clone(); });
224225
}
225226
uncommonHeaders.reserve(other.uncommonHeaders.size());
226227
for (auto& [key, header]: other.uncommonHeaders) {
227228
// It should not be possible to have duplicate keys here.
228-
uncommonHeaders.insert(kj::str(key), header.clone());
229+
uncommonHeaders.insert(kj::str(key), header->clone());
229230
}
230231
}
231232

@@ -243,6 +244,19 @@ Headers::Headers(jsg::Lock& js, const kj::HttpHeaders& other, Guard guard): guar
243244
this->guard = guard;
244245
}
245246

247+
kj::Maybe<Headers::Header&> Headers::tryGetHeader(const HeaderKey& key) {
248+
KJ_SWITCH_ONEOF(key) {
249+
KJ_CASE_ONEOF(idx, kj::uint) {
250+
return commonHeaders[idx].map([](kj::Own<Header>& header) -> Header& { return *header; });
251+
}
252+
KJ_CASE_ONEOF(name, kj::String) {
253+
return uncommonHeaders.find(name).map(
254+
[](kj::Own<Header>& header) -> Header& { return *header; });
255+
}
256+
}
257+
KJ_UNREACHABLE;
258+
}
259+
246260
jsg::Ref<Headers> Headers::clone(jsg::Lock& js) const {
247261
auto result = js.alloc<Headers>(js, *this);
248262
result->guard = guard;
@@ -254,26 +268,26 @@ jsg::Ref<Headers> Headers::clone(jsg::Lock& js) const {
254268
void Headers::shallowCopyTo(kj::HttpHeaders& out) {
255269
for (kj::uint i = 1; i < commonHeaders.size(); i++) {
256270
KJ_IF_SOME(header, commonHeaders[i]) {
257-
KJ_IF_SOME(name, header.name) {
258-
for (auto& value: header.values) {
271+
KJ_IF_SOME(name, header->name) {
272+
for (auto& value: header->values) {
259273
out.addPtrPtr(name, value);
260274
}
261275
} else {
262276
auto name = getCommonHeaderName(i);
263-
for (auto& value: header.values) {
277+
for (auto& value: header->values) {
264278
out.addPtrPtr(name, value);
265279
}
266280
}
267281
}
268282
}
269283

270284
for (auto& header: uncommonHeaders) {
271-
KJ_IF_SOME(name, header.value.name) {
272-
for (auto& value: header.value.values) {
285+
KJ_IF_SOME(name, header.value->name) {
286+
for (auto& value: header.value->values) {
273287
out.addPtrPtr(name, value);
274288
}
275289
} else {
276-
for (auto& value: header.value.values) {
290+
for (auto& value: header.value->values) {
277291
out.addPtrPtr(header.key, value);
278292
}
279293
}
@@ -288,22 +302,22 @@ kj::Array<Headers::DisplayedHeader> Headers::getDisplayedHeaders(jsg::Lock& js)
288302
for (kj::uint i = 1; i < commonHeaders.size(); i++) {
289303
KJ_IF_SOME(header, commonHeaders[i]) {
290304
if (getSetCookie && i == static_cast<uint>(capnp::CommonHeaderName::SET_COOKIE)) {
291-
reserved += header.values.size();
305+
reserved += header->values.size();
292306
} else {
293307
reserved += 1;
294308
}
295309
}
296310
}
297311
for (auto& header: uncommonHeaders) {
298-
reserved += header.value.values.size();
312+
reserved += header.value->values.size();
299313
}
300314
kj::Vector<Headers::DisplayedHeader> vec(reserved);
301315

302316
for (kj::uint i = 1; i < commonHeaders.size(); i++) {
303317
auto name = getCommonHeaderName(i);
304318
KJ_IF_SOME(header, commonHeaders[i]) {
305319
if (getSetCookie && i == static_cast<uint>(capnp::CommonHeaderName::SET_COOKIE)) {
306-
for (auto& value: header.values) {
320+
for (auto& value: header->values) {
307321
vec.add(Headers::DisplayedHeader{
308322
.key = kj::str(name),
309323
.value = kj::str(value),
@@ -312,7 +326,7 @@ kj::Array<Headers::DisplayedHeader> Headers::getDisplayedHeaders(jsg::Lock& js)
312326
} else {
313327
vec.add(Headers::DisplayedHeader{
314328
.key = kj::str(name),
315-
.value = kj::strArray(header.values, ", "),
329+
.value = kj::strArray(header->values, ", "),
316330
});
317331
}
318332
}
@@ -321,7 +335,7 @@ kj::Array<Headers::DisplayedHeader> Headers::getDisplayedHeaders(jsg::Lock& js)
321335
for (auto& header: uncommonHeaders) {
322336
vec.add(Headers::DisplayedHeader{
323337
.key = kj::str(header.key),
324-
.value = kj::strArray(header.value.values, ", "),
338+
.value = kj::strArray(header.value->values, ", "),
325339
});
326340
}
327341

@@ -380,28 +394,21 @@ kj::Maybe<kj::String> Headers::get(jsg::Lock& js, kj::String name) {
380394
}
381395

382396
kj::Maybe<kj::String> Headers::getUnguarded(jsg::Lock&, kj::StringPtr name) {
383-
KJ_SWITCH_ONEOF(getHeaderKeyFor(name)) {
384-
KJ_CASE_ONEOF(id, kj::uint) {
385-
return commonHeaders[id].map([](auto& header) { return kj::strArray(header.values, ", "); });
386-
}
387-
KJ_CASE_ONEOF(name, kj::String) {
388-
return uncommonHeaders.find(name).map(
389-
[](auto& header) { return kj::strArray(header.values, ", "); });
390-
}
391-
}
392-
KJ_UNREACHABLE;
397+
return tryGetHeader(getHeaderKeyFor(name)).map([](Header& header) {
398+
return kj::strArray(header.values, ", ");
399+
});
393400
}
394401

395402
kj::Maybe<kj::String> Headers::getCommon(jsg::Lock& js, capnp::CommonHeaderName idx) {
396403
kj::uint index = static_cast<kj::uint>(idx);
397404
KJ_DASSERT(index <= Headers::MAX_COMMON_HEADER_ID);
398-
return commonHeaders[index].map([](auto& header) { return kj::strArray(header.values, ", "); });
405+
return commonHeaders[index].map([](auto& header) { return kj::strArray(header->values, ", "); });
399406
}
400407

401408
kj::Array<kj::StringPtr> Headers::getSetCookie() {
402409
auto& header = commonHeaders[static_cast<kj::uint>(capnp::CommonHeaderName::SET_COOKIE)];
403410
KJ_IF_SOME(h, header) {
404-
return KJ_MAP(value, h.values) { return value.asPtr(); };
411+
return KJ_MAP(value, h->values) { return value.asPtr(); };
405412
}
406413
return nullptr;
407414
}
@@ -418,15 +425,7 @@ kj::Array<kj::StringPtr> Headers::getAll(kj::String name) {
418425
}
419426

420427
bool Headers::has(kj::String name) {
421-
KJ_SWITCH_ONEOF(getHeaderKeyFor(name)) {
422-
KJ_CASE_ONEOF(id, kj::uint) {
423-
return commonHeaders[id] != kj::none;
424-
}
425-
KJ_CASE_ONEOF(name, kj::String) {
426-
return uncommonHeaders.find(name) != kj::none;
427-
}
428-
}
429-
KJ_UNREACHABLE;
428+
return tryGetHeader(getHeaderKeyFor(name)) != kj::none;
430429
}
431430

432431
bool Headers::hasCommon(capnp::CommonHeaderName idx) {
@@ -444,15 +443,15 @@ void Headers::setUnguarded(jsg::Lock& js, kj::String name, kj::String value) {
444443
KJ_SWITCH_ONEOF(getHeaderKeyFor(name)) {
445444
KJ_CASE_ONEOF(id, kj::uint) {
446445
KJ_IF_SOME(existing, commonHeaders[id]) {
447-
existing.values.resize(1);
448-
existing.values[0] = kj::mv(value);
446+
existing->values.resize(1);
447+
existing->values[0] = kj::mv(value);
449448
} else {
450-
auto& created = commonHeaders[id].emplace();
449+
auto& created = commonHeaders[id].emplace(kj::heap(Header()));
451450
if (name != getCommonHeaderName(id)) {
452-
created.name = kj::mv(name);
451+
created->name = kj::mv(name);
453452
}
454-
created.values.resize(1);
455-
created.values[0] = kj::mv(value);
453+
created->values.resize(1);
454+
created->values[0] = kj::mv(value);
456455
}
457456
return;
458457
}
@@ -463,11 +462,11 @@ void Headers::setUnguarded(jsg::Lock& js, kj::String name, kj::String value) {
463462
if (name != n) maybeName = kj::mv(name);
464463
return Ret{
465464
.key = kj::mv(n),
466-
.value = Header(kj::mv(maybeName)),
465+
.value = kj::heap(Header(kj::mv(maybeName))),
467466
};
468467
});
469-
header.values.resize(1);
470-
header.values[0] = kj::mv(value);
468+
header->values.resize(1);
469+
header->values[0] = kj::mv(value);
471470
return;
472471
}
473472
}
@@ -478,12 +477,12 @@ void Headers::setCommon(capnp::CommonHeaderName idx, kj::String value) {
478477
kj::uint index = static_cast<kj::uint>(idx);
479478
KJ_DASSERT(index <= Headers::MAX_COMMON_HEADER_ID);
480479
KJ_IF_SOME(existing, commonHeaders[index]) {
481-
existing.values.resize(1);
482-
existing.values[0] = kj::mv(value);
480+
existing->values.resize(1);
481+
existing->values[0] = kj::mv(value);
483482
} else {
484-
auto& created = commonHeaders[index].emplace();
485-
created.values.resize(1);
486-
created.values[0] = kj::mv(value);
483+
auto& created = commonHeaders[index].emplace(kj::heap(Header()));
484+
created->values.resize(1);
485+
created->values[0] = kj::mv(value);
487486
}
488487
}
489488

@@ -496,31 +495,31 @@ void Headers::appendUnguarded(jsg::Lock& js, kj::String name, kj::String value)
496495
KJ_SWITCH_ONEOF(getHeaderKeyFor(name)) {
497496
KJ_CASE_ONEOF(id, kj::uint) {
498497
KJ_IF_SOME(existing, commonHeaders[id]) {
499-
existing.values.add(kj::mv(value));
498+
existing->values.add(kj::mv(value));
500499
} else {
501-
auto& created = commonHeaders[id].emplace();
500+
auto& created = commonHeaders[id].emplace(kj::heap(Header()));
502501
if (name != getCommonHeaderName(id)) {
503-
created.name = kj::mv(name);
502+
created->name = kj::mv(name);
504503
}
505-
created.values.resize(1);
506-
created.values[0] = kj::mv(value);
504+
created->values.resize(1);
505+
created->values[0] = kj::mv(value);
507506
}
508507
return;
509508
}
510509
KJ_CASE_ONEOF(n, kj::String) {
511510
KJ_IF_SOME(existing, uncommonHeaders.find(n)) {
512-
existing.values.add(kj::mv(value));
511+
existing->values.add(kj::mv(value));
513512
} else {
514513
using Ret = typename decltype(uncommonHeaders)::Entry;
515514
auto& header = uncommonHeaders.findOrCreate(n, [&] -> Ret {
516515
kj::Maybe<kj::String> maybeName;
517516
if (name != n) maybeName = kj::mv(name);
518517
return Ret{
519518
.key = kj::mv(n),
520-
.value = Header(kj::mv(maybeName)),
519+
.value = kj::heap(Header(kj::mv(maybeName))),
521520
};
522521
});
523-
header.values.add(kj::mv(value));
522+
header->values.add(kj::mv(value));
524523
}
525524
return;
526525
}
@@ -654,32 +653,32 @@ void Headers::serialize(jsg::Lock& js, jsg::Serializer& serializer) {
654653
uint count = 0;
655654
for (auto& header: commonHeaders) {
656655
KJ_IF_SOME(h, header) {
657-
count += h.values.size();
656+
count += h->values.size();
658657
}
659658
}
660659
for (auto& header: uncommonHeaders) {
661-
count += header.value.values.size();
660+
count += header.value->values.size();
662661
}
663662
serializer.writeRawUint32(count);
664663

665664
// Now write key/values.
666665
for (kj::uint i = 1; i < commonHeaders.size(); i++) {
667666
KJ_IF_SOME(header, commonHeaders[i]) {
668-
for (auto& value: header.values) {
667+
for (auto& value: header->values) {
669668
serializer.writeRawUint32(i);
670669
serializer.writeLengthDelimited(value);
671670
}
672671
}
673672
}
674673
for (auto& header: uncommonHeaders) {
675674
auto name = ([&] -> kj::StringPtr {
676-
KJ_IF_SOME(name, header.value.name) {
675+
KJ_IF_SOME(name, header.value->name) {
677676
return name;
678677
} else {
679678
return header.key;
680679
}
681680
})();
682-
for (auto& value: header.value.values) {
681+
for (auto& value: header.value->values) {
683682
serializer.writeRawUint32(0);
684683
serializer.writeLengthDelimited(name);
685684
serializer.writeLengthDelimited(value);

‎src/workerd/api/headers.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,11 @@ class Headers final: public jsg::Object {
173173
values.reserve(1);
174174
}
175175

176-
Header clone() const {
176+
kj::Own<Header> clone() const {
177177
Header header;
178178
header.name = name.map([](const kj::String& s) { return kj::str(s); });
179179
header.values = KJ_MAP(v, values) { return kj::str(v); };
180-
return header;
180+
return kj::heap(kj::mv(header));
181181
}
182182

183183
JSG_MEMORY_INFO(Header) {
@@ -189,13 +189,15 @@ class Headers final: public jsg::Object {
189189
};
190190

191191
// This wastes one slot, but it is a fixed array for fast access.
192-
kj::FixedArray<kj::Maybe<Header>, MAX_COMMON_HEADER_ID + 1> commonHeaders;
192+
kj::FixedArray<kj::Maybe<kj::Own<Header>>, MAX_COMMON_HEADER_ID + 1> commonHeaders;
193193

194194
// The key is always lower-case.
195-
kj::HashMap<kj::String, Header> uncommonHeaders;
195+
kj::HashMap<kj::String, kj::Own<Header>> uncommonHeaders;
196196

197197
Guard guard;
198198

199+
kj::Maybe<Header&> tryGetHeader(const HeaderKey& key);
200+
199201
void checkGuard() {
200202
JSG_REQUIRE(guard == Guard::NONE, TypeError, "Can't modify immutable headers.");
201203
}

0 commit comments

Comments
 (0)