Skip to content

Commit 6789c43

Browse files
Merge pull request #6070 from cloudflare/harris/2026-02-13-fix-form-data-warning
fix: revive dead FormData Content-Type warning (#6067)
2 parents 463132d + 56aeaf3 commit 6789c43

File tree

11 files changed

+166
-26
lines changed

11 files changed

+166
-26
lines changed

src/workerd/api/cache.c++

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ jsg::Promise<void> Cache::put(jsg::Lock& js,
335335
// this behavior. Silently discarding 304s maintains backwards compatibility and is actually
336336
// still spec-conformant.
337337

338-
if (context.isInspectorEnabled()) {
338+
if (context.hasWarningHandler()) {
339339
context.logWarning(
340340
"Ignoring attempt to Cache.put() a 304 status response. 304 responses "
341341
"are not meaningful to cache, and a potential source of bugs. Consider validating that "

src/workerd/api/headers.c++

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ static_assert(std::size(COMMON_HEADER_NAMES) == (Headers::MAX_COMMON_HEADER_ID +
275275

276276
void maybeWarnIfBadHeaderString(kj::StringPtr name, kj::StringPtr str) {
277277
KJ_IF_SOME(context, IoContext::tryCurrent()) {
278-
if (context.isInspectorEnabled()) {
278+
if (context.hasWarningHandler()) {
279279
if (!simdutf::validate_ascii(str.begin(), str.size())) {
280280
// The string contains non-ASCII characters. While any 8-bit value is technically valid
281281
// in HTTP headers, we encode header strings as UTF-8, so we want to warn the user that

src/workerd/api/http.c++

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -174,14 +174,16 @@ Body::Body(jsg::Lock& js, kj::Maybe<ExtractedBody> init, Headers& headers)
174174
// The spec allows the user to override the Content-Type, if they wish, so we only set
175175
// the Content-Type if it doesn't already exist.
176176
headers.setCommon(capnp::CommonHeaderName::CONTENT_TYPE, kj::mv(ct));
177-
} else if (MimeType::FORM_DATA == ct) {
178-
// Custom content-type request/responses with FormData are broken since they require a
179-
// boundary parameter only the FormData serializer can provide. Let's warn if a dev does this.
180-
IoContext::current().logWarning(
181-
"A FormData body was provided with a custom Content-Type header when constructing "
182-
"a Request or Response object. This will prevent the recipient of the Request or "
183-
"Response from being able to parse the body. Consider omitting the custom "
184-
"Content-Type header.");
177+
} else KJ_IF_SOME(parsed, MimeType::tryParse(ct)) {
178+
if (MimeType::FORM_DATA == parsed) {
179+
// Custom content-type request/responses with FormData are broken since they require a
180+
// boundary parameter only the FormData serializer can provide. Let's warn if a dev does this.
181+
IoContext::current().logWarning(
182+
"A FormData body was provided with a custom Content-Type header when constructing "
183+
"a Request or Response object. This will prevent the recipient of the Request or "
184+
"Response from being able to parse the body. Consider omitting the custom "
185+
"Content-Type header.");
186+
}
185187
}
186188
}
187189
return kj::mv(i.impl);
@@ -271,7 +273,7 @@ jsg::Promise<kj::String> Body::text(jsg::Lock& js) {
271273
// search-and-replace across your whole site and you forgot that it'll apply to images too.
272274
// When running in the fiddle, let's warn the developer if they do this.
273275
auto& context = IoContext::current();
274-
if (context.isInspectorEnabled()) {
276+
if (context.hasWarningHandler()) {
275277
KJ_IF_SOME(type, headersRef.getCommon(js, capnp::CommonHeaderName::CONTENT_TYPE)) {
276278
maybeWarnIfNotText(js, type);
277279
}
@@ -1058,7 +1060,7 @@ jsg::Ref<Response> Response::constructor(jsg::Lock& js,
10581060
"Response with null body status (101, 204, 205, or 304) cannot have a body.");
10591061

10601062
auto& context = IoContext::current();
1061-
if (context.isInspectorEnabled()) {
1063+
if (context.hasWarningHandler()) {
10621064
context.logWarning(kj::str("Constructing a Response with a null body status (", statusCode,
10631065
") and a non-null, "
10641066
"zero-length body. This is technically incorrect, and we recommend you update your "

src/workerd/api/streams/internal.c++

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ namespace {
2323
// papers over is fixed.
2424
[[noreturn]] void throwTypeErrorAndConsoleWarn(kj::StringPtr message) {
2525
KJ_IF_SOME(context, IoContext::tryCurrent()) {
26-
if (context.isInspectorEnabled()) {
26+
if (context.hasWarningHandler()) {
2727
context.logWarning(message);
2828
}
2929
}
@@ -918,7 +918,7 @@ ReadableStreamController::Tee ReadableStreamInternalController::tee(jsg::Lock& j
918918
auto makeTee = [&](kj::Own<ReadableStreamSource> b1,
919919
kj::Own<ReadableStreamSource> b2) -> Tee {
920920
doClose(js);
921-
if (ioContext.isInspectorEnabled()) {
921+
if (ioContext.hasWarningHandler()) {
922922
b1 = kj::heap<WarnIfUnusedStream>(js, kj::mv(b1), ioContext);
923923
b2 = kj::heap<WarnIfUnusedStream>(js, kj::mv(b2), ioContext);
924924
}

src/workerd/api/tests/BUILD.bazel

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,15 @@ wd_test(
270270
data = ["form-data-test-ts.ts"],
271271
)
272272

273+
wd_test(
274+
src = "warnings-test.wd-test",
275+
args = ["--experimental"],
276+
data = [
277+
"warnings-tail.js",
278+
"warnings-test.js",
279+
],
280+
)
281+
273282
wd_test(
274283
size = "large",
275284
src = "global-scope-test.wd-test",
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Copyright (c) 2026 Cloudflare, Inc.
2+
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
3+
// https://opensource.org/licenses/Apache-2.0
4+
import * as assert from 'node:assert';
5+
6+
// ================================================================================================
7+
// Expected warnings registry.
8+
//
9+
// Add new entries here when introducing logWarning() calls. Each entry is a substring that must
10+
// appear in at least one warn-level tail event's message. The trigger worker (warnings-test.js)
11+
// must exercise a code path that produces each warning.
12+
//
13+
// NOTE: Some logWarning() calls go through jsg::Lock::logWarning() instead of
14+
// IoContext::logWarning(). The jsg path does not currently emit to the tracer, so those warnings
15+
// are not observable via tail workers and cannot be tested here (e.g. .text() on non-text body).
16+
const EXPECTED_WARNINGS = [
17+
// From Body::Body (http.c++) — FormData body with a custom Content-Type header.
18+
'FormData body was provided',
19+
// From Response constructor (http.c++) — null-body status with zero-length body.
20+
'Constructing a Response with a null body status',
21+
];
22+
// ================================================================================================
23+
24+
let allEvents = [];
25+
26+
export default {
27+
tailStream(event) {
28+
allEvents.push(event.event);
29+
return (event) => {
30+
allEvents.push(event.event);
31+
};
32+
},
33+
};
34+
35+
export const test = {
36+
async test() {
37+
// HACK: Tail events from the trigger worker's fetch invocation are delivered asynchronously
38+
// across service boundaries. There is no JS-level synchronization primitive to await their
39+
// arrival — a plain `await promise` doesn't drive the I/O loop that delivers them. We must
40+
// use scheduler.wait() to yield while the runtime processes pending I/O.
41+
const TIMEOUT_MS = 5000;
42+
const POLL_MS = 10;
43+
const deadline = Date.now() + TIMEOUT_MS;
44+
45+
const unseen = () =>
46+
EXPECTED_WARNINGS.filter(
47+
(substring) =>
48+
!allEvents.some(
49+
(e) =>
50+
e.type === 'log' &&
51+
e.level === 'warn' &&
52+
e.message?.[0]?.includes(substring)
53+
)
54+
);
55+
56+
while (unseen().length > 0 && Date.now() < deadline) {
57+
await scheduler.wait(POLL_MS);
58+
}
59+
60+
const missing = unseen();
61+
assert.strictEqual(
62+
missing.length,
63+
0,
64+
`${missing.length} expected warning(s) were not observed after ${TIMEOUT_MS}ms:\n` +
65+
missing.map((s) => ` - "${s}"`).join('\n')
66+
);
67+
},
68+
};
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright (c) 2026 Cloudflare, Inc.
2+
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
3+
// https://opensource.org/licenses/Apache-2.0
4+
5+
export default {
6+
async fetch(request) {
7+
// --- FormData body with custom Content-Type (http.c++) ---
8+
// The custom Content-Type will lack the boundary parameter that the FormData serializer
9+
// generates, preventing the recipient from parsing the body.
10+
const formData = new FormData();
11+
formData.append('key', 'value');
12+
const req = new Request('http://example.com', {
13+
method: 'POST',
14+
body: formData,
15+
headers: { 'Content-Type': 'multipart/form-data' },
16+
});
17+
18+
// --- Null-body status with zero-length body (http.c++) ---
19+
// Constructing a Response with a null-body status (204) and a non-null, zero-length body
20+
// is technically incorrect and should warn.
21+
const resp = new Response('', { status: 204 });
22+
23+
return new Response('ok');
24+
},
25+
};
26+
27+
export const test = {
28+
async test(ctrl, env) {
29+
// Invoke via service binding so the fetch() handler runs in a traced invocation
30+
// (test() handlers are intentionally not traced by the test runner).
31+
await env.SELF.fetch('http://dummy');
32+
},
33+
};
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
using Workerd = import "/workerd/workerd.capnp";
2+
3+
const unitTests :Workerd.Config = (
4+
services = [
5+
( name = "trigger",
6+
worker = (
7+
modules = [
8+
( name = "worker", esModule = embed "warnings-test.js" )
9+
],
10+
bindings = [
11+
( name = "SELF", service = "trigger" ),
12+
],
13+
compatibilityFlags = ["experimental", "nodejs_compat"],
14+
streamingTails = ["tail"],
15+
),
16+
),
17+
( name = "tail",
18+
worker = (
19+
modules = [
20+
( name = "worker", esModule = embed "warnings-tail.js" )
21+
],
22+
compatibilityFlags = ["experimental", "nodejs_compat"],
23+
),
24+
),
25+
],
26+
);

src/workerd/io/io-context.c++

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,11 @@ bool IoContext::isFiddle() {
394394
return thread.isFiddle();
395395
}
396396

397+
bool IoContext::hasWarningHandler() {
398+
return isInspectorEnabled() || getWorkerTracer() != kj::none ||
399+
::kj::_::Debug::shouldLog(::kj::LogSeverity::INFO);
400+
}
401+
397402
void IoContext::logWarning(kj::StringPtr description) {
398403
KJ_REQUIRE_NONNULL(currentLock).logWarning(description);
399404
}

src/workerd/io/io-context.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -326,12 +326,17 @@ class IoContext final: public kj::Refcounted, private kj::TaskSet::ErrorHandler
326326
bool isInspectorEnabled();
327327
bool isFiddle();
328328

329-
// Log a warning to the inspector. This is a no-op if the inspector is not enabled.
329+
// Returns true if there is something listening for warnings — the Chrome DevTools inspector,
330+
// a streaming tail worker tracer, or --verbose stderr logging. Use this to guard expensive
331+
// warning-message construction that should be skipped when nobody would see the result.
332+
bool hasWarningHandler();
333+
334+
// Log a warning. Emits to the Chrome DevTools inspector (if connected), stderr, and to the
335+
// streaming tail worker tracer (if active).
330336
void logWarning(kj::StringPtr description);
331337

332-
// Log a warning to the inspector. This is a no-op if the inspector is not enabled. Deduplicates
333-
// warning messages such that a single unique message will only be logged once for the lifetime of
334-
// an isolate.
338+
// Log a warning, deduplicating so that each unique message is only logged once for the lifetime
339+
// of an isolate. Emits to the same destinations as logWarning().
335340
void logWarningOnce(kj::StringPtr description);
336341

337342
// Log an internal error message. Deduplicates log messages such that a single unique message will

0 commit comments

Comments
 (0)