Implement headers.getSetCookie()

Refs: whatwg/fetch#1346
cloudflare · Feb 13, 2023 · 7b7fd2e · 7b7fd2e
1 parent 5cf69f7
commit 7b7fd2e
Show file tree

Hide file tree

Showing 9 changed files with 152 additions and 41 deletions.
diff --git a/src/workerd/api/form-data.c++ b/src/workerd/api/form-data.c++
@@ -389,15 +389,21 @@ void FormData::set(kj::String name,
   }
 }
 
-jsg::Ref<FormData::EntryIterator> FormData::entries(jsg::Lock&) {
+jsg::Ref<FormData::EntryIterator> FormData::entries(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
   return jsg::alloc<EntryIterator>(IteratorState { JSG_THIS });
 }
 
-jsg::Ref<FormData::KeyIterator> FormData::keys(jsg::Lock&) {
+jsg::Ref<FormData::KeyIterator> FormData::keys(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
   return jsg::alloc<KeyIterator>(IteratorState { JSG_THIS });
 }
 
-jsg::Ref<FormData::ValueIterator> FormData::values(jsg::Lock&) {
+jsg::Ref<FormData::ValueIterator> FormData::values(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
   return jsg::alloc<ValueIterator>(IteratorState { JSG_THIS });
 }
 

diff --git a/src/workerd/api/form-data.h b/src/workerd/api/form-data.h
@@ -8,6 +8,7 @@
 #include <kj/compat/url.h>
 #include <workerd/jsg/jsg.h>
 #include "blob.h"
+#include <workerd/io/compatibility-date.capnp.h>
 
 namespace workerd::api {
 

diff --git a/src/workerd/api/http.c++ b/src/workerd/api/http.c++
@@ -161,15 +161,40 @@ bool Headers::hasLowerCase(kj::StringPtr name) {
   return headers.find(name) != headers.end();
 }
 
-kj::Array<Headers::DisplayedHeader> Headers::getDisplayedHeaders() {
-  auto headersCopy = KJ_MAP(mapEntry, headers) {
-    const auto& header = mapEntry.second;
-    return DisplayedHeader {
-      jsg::ByteString(kj::str(header.key)),
-      jsg::ByteString(kj::strArray(header.values, ", "))
+kj::Array<Headers::DisplayedHeader> Headers::getDisplayedHeaders(
+    CompatibilityFlags::Reader featureFlags) {
+
+  if (featureFlags.getHttpHeadersGetSetCookie()) {
+    kj::Vector<Headers::DisplayedHeader> copy;
+    for (auto& entry : headers) {
+      if (entry.first == "set-cookie") {
+        // For set-cookie entries, we iterate each individually without
+        // combining them.
+        for (auto& value : entry.second.values) {
+          copy.add(Headers::DisplayedHeader {
+            .key = jsg::ByteString(kj::str(entry.first)),
+            .value = jsg::ByteString(kj::str(value)),
+          });
+        }
+      } else {
+        copy.add(Headers::DisplayedHeader {
+          .key = jsg::ByteString(kj::str(entry.first)),
+          .value = jsg::ByteString(kj::strArray(entry.second.values, ", "))
+        });
+      }
+    }
+    return copy.releaseAsArray();
+  } else {
+    // The old behavior before the standard getSetCookie() API was introduced...
+    auto headersCopy = KJ_MAP(mapEntry, headers) {
+      const auto& header = mapEntry.second;
+      return DisplayedHeader {
+        jsg::ByteString(kj::str(header.key)),
+        jsg::ByteString(kj::strArray(header.values, ", "))
+      };
     };
-  };
-  return headersCopy;
+    return headersCopy;
+  }
 }
 
 jsg::Ref<Headers> Headers::constructor(jsg::Lock& js, jsg::Optional<Initializer> init) {
@@ -227,19 +252,26 @@ kj::Maybe<jsg::ByteString> Headers::get(jsg::ByteString name) {
   }
 }
 
+kj::ArrayPtr<jsg::ByteString> Headers::getSetCookie() {
+  auto iter = headers.find("set-cookie");
+  if (iter == headers.end()) {
+    return nullptr;
+  } else {
+    return iter->second.values.asPtr();
+  }
+}
+
 kj::ArrayPtr<jsg::ByteString> Headers::getAll(jsg::ByteString name) {
   requireValidHeaderName(name);
 
   if (strcasecmp(name.cStr(), "set-cookie") != 0) {
     JSG_FAIL_REQUIRE(TypeError, "getAll() can only be used with the header name \"Set-Cookie\".");
   }
 
-  auto iter = headers.find(toLower(kj::mv(name)));
-  if (iter == headers.end()) {
-    return nullptr;
-  } else {
-    return iter->second.values.asPtr();
-  }
+  // getSetCookie() is the standard API here. getAll(...) is our legacy non-standard extension
+  // for the same use case. We continue to support getAll for backwards compatibility but moving
+  // forward users really should be using getSetCookie.
+  return getSetCookie();
 }
 
 bool Headers::has(jsg::ByteString name) {
@@ -304,26 +336,68 @@ void Headers::delete_(jsg::ByteString name) {
 //   applied to the header map elements? We'd still copy the whole data structure to avoid iterator
 //   invalidation, but the elements would be cheaper to copy.
 
-jsg::Ref<Headers::EntryIterator> Headers::entries(jsg::Lock&) {
-  return jsg::alloc<EntryIterator>(IteratorState<DisplayedHeader> { getDisplayedHeaders() });
+jsg::Ref<Headers::EntryIterator> Headers::entries(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
+  return jsg::alloc<EntryIterator>(IteratorState<DisplayedHeader> {
+    getDisplayedHeaders(featureFlags)
+  });
 }
-jsg::Ref<Headers::KeyIterator> Headers::keys(jsg::Lock&) {
-  auto keysCopy = KJ_MAP(mapEntry, headers) {
-    return jsg::ByteString(kj::str(mapEntry.second.key));
-  };
-  return jsg::alloc<KeyIterator>(IteratorState<jsg::ByteString> { kj::mv(keysCopy) });
+jsg::Ref<Headers::KeyIterator> Headers::keys(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
+  if (featureFlags.getHttpHeadersGetSetCookie()) {
+    kj::Vector<jsg::ByteString> keysCopy;
+    for (auto& entry : headers) {
+      // Set-Cookie headers must be handled specially. They should never be combined into a
+      // single value, so the values iterator must separate them. It seems a bit silly, but
+      // the keys iterator can end up having multiple set-cookie instances.
+      if (entry.first == "set-cookie") {
+        for (auto n = 0; n < entry.second.values.size(); n++) {
+          keysCopy.add(jsg::ByteString(kj::str(entry.first)));
+        }
+      } else {
+        keysCopy.add(jsg::ByteString(kj::str(entry.first)));
+      }
+    }
+    return jsg::alloc<KeyIterator>(IteratorState<jsg::ByteString> { keysCopy.releaseAsArray() });
+  } else {
+    auto keysCopy = KJ_MAP(mapEntry, headers) {
+      return jsg::ByteString(kj::str(mapEntry.second.key));
+    };
+    return jsg::alloc<KeyIterator>(IteratorState<jsg::ByteString> { kj::mv(keysCopy) });
+  }
 }
-jsg::Ref<Headers::ValueIterator> Headers::values(jsg::Lock&) {
-  auto valuesCopy = KJ_MAP(mapEntry, headers) {
-    return jsg::ByteString(kj::strArray(mapEntry.second.values, ", "));
-  };
-  return jsg::alloc<ValueIterator>(IteratorState<jsg::ByteString> { kj::mv(valuesCopy) });
+jsg::Ref<Headers::ValueIterator> Headers::values(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
+  if (featureFlags.getHttpHeadersGetSetCookie()) {
+    kj::Vector<jsg::ByteString> values;
+    for (auto& entry : headers) {
+      // Set-Cookie headers must be handled specially. They should never be combined into a
+      // single value, so the values iterator must separate them.
+      if (entry.first == "set-cookie") {
+        for (auto& value : entry.second.values) {
+          values.add(jsg::ByteString(kj::str(value)));
+        }
+      } else {
+        values.add(jsg::ByteString(kj::strArray(entry.second.values, ", ")));
+      }
+    }
+    return jsg::alloc<ValueIterator>(IteratorState<jsg::ByteString> { values.releaseAsArray() });
+  } else {
+    auto valuesCopy = KJ_MAP(mapEntry, headers) {
+      return jsg::ByteString(kj::strArray(mapEntry.second.values, ", "));
+    };
+    return jsg::alloc<ValueIterator>(IteratorState<jsg::ByteString> { kj::mv(valuesCopy) });
+  }
 }
 
 void Headers::forEach(
     jsg::Lock& js,
     jsg::V8Ref<v8::Function> callback,
-    jsg::Optional<jsg::Value> thisArg) {
+    jsg::Optional<jsg::Value> thisArg,
+    CompatibilityFlags::Reader featureFlags) {
   auto localCallback = callback.getHandle(js);
   auto localThisArg = thisArg.map([&](jsg::Value& v) { return v.getHandle(js); })
       .orDefault(js.v8Undefined());
@@ -335,7 +409,7 @@ void Headers::forEach(
   auto localHeaders = KJ_ASSERT_NONNULL(JSG_THIS.tryGetHandle(isolate));
 
   auto context = isolate->GetCurrentContext();  // Needed later for Call().
-  for (auto& entry: getDisplayedHeaders()) {
+  for (auto& entry: getDisplayedHeaders(featureFlags)) {
     static constexpr auto ARG_COUNT = 3;
     v8::Local<v8::Value> args[ARG_COUNT] = {
       jsg::v8Str(isolate, entry.value),

diff --git a/src/workerd/api/http.h b/src/workerd/api/http.h
@@ -61,7 +61,7 @@ class Headers: public jsg::Object {
   // Like has(), but only call this with an already-lower-case `name`. Useful to avoid an
   // unnecessary string allocation. Not part of the JS interface.
 
-  kj::Array<DisplayedHeader> getDisplayedHeaders();
+  kj::Array<DisplayedHeader> getDisplayedHeaders(CompatibilityFlags::Reader featureFlags);
   // Returns headers with lower-case name and comma-concatenated duplicates.
 
   using ByteStringPair = jsg::Sequence<jsg::ByteString>;
@@ -84,13 +84,20 @@ class Headers: public jsg::Object {
   static jsg::Ref<Headers> constructor(jsg::Lock& js, jsg::Optional<Initializer> init);
   kj::Maybe<jsg::ByteString> get(jsg::ByteString name);
   kj::ArrayPtr<jsg::ByteString> getAll(jsg::ByteString name);
+  // getAll is a legacy non-standard extension API that we introduced before
+  // getSetCookie() was defined. We continue to support it for backwards
+  // compatibility but users really ought to be using getSetCookie() now.
+  kj::ArrayPtr<jsg::ByteString> getSetCookie();
+  // The Set-Cookie header is special in that it is the only HTTP header that
+  // is not permitted to be combined into a single instance.
   bool has(jsg::ByteString name);
   void set(jsg::ByteString name, jsg::ByteString value);
   void append(jsg::ByteString name, jsg::ByteString value);
   void delete_(jsg::ByteString name);
   void forEach(jsg::Lock& js,
       jsg::V8Ref<v8::Function>,
-      jsg::Optional<jsg::Value>);
+      jsg::Optional<jsg::Value>,
+      CompatibilityFlags::Reader featureFlags);
 
   JSG_ITERATOR(EntryIterator, entries,
                 kj::Array<jsg::ByteString>,
@@ -107,9 +114,12 @@ class Headers: public jsg::Object {
 
   // JavaScript API.
 
-  JSG_RESOURCE_TYPE(Headers) {
+  JSG_RESOURCE_TYPE(Headers, CompatibilityFlags::Reader flags) {
     JSG_METHOD(get);
     JSG_METHOD(getAll);
+    if (flags.getHttpHeadersGetSetCookie()) {
+      JSG_METHOD(getSetCookie);
+    }
     JSG_METHOD(has);
     JSG_METHOD(set);
     JSG_METHOD(append);

diff --git a/src/workerd/api/url-standard.c++ b/src/workerd/api/url-standard.c++
@@ -2006,15 +2006,21 @@ void URLSearchParams::sort() {
   update();
 }
 
-jsg::Ref<URLSearchParams::EntryIterator> URLSearchParams::entries(jsg::Lock&) {
+jsg::Ref<URLSearchParams::EntryIterator> URLSearchParams::entries(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
   return jsg::alloc<URLSearchParams::EntryIterator>(IteratorState { JSG_THIS });
 }
 
-jsg::Ref<URLSearchParams::KeyIterator> URLSearchParams::keys(jsg::Lock&) {
+jsg::Ref<URLSearchParams::KeyIterator> URLSearchParams::keys(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
   return jsg::alloc<URLSearchParams::KeyIterator>(IteratorState { JSG_THIS });
 }
 
-jsg::Ref<URLSearchParams::ValueIterator> URLSearchParams::values(jsg::Lock&) {
+jsg::Ref<URLSearchParams::ValueIterator> URLSearchParams::values(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
   return jsg::alloc<URLSearchParams::ValueIterator>(IteratorState { JSG_THIS });
 }
 

diff --git a/src/workerd/api/url-standard.h b/src/workerd/api/url-standard.h
@@ -8,6 +8,7 @@
 #include "form-data.h"
 #include <workerd/jsg/jsg.h>
 #include <workerd/jsg/string.h>
+#include <workerd/io/compatibility-date.capnp.h>
 
 namespace workerd::api {
   // The original URL implementation based on kj::Url is not compliant with the

diff --git a/src/workerd/api/url.c++ b/src/workerd/api/url.c++
@@ -611,15 +611,21 @@ void URLSearchParams::forEach(
   }
 }
 
-jsg::Ref<URLSearchParams::EntryIterator> URLSearchParams::entries(jsg::Lock&) {
+jsg::Ref<URLSearchParams::EntryIterator> URLSearchParams::entries(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
   return jsg::alloc<EntryIterator>(IteratorState { JSG_THIS });
 }
 
-jsg::Ref<URLSearchParams::KeyIterator> URLSearchParams::keys(jsg::Lock&) {
+jsg::Ref<URLSearchParams::KeyIterator> URLSearchParams::keys(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
   return jsg::alloc<KeyIterator>(IteratorState { JSG_THIS });
 }
 
-jsg::Ref<URLSearchParams::ValueIterator> URLSearchParams::values(jsg::Lock&) {
+jsg::Ref<URLSearchParams::ValueIterator> URLSearchParams::values(
+    jsg::Lock&,
+    CompatibilityFlags::Reader featureFlags) {
   return jsg::alloc<ValueIterator>(IteratorState { JSG_THIS });
 }
 

diff --git a/src/workerd/io/compatibility-date.capnp b/src/workerd/io/compatibility-date.capnp
@@ -274,4 +274,11 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
       $experimental;
   # Experimental, allows getting a durable object stub that ensures the object already exists.
   # This is currently a work in progress mechanism that is not yet available for use in workerd.
+
+  httpHeadersGetSetCookie @26 :Bool
+      $compatEnableFlag("http_headers_getsetcookie")
+      $compatDisableFlag("no_http_headers_getsetcookie")
+      $compatEnableDate("2023-03-01");
+  # Enables the new headers.getSetCookie() API and the corresponding changes in behavior for
+  # the Header objects keys() and entries() iterators.
 }
diff --git a/src/workerd/jsg/iterator.h b/src/workerd/jsg/iterator.h
@@ -921,7 +921,7 @@ class AsyncIteratorBase: public Object {
       JSG_ITERABLE(self);                                                                      \
     }                                                                                           \
   };                                                                                            \
-  jsg::Ref<Name> Label(jsg::Lock&);
+  jsg::Ref<Name> Label(jsg::Lock&, CompatibilityFlags::Reader featureFlags);
 // The JSG_ITERATOR macro provides a mechanism for easily implementing JavaScript-style iterators
 // for JSG_RESOURCE_TYPES.
 //