From e7c7e71c7c54d6f74cc80f3f10bb8daa14873933 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 4 Oct 2024 10:45:18 -0700 Subject: [PATCH] Add compat flag to add toStringTag --- src/workerd/api/BUILD.bazel | 6 ++++++ src/workerd/api/tests/global-scope-test.wd-test | 2 +- src/workerd/api/tests/no-to-string-tag-test.js | 8 ++++++++ .../api/tests/no-to-string-tag-test.wd-test | 15 +++++++++++++++ src/workerd/io/compatibility-date.capnp | 9 +++++++++ src/workerd/io/worker.c++ | 3 +++ src/workerd/jsg/jsg-test.c++ | 2 -- src/workerd/jsg/jsg.c++ | 4 ++++ src/workerd/jsg/jsg.h | 1 + src/workerd/jsg/resource.h | 5 ++++- src/workerd/jsg/setup.h | 9 +++++++++ src/workerd/jsg/util.c++ | 5 +++++ src/workerd/jsg/util.h | 1 + 13 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 src/workerd/api/tests/no-to-string-tag-test.js create mode 100644 src/workerd/api/tests/no-to-string-tag-test.wd-test diff --git a/src/workerd/api/BUILD.bazel b/src/workerd/api/BUILD.bazel index 45492327a46..44e1e46964f 100644 --- a/src/workerd/api/BUILD.bazel +++ b/src/workerd/api/BUILD.bazel @@ -547,3 +547,9 @@ wd_test( args = ["--experimental"], data = ["tests/error-in-error-event-test.js"], ) + +wd_test( + src = "tests/no-to-string-tag-test.wd-test", + args = ["--experimental"], + data = ["tests/no-to-string-tag-test.js"], +) diff --git a/src/workerd/api/tests/global-scope-test.wd-test b/src/workerd/api/tests/global-scope-test.wd-test index 13783d6aafa..1199c1511a1 100644 --- a/src/workerd/api/tests/global-scope-test.wd-test +++ b/src/workerd/api/tests/global-scope-test.wd-test @@ -8,7 +8,7 @@ const unitTests :Workerd.Config = ( (name = "worker", esModule = embed "global-scope-test.js") ], compatibilityDate = "2023-01-15", - compatibilityFlags = ["nodejs_compat"] + compatibilityFlags = ["nodejs_compat", "set_tostring_tag"] ) ), ], diff --git a/src/workerd/api/tests/no-to-string-tag-test.js b/src/workerd/api/tests/no-to-string-tag-test.js new file mode 100644 index 00000000000..2b923091742 --- /dev/null +++ b/src/workerd/api/tests/no-to-string-tag-test.js @@ -0,0 +1,8 @@ +import { strictEqual } from 'node:assert'; + +export default { + test() { + const h = new Headers(); + strictEqual(h[Symbol.toStringTag], undefined); + }, +}; diff --git a/src/workerd/api/tests/no-to-string-tag-test.wd-test b/src/workerd/api/tests/no-to-string-tag-test.wd-test new file mode 100644 index 00000000000..2219e3f5341 --- /dev/null +++ b/src/workerd/api/tests/no-to-string-tag-test.wd-test @@ -0,0 +1,15 @@ +using Workerd = import "/workerd/workerd.capnp"; + +const unitTests :Workerd.Config = ( + services = [ + ( name = "no-to-string-tag-test", + worker = ( + modules = [ + (name = "worker", esModule = embed "no-to-string-tag-test.js") + ], + compatibilityDate = "2023-01-15", + compatibilityFlags = ["nodejs_compat"] + ) + ), + ], +); diff --git a/src/workerd/io/compatibility-date.capnp b/src/workerd/io/compatibility-date.capnp index d35e7f18b30..71b422ecf2f 100644 --- a/src/workerd/io/compatibility-date.capnp +++ b/src/workerd/io/compatibility-date.capnp @@ -622,4 +622,13 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef { # We plan to turn this on always quite soon. It would be an autogate but we need to test # our logic both at upload time and at runtime, and this seemed like the easiest way to # make sure we keep things in sync. + + setToStringTag @64 :Bool + $compatEnableFlag("set_tostring_tag") + $compatDisableFlag("do_not_set_tostring_tag") + $compatEnableDate("2024-09-26"); + # A change was made that set the Symbol.toStringTag on all jsg::Objects in order to + # fix several spec compliance bugs. Unfortunately it turns out that was more breaking + # than expected. This flag restores the original behavior for compat dates before + # 2024-09-26 } diff --git a/src/workerd/io/worker.c++ b/src/workerd/io/worker.c++ index 75fcb3e5d31..9111eb48f08 100644 --- a/src/workerd/io/worker.c++ +++ b/src/workerd/io/worker.c++ @@ -1028,6 +1028,9 @@ Worker::Isolate::Isolate(kj::Own apiParam, lock->setCaptureThrowsAsRejections(features.getCaptureThrowsAsRejections()); lock->setCommonJsExportDefault(features.getExportCommonJsDefaultNamespace()); + if (features.getSetToStringTag()) { + lock->setToStringTag(); + } if (features.getNodeJsCompatV2()) { lock->setNodeJsCompatEnabled(); } diff --git a/src/workerd/jsg/jsg-test.c++ b/src/workerd/jsg/jsg-test.c++ index 88f40e59e6b..8a875225cbf 100644 --- a/src/workerd/jsg/jsg-test.c++ +++ b/src/workerd/jsg/jsg-test.c++ @@ -115,8 +115,6 @@ KJ_TEST("inheritance") { e.expectEval("newExtendedAsBase(123, 'foo') instanceof NumberBox", "boolean", "true"); e.expectEval("newExtendedAsBase(123, 'foo') instanceof ExtendedNumberBox", "boolean", "true"); - - e.expectEval("(new Other())[Symbol.toStringTag]", "string", "Other"); } // ======================================================================================== diff --git a/src/workerd/jsg/jsg.c++ b/src/workerd/jsg/jsg.c++ index f5c3c520bde..05e346f2af0 100644 --- a/src/workerd/jsg/jsg.c++ +++ b/src/workerd/jsg/jsg.c++ @@ -195,6 +195,10 @@ void Lock::setNodeJsCompatEnabled() { IsolateBase::from(v8Isolate).setNodeJsCompatEnabled({}, true); } +void Lock::setToStringTag() { + IsolateBase::from(v8Isolate).enableSetToStringTag(); +} + void Lock::setCommonJsExportDefault(bool exportDefault) { IsolateBase::from(v8Isolate).setCommonJsExportDefault({}, exportDefault); } diff --git a/src/workerd/jsg/jsg.h b/src/workerd/jsg/jsg.h index 5c9b2d5bb0a..c0a17b604a5 100644 --- a/src/workerd/jsg/jsg.h +++ b/src/workerd/jsg/jsg.h @@ -2514,6 +2514,7 @@ class Lock { void setCommonJsExportDefault(bool exportDefault); void setNodeJsCompatEnabled(); + void setToStringTag(); using Logger = void(Lock&, kj::StringPtr); void setLoggerCallback(kj::Function&& logger); diff --git a/src/workerd/jsg/resource.h b/src/workerd/jsg/resource.h index 8a287ceb4b5..97ab96fb7e0 100644 --- a/src/workerd/jsg/resource.h +++ b/src/workerd/jsg/resource.h @@ -1533,7 +1533,10 @@ class ResourceWrapper { auto classname = v8StrIntern(isolate, typeName(typeid(T))); - prototype->Set(v8::Symbol::GetToStringTag(isolate), classname, v8::PropertyAttribute::DontEnum); + if (getShouldSetToStringTag(isolate)) { + prototype->Set( + v8::Symbol::GetToStringTag(isolate), classname, v8::PropertyAttribute::DontEnum); + } // Previously, miniflare would use the lack of a Symbol.toStringTag on a class to // detect a type that came from the runtime. That's obviously a bit problematic because diff --git a/src/workerd/jsg/setup.h b/src/workerd/jsg/setup.h index 12fda5a5c94..3990d2d0180 100644 --- a/src/workerd/jsg/setup.h +++ b/src/workerd/jsg/setup.h @@ -143,6 +143,14 @@ class IsolateBase { return nodeJsCompatEnabled; } + inline bool shouldSetToStringTag() const { + return setToStringTag; + } + + void enableSetToStringTag() { + setToStringTag = true; + } + // The logger will be optionally set by the isolate setup logic if there is anywhere // for the log to go (for instance, if debug logging is enabled or the inspector is // being used). @@ -229,6 +237,7 @@ class IsolateBase { bool exportCommonJsDefault = false; bool asyncContextTrackingEnabled = false; bool nodeJsCompatEnabled = false; + bool setToStringTag = false; kj::Maybe> maybeLogger; kj::Maybe> maybeErrorReporter; diff --git a/src/workerd/jsg/util.c++ b/src/workerd/jsg/util.c++ index 74d54908fc5..2a303ba5d88 100644 --- a/src/workerd/jsg/util.c++ +++ b/src/workerd/jsg/util.c++ @@ -29,6 +29,11 @@ bool getCommonJsExportDefault(v8::Isolate* isolate) { return jsgIsolate.getCommonJsExportDefault(); } +bool getShouldSetToStringTag(v8::Isolate* isolate) { + auto& jsgIsolate = *reinterpret_cast(isolate->GetData(0)); + return jsgIsolate.shouldSetToStringTag(); +} + #if _WIN32 kj::String fullyQualifiedTypeName(const std::type_info& type) { // type.name() returns a human-readable name on Windows: diff --git a/src/workerd/jsg/util.h b/src/workerd/jsg/util.h index 30511c9d63e..458e9e0c8cf 100644 --- a/src/workerd/jsg/util.h +++ b/src/workerd/jsg/util.h @@ -45,6 +45,7 @@ class JsExceptionThrown: public std::exception { bool getCaptureThrowsAsRejections(v8::Isolate* isolate); bool getCommonJsExportDefault(v8::Isolate* isolate); +bool getShouldSetToStringTag(v8::Isolate* isolate); kj::String fullyQualifiedTypeName(const std::type_info& type); kj::String typeName(const std::type_info& type);