Skip to content

Commit

Permalink
Merge pull request #1616 from cloudflare/jsnell/fixup-text-warning
Browse files Browse the repository at this point in the history
  • Loading branch information
jasnell committed Feb 5, 2024
2 parents 7ac486e + 573e4a2 commit 6283473
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 15 deletions.
2 changes: 1 addition & 1 deletion src/workerd/api/http.c++
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ jsg::Promise<kj::String> Body::text(jsg::Lock& js) {
auto& context = IoContext::current();
if (context.isInspectorEnabled()) {
KJ_IF_SOME(type, headersRef.get(jsg::ByteString(kj::str("Content-Type")))) {
maybeWarnIfNotText(type);
maybeWarnIfNotText(js, type);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/r2-bucket.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,7 @@ jsg::Promise<kj::String> R2Bucket::GetResult::text(jsg::Lock& js) {
if (context.isInspectorEnabled()) {
// httpMetadata can't be null because GetResult always populates it.
KJ_IF_SOME(type, KJ_REQUIRE_NONNULL(httpMetadata).contentType) {
maybeWarnIfNotText(type);
maybeWarnIfNotText(js, type);
}
}

Expand Down
20 changes: 8 additions & 12 deletions src/workerd/api/util.c++
Original file line number Diff line number Diff line change
Expand Up @@ -225,20 +225,16 @@ kj::Maybe<jsg::V8Ref<v8::Object>> cloneRequestCf(
return kj::none;
}

void maybeWarnIfNotText(kj::StringPtr str) {
void maybeWarnIfNotText(jsg::Lock& js, kj::StringPtr str) {
KJ_IF_SOME(parsed, MimeType::tryParse(str)) {
if (MimeType::isText(parsed)) return;
}
// A common mistake is to call .text() on non-text content, e.g. because you're implementing a
// search-and-replace across your whole site and you forgot that it'll apply to images too.
// When running in the fiddle, let's warn the developer if they do this.
if (!str.startsWith("text/") &&
!str.endsWith("charset=UTF-8") &&
!str.endsWith("charset=utf-8") &&
!str.endsWith("xml") &&
!str.endsWith("json") &&
!str.endsWith("javascript")) {
IoContext::current().logWarning(kj::str(
"Called .text() on an HTTP body which does not appear to be text. The body's "
"Content-Type is \"", str, "\". The result will probably be corrupted. Consider "
"checking the Content-Type header before interpreting entities as text."));
}
js.logWarning(kj::str(
"Called .text() on an HTTP body which does not appear to be text. The body's "
"Content-Type is \"", str, "\". The result will probably be corrupted. Consider "
"checking the Content-Type header before interpreting entities as text."));
}
} // namespace workerd::api
2 changes: 1 addition & 1 deletion src/workerd/api/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,6 @@ double dateNow();

// =======================================================================================

void maybeWarnIfNotText(kj::StringPtr str);
void maybeWarnIfNotText(jsg::Lock& js, kj::StringPtr str);

} // namespace workerd::api
7 changes: 7 additions & 0 deletions src/workerd/util/mimetype-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,13 @@ KJ_TEST("WHATWG tests") {
KJ_ASSERT(MimeType::isJavascript(MimeType::JAVASCRIPT));
KJ_ASSERT(MimeType::isJavascript(MimeType::XJAVASCRIPT));
KJ_ASSERT(MimeType::isJavascript(MimeType::TEXT_JAVASCRIPT));

KJ_ASSERT(MimeType::isText(MimeType::PLAINTEXT));
KJ_ASSERT(MimeType::isText(MimeType::JSON));
KJ_ASSERT(MimeType::isText(MimeType::JAVASCRIPT));
KJ_ASSERT(MimeType::isText(MimeType::XJAVASCRIPT));
KJ_ASSERT(MimeType::isText(
KJ_ASSERT_NONNULL(MimeType::tryParse("application/json; charset=\"utf-8\""))));
}

} // namespace
Expand Down
8 changes: 8 additions & 0 deletions src/workerd/util/mimetype.c++
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,14 @@ const MimeType MimeType::MANIFEST_JSON = MimeType("application"_kj, "manifest+js
const MimeType MimeType::VTT = MimeType("text"_kj, "vtt"_kj);
const MimeType MimeType::EVENT_STREAM = MimeType("text"_kj, "event-stream"_kj);

bool MimeType::isText(const MimeType& mimeType) {
auto type = mimeType.type();
return type == "text" ||
isXml(mimeType) ||
isJson(mimeType) ||
isJavascript(mimeType);
}

bool MimeType::isXml(const MimeType& mimeType) {
auto type = mimeType.type();
auto subtype = mimeType.subtype();
Expand Down
1 change: 1 addition & 0 deletions src/workerd/util/mimetype.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class MimeType final {
static bool isImage(const MimeType& mimeType);
static bool isVideo(const MimeType& mimeType);
static bool isAudio(const MimeType& mimeType);
static bool isText(const MimeType& mimeType);

static const MimeType JSON;
static const MimeType PLAINTEXT;
Expand Down

0 comments on commit 6283473

Please sign in to comment.