Fixup CfProperty lazy parsing (#1159)

src/workerd/api/BUILD.bazel

@@ -63,6 +63,7 @@ wd_cc_capnp_library(
 ) for f in glob(
     ["**/*-test.c++"],
     exclude = [
+        "cf-property-test.c++",
         "node/*-test.c++",
     ],
 )]
@@ -72,10 +73,17 @@ kj_test(
     deps = ["//src/workerd/tests:test-fixture"],
 )
 
+kj_test(
+    src = "cf-property-test.c++",
+    deps = ["//src/workerd/tests:test-fixture"],
+)
+
 [wd_test(
     src = f,
     args = ["--experimental"],
     data = [f.removesuffix(".wd-test") + ".js"],
 ) for f in glob(
     ["**/*.wd-test"],
 )]
+
+

src/workerd/api/cf-property-test.c++

@@ -0,0 +1,48 @@
+// Copyright (c) 2017-2022 Cloudflare, Inc.
+// Licensed under the Apache 2.0 license found in the LICENSE file or at:
+//     https://opensource.org/licenses/Apache-2.0
+
+#include "cf-property.h"
+#include <workerd/jsg/jsg.h>
+#include <workerd/tests/test-fixture.h>
+
+namespace workerd::api {
+namespace {
+
+KJ_TEST("Test that CfProperty is frozen by default") {
+  TestFixture fixture({
+    .mainModuleSource = R"SCRIPT(
+      export default {
+        async fetch(request) {
+          request.cf.foo = 100;
+          return new Response(`OK`);
+        },
+      };
+    )SCRIPT"_kj});
+
+  try {
+    auto result = fixture.runRequest(kj::HttpMethod::POST, "http://www.example.com"_kj, "TEST"_kj);
+    KJ_FAIL_REQUIRE("exception expected");
+  } catch (kj::Exception& e) {
+    KJ_EXPECT(e.getDescription() == "jsg.TypeError: Cannot add property foo, object is not extensible"_kj);
+  }
+}
+
+
+KJ_TEST("Test that CfProperty::deepClone returns editable object") {
+  TestFixture fixture({
+    .mainModuleSource = R"SCRIPT(
+      export default {
+        async fetch(request) {
+          const req = new Request(request);
+          req.cf.foo = 100;
+          return new Response(`OK`);
+        },
+      };
+    )SCRIPT"_kj});
+  auto result = fixture.runRequest(kj::HttpMethod::POST, "http://www.example.com"_kj, "TEST"_kj);
+  KJ_EXPECT(result.statusCode == 200);
+}
+
+}  // namespace
+}  // namespace workerd::api

src/workerd/api/cf-property.c++

@@ -109,13 +109,22 @@ kj::Maybe<kj::String> CfProperty::serialize(jsg::Lock& js) {
 }
 
 CfProperty CfProperty::deepClone(jsg::Lock& js) {
+  // By default, when CfProperty is lazily parsed, the resulting JS object
+  // will be recursively frozen, preventing edits. However, when the CfProperty
+  // is cloned and the clone is lazily parsed, the resulting JS object must not
+  // be frozen! So, to ensure that, we'll force the parse to occur here if it
+  // hasn't been parsed already, this will ensure that the clone receives the
+  // parsed object via json cloning below rather than the raw string.
+  // TODO(cleanup): With a bit of refactoring we can preserve the lazy parsing
+  // optimization through the clone. But for now, let's just do the easy thing.
+  getRef(js);
   KJ_IF_MAYBE(cf, value) {
     KJ_SWITCH_ONEOF(*cf) {
       KJ_CASE_ONEOF(parsed, jsg::JsRef<jsg::JsObject>) {
         return CfProperty(jsg::JsRef(js, parsed.getHandle(js).jsonClone(js)));
       }
       KJ_CASE_ONEOF(unparsed, kj::String) {
-        return CfProperty(unparsed.asPtr());
+        KJ_FAIL_REQUIRE("The cf property should have been lazily parsed!");
       }
     }
   }

src/workerd/tests/test-fixture.c++

@@ -345,7 +345,7 @@ TestFixture::Response TestFixture::runRequest(
         requestHeaders,
         requestBody,
         response,
-        nullptr,
+        "{}"_kj,
         env.lock,
         env.lock.getExportedHandler(nullptr, nullptr));
   });