From c1d2a0b7b7d591b935b3054deda948da5fcf8872 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Fri, 3 Jul 2026 23:35:13 +0800 Subject: [PATCH] Pre-size FormData serialization buffer --- src/workerd/api/form-data.c++ | 64 +++++++++++++++++++++++-- src/workerd/api/tests/form-data-test.js | 13 ++++- src/workerd/tests/BUILD.bazel | 6 +++ src/workerd/tests/bench-form-data.c++ | 57 ++++++++++++++++++++++ 4 files changed, 136 insertions(+), 4 deletions(-) create mode 100644 src/workerd/tests/bench-form-data.c++ diff --git a/src/workerd/api/form-data.c++ b/src/workerd/api/form-data.c++ index 592e649fd6b..274d8383bfd 100644 --- a/src/workerd/api/form-data.c++ +++ b/src/workerd/api/form-data.c++ @@ -24,6 +24,33 @@ namespace workerd::api { namespace { +void addToSerializedSize(size_t& total, size_t amount) { + static constexpr size_t kMaxSize = kj::maxValue; + JSG_REQUIRE(amount <= kMaxSize - total, RangeError, "FormData is too large to serialize."); + total += amount; +} + +size_t getEscapedSize(kj::StringPtr value) { + JSG_REQUIRE(!value.endsWith("\\"), TypeError, "Name or filename can't end with backslash"); + + size_t result = 0; + for (char c: value) { + switch (c) { + case '\"': + case '\n': + addToSerializedSize(result, 3); + break; + case '\\': + addToSerializedSize(result, 2); + break; + default: + addToSerializedSize(result, 1); + break; + } + } + return result; +} + // Like split() in kj/compat/url.c++, but splits at a substring rather than a character. kj::ArrayPtr splitAtSubString(kj::ArrayPtr& text, kj::StringPtr subString) { // TODO(perf): Use a Boyer-Moore search? @@ -314,9 +341,39 @@ kj::Array FormData::serialize(kj::ArrayPtr boundary) { JSG_REQUIRE(boundary.size() > 0 && boundary.size() <= 70, TypeError, "Length of multipart/form-data boundary string must be in the range [1, 70]."); - // TODO(perf): We should be able to trivially calculate the length of the serialized form data - // beforehand. I tried, but apparently my math REALLY sucks and I hate memory overruns, so ... - auto builder = kj::Vector{}; + size_t serializedSize = 0; + for (auto& kv: data) { + addToSerializedSize(serializedSize, "--"_kj.size()); + addToSerializedSize(serializedSize, boundary.size()); + addToSerializedSize(serializedSize, "\r\n"_kj.size()); + addToSerializedSize(serializedSize, "Content-Disposition: form-data; name=\""_kj.size()); + addToSerializedSize(serializedSize, getEscapedSize(kv.name)); + KJ_SWITCH_ONEOF(kv.value) { + KJ_CASE_ONEOF(text, kj::String) { + addToSerializedSize(serializedSize, "\"\r\n\r\n"_kj.size()); + addToSerializedSize(serializedSize, text.size()); + } + KJ_CASE_ONEOF(file, jsg::Ref) { + addToSerializedSize(serializedSize, "\"; filename=\""_kj.size()); + addToSerializedSize(serializedSize, getEscapedSize(file->getName())); + addToSerializedSize(serializedSize, "\"\r\nContent-Type: "_kj.size()); + auto type = file->getType(); + if (type == nullptr) { + addToSerializedSize(serializedSize, MimeType::OCTET_STREAM.toString().size()); + } else { + addToSerializedSize(serializedSize, type.size()); + } + addToSerializedSize(serializedSize, "\r\n\r\n"_kj.size()); + addToSerializedSize(serializedSize, file->getData().size()); + } + } + addToSerializedSize(serializedSize, "\r\n"_kj.size()); + } + addToSerializedSize(serializedSize, "--"_kj.size()); + addToSerializedSize(serializedSize, boundary.size()); + addToSerializedSize(serializedSize, "--"_kj.size()); + + kj::Vector builder(serializedSize); for (auto& kv: data) { builder.addAll("--"_kj); @@ -349,6 +406,7 @@ kj::Array FormData::serialize(kj::ArrayPtr boundary) { builder.addAll(boundary); builder.addAll("--"_kj); + KJ_ASSERT(builder.size() == serializedSize); return builder.releaseAsArray().releaseAsBytes(); } diff --git a/src/workerd/api/tests/form-data-test.js b/src/workerd/api/tests/form-data-test.js index 9d894bc88d2..e7654ec6497 100644 --- a/src/workerd/api/tests/form-data-test.js +++ b/src/workerd/api/tests/form-data-test.js @@ -515,6 +515,11 @@ export const testFormDataSerializer = { expected.append('field0', 'part2'); expected.append('field1', 'part3'); expected.append('field-with-a-"-in-it', 'part4'); + expected.append( + 'file-field\n"\\name', + new File(['file-part'], 'file\n"\\name.txt', { type: 'text/plain' }) + ); + expected.append('default-file', new File(['default-part'], 'default.bin')); // Serialize the FormData. const response = new Response(expected); @@ -535,10 +540,16 @@ export const testFormDataSerializer = { 'field0=part2', 'field1=part3', 'field-with-a-%22-in-it=part4', + 'file-field%0A%22\\name=file%0A%22\\name.txt:text/plain:file-part', + 'default-file=default.bin:application/octet-stream:default-part', ]; const actualData = []; for (let [k, v] of actual) { - actualData.push(`${k}=${v}`); + if (v instanceof File) { + actualData.push(`${k}=${v.name}:${v.type}:${await v.text()}`); + } else { + actualData.push(`${k}=${v}`); + } } strictEqual('' + actualData.join(','), '' + expectedData.join(',')); diff --git a/src/workerd/tests/BUILD.bazel b/src/workerd/tests/BUILD.bazel index 298440ae427..4fdcd7fc25a 100644 --- a/src/workerd/tests/BUILD.bazel +++ b/src/workerd/tests/BUILD.bazel @@ -81,6 +81,12 @@ wd_cc_benchmark( ], ) +wd_cc_benchmark( + name = "bench-form-data", + srcs = ["bench-form-data.c++"], + deps = [":test-fixture"], +) + wd_cc_benchmark( name = "bench-global-scope", srcs = ["bench-global-scope.c++"], diff --git a/src/workerd/tests/bench-form-data.c++ b/src/workerd/tests/bench-form-data.c++ new file mode 100644 index 00000000000..ba55919b2fe --- /dev/null +++ b/src/workerd/tests/bench-form-data.c++ @@ -0,0 +1,57 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +#include +#include +#include + +namespace workerd { +namespace { + +struct FormDataSerialization: public benchmark::Fixture { + virtual ~FormDataSerialization() noexcept(true) {} + + void SetUp(benchmark::State& state) noexcept(true) override { + fixture = kj::heap(); + } + + void TearDown(benchmark::State& state) noexcept(true) override { + fixture = nullptr; + } + + kj::Own fixture; +}; + +BENCHMARK_DEFINE_F(FormDataSerialization, singleFile)(benchmark::State& state) { + static constexpr kj::StringPtr BOUNDARY = "0123456789abcdef0123456789abcdef"_kj; + const size_t payloadSize = state.range(0); + + fixture->runInIoContext([&](const TestFixture::Environment& env) { + auto& js = env.js; + auto bytes = jsg::JsUint8Array::create(js, payloadSize); + memset(bytes.asArrayPtr().begin(), 'x', payloadSize); + + auto file = js.alloc(js, jsg::JsBufferSource(bytes), kj::str("upload.bin"), + kj::str("application/octet-stream"), 0); + auto formData = js.alloc(); + formData->append(js, kj::str("file"), kj::mv(file), kj::none); + + for (auto _: state) { + auto serialized = formData->serialize(BOUNDARY); + benchmark::DoNotOptimize(serialized.begin()); + benchmark::ClobberMemory(); + } + }); + + state.SetBytesProcessed(state.iterations() * payloadSize); +} + +BENCHMARK_REGISTER_F(FormDataSerialization, singleFile) + ->Arg(1024) + ->Arg(1024 * 1024) + ->Arg(16 * 1024 * 1024) + ->Unit(benchmark::kMillisecond); + +} // namespace +} // namespace workerd