handle invalid utf-16 surrogates on TextDecoder

anonrig · anonrig · commit 8dee6c36ab60 · 2025-12-17T17:19:27.000-05:00
diff --git a/src/workerd/api/BUILD.bazel b/src/workerd/api/BUILD.bazel
@@ -433,7 +433,9 @@ wd_cc_library(
     srcs = ["encoding.c++"],
     hdrs = ["encoding.h"],
     implementation_deps = [
+        "//src/workerd/io:features",
         "//src/workerd/util:strings",
+        "@simdutf",
     ],
     visibility = ["//visibility:public"],
     deps = [
@@ -537,6 +539,7 @@ kj_test(
     src = "data-url-test.c++",
     deps = [
         ":data-url",
+        "//src/workerd/io",
     ],
 )
 
diff --git a/src/workerd/api/encoding.c++ b/src/workerd/api/encoding.c++
@@ -4,8 +4,10 @@
 
 #include "encoding.h"
 
+#include "simdutf.h"
 #include "util.h"
 
+#include <workerd/io/features.h>
 #include <workerd/jsg/jsg.h>
 #include <workerd/util/strings.h>
 
@@ -289,7 +291,7 @@ kj::Maybe<IcuDecoder> IcuDecoder::create(Encoding encoding, bool fatal, bool ign
     if (U_FAILURE(status)) return kj::none;
   }
 
-  return IcuDecoder(encoding, inner, ignoreBom);
+  return IcuDecoder(encoding, inner, fatal, ignoreBom);
 }
 
 kj::Maybe<jsg::JsString> IcuDecoder::decode(
@@ -350,7 +352,32 @@ kj::Maybe<jsg::JsString> IcuDecoder::decode(
           omitInitialBom = data[0] == 0xfeff;
           bomSeen = true;
         }
-        return js.str(data.slice(omitInitialBom ? 1 : 0, data.size()));
+
+        auto slice = data.slice(omitInitialBom ? 1 : 0, data.size());
+
+        // If pedanticWpt flag is enabled, then we follow the spec and fix invalid
+        // surrogates on the UTF-16 input.
+        if (slice.size() == 0 || !FeatureFlags::get(js).getPedanticWpt()) {
+          return js.str(slice);
+        }
+
+        if (simdutf::validate_utf16(slice.begin(), slice.size())) {
+          return js.str(slice);
+        }
+
+        if (fatal) {
+          // In fatal mode, return error for invalid surrogates
+          return kj::none;
+        }
+
+        // In non-fatal mode, replace invalid surrogates with U+FFFD.
+        // Output size equals input size because each invalid surrogate (1 code unit)
+        // is replaced with U+FFFD (also 1 code unit).
+        // Use stack allocation for small strings (up to 256 code units) to avoid
+        // heap allocation overhead.
+        kj::SmallArray<char16_t, 256> fixed(slice.size());
+        simdutf::to_well_formed_utf16(slice.begin(), slice.size(), fixed.begin());
+        return js.str(fixed.asPtr());
       }
     }
   }
diff --git a/src/workerd/api/encoding.h b/src/workerd/api/encoding.h
@@ -95,9 +95,10 @@ class AsciiDecoder final: public Decoder {
 // of encodings required by the Encoding specification.
 class IcuDecoder final: public Decoder {
  public:
-  IcuDecoder(Encoding encoding, UConverter* converter, bool ignoreBom)
+  IcuDecoder(Encoding encoding, UConverter* converter, bool fatal, bool ignoreBom)
       : encoding(encoding),
         inner(converter),
+        fatal(fatal),
         ignoreBom(ignoreBom),
         bomSeen(false) {}
   IcuDecoder(IcuDecoder&&) = default;
@@ -124,6 +125,7 @@ class IcuDecoder final: public Decoder {
   Encoding encoding;
   std::unique_ptr<UConverter, ConverterDeleter> inner;
 
+  bool fatal;
   bool ignoreBom;
   bool bomSeen;
 };
diff --git a/src/workerd/io/BUILD.bazel b/src/workerd/io/BUILD.bazel
@@ -47,7 +47,6 @@ wd_cc_library(
     ] + ["//src/workerd/api:srcs"],
     hdrs = [
         "compatibility-date.h",
-        "features.h",
         "hibernation-manager.h",
         "io-channels.h",
         "io-context.h",
@@ -90,6 +89,7 @@ wd_cc_library(
         ":actor-storage_capnp",
         ":cdp_capnp",
         ":container_capnp",
+        ":features",
         ":frankenvalue",
         ":io-gate",
         ":io-helpers",
@@ -125,6 +125,19 @@ wd_cc_library(
     ],
 )
 
+# Headers only target to avoid having circular dependencies.
+wd_cc_library(
+    name = "features",
+    hdrs = ["features.h"],
+    implementation_deps = [
+        "//src/workerd/jsg",
+    ],
+    visibility = ["//visibility:public"],
+    deps = [
+        ":compatibility-date_capnp",
+    ],
+)
+
 wd_cc_library(
     name = "worker-modules",
     srcs = ["worker-modules.c++"],
diff --git a/src/wpt/encoding-test.ts b/src/wpt/encoding-test.ts
@@ -165,21 +165,7 @@ export default {
     ],
   },
   'textdecoder-streaming.any.js': {},
-  'textdecoder-utf16-surrogates.any.js': {
-    comment: 'Investigate why we are not blocking invalid surrogates',
-    expectedFailures: [
-      'utf-16le - lone surrogate lead',
-      'utf-16le - lone surrogate lead (fatal flag set)',
-      'utf-16le - lone surrogate trail',
-      'utf-16le - lone surrogate trail (fatal flag set)',
-      'utf-16le - unmatched surrogate lead',
-      'utf-16le - unmatched surrogate lead (fatal flag set)',
-      'utf-16le - unmatched surrogate trail',
-      'utf-16le - unmatched surrogate trail (fatal flag set)',
-      'utf-16le - swapped surrogate pair',
-      'utf-16le - swapped surrogate pair (fatal flag set)',
-    ],
-  },
+  'textdecoder-utf16-surrogates.any.js': {},
   'textencoder-constructor-non-utf.any.js': {
     comment: 'Investigate this',
     expectedFailures: [
