Warn if URLSearchParams has() or delete() are called with multiple ar…

…gs (#635)
cloudflare · May 12, 2023 · 497fe0c · 497fe0c
1 parent 556c57c
commit 497fe0c
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 4 deletions.
diff --git a/src/workerd/api/url-standard.c++ b/src/workerd/api/url-standard.c++
@@ -1943,7 +1943,16 @@ void URLSearchParams::append(jsg::UsvString name, jsg::UsvString value) {
   update();
 }
 
-void URLSearchParams::delete_(jsg::UsvString name) {
+void URLSearchParams::delete_(jsg::UsvString name, jsg::Optional<jsg::Value> value) {
+  KJ_IF_MAYBE(v, value) {
+    // The WHATWG recently added new arguments to the delete() and has() methods.
+    // We need to determine if adding those will break anyone if they are added
+    // without a compat flag. For now, we're just logging so we can know for sure.
+    // If we get this warning even once in production we'll have to introduce the
+    // new arguments behind a compat flag.
+    // https://github.com/whatwg/url/pull/735
+    LOG_WARNING_ONCE("URLSearchParams.prototype.delete() called with a second argument.");
+  }
   auto pivot = std::remove_if(list.begin(), list.end(),
                               [&name](auto& kv) { return kv.name == name; });
   list.truncate(pivot - list.begin());
@@ -1965,7 +1974,16 @@ kj::Array<jsg::UsvStringPtr> URLSearchParams::getAll(jsg::UsvString name) {
   return result.releaseAsArray();
 }
 
-bool URLSearchParams::has(jsg::UsvString name) {
+bool URLSearchParams::has(jsg::UsvString name, jsg::Optional<jsg::Value> value) {
+  KJ_IF_MAYBE(v, value) {
+    // The WHATWG recently added new arguments to the delete() and has() methods.
+    // We need to determine if adding those will break anyone if they are added
+    // without a compat flag. For now, we're just logging so we can know for sure.
+    // If we get this warning even once in production we'll have to introduce the
+    // new arguments behind a compat flag.
+    // https://github.com/whatwg/url/pull/735
+    LOG_WARNING_ONCE("URLSearchParams.prototype.has() called with a second argument.");
+  }
   for (auto& entry : list) {
     if (entry.name == name) return true;
   }

diff --git a/src/workerd/api/url-standard.h b/src/workerd/api/url-standard.h
@@ -96,10 +96,10 @@ class URLSearchParams: public jsg::Object {
   }
 
   void append(jsg::UsvString name, jsg::UsvString value);
-  void delete_(jsg::UsvString name);
+  void delete_(jsg::UsvString name, jsg::Optional<jsg::Value> value);
   kj::Maybe<jsg::UsvStringPtr> get(jsg::UsvString name);
   kj::Array<jsg::UsvStringPtr> getAll(jsg::UsvString name);
-  bool has(jsg::UsvString name);
+  bool has(jsg::UsvString name, jsg::Optional<jsg::Value> value);
   void set(jsg::UsvString name, jsg::UsvString value);
   void sort();
 

diff --git a/src/workerd/util/sentry.h b/src/workerd/util/sentry.h
@@ -48,6 +48,15 @@ inline kj::StringPtr maybeOmitColoFromSentry(uint32_t coloId) {
     KJ_LOG(severity, __VA_ARGS__); \
   }
 
+// Log this to Sentry once ever per process. Typically will be better to use LOG_ERROR_PERIODICALLY (or a future LOG_WARNING_PERIODICALLY level equivalent).
+#define LOG_WARNING_ONCE(...)                                                  \
+  do {                                                                         \
+    static bool logOnce KJ_UNUSED = [&]() {                                    \
+      KJ_LOG(WARNING, __VA_ARGS__);                                            \
+      return true;                                                             \
+    }();                                                                       \
+  } while (0)
+
 // Log this to Sentry once ever per process. Typically will be better to use LOG_ERROR_PERIODICALLY.
 #define LOG_ERROR_ONCE(...)                                                    \
   do {                                                                         \