URL, URLSearchParams: throw a RangeError when the percent-encoded result does not fit in a string

scripts/build/deps/webkit.ts

@@ -3,7 +3,7 @@
  * for local mode. Override via `--webkit-version=<hash>` to test a branch.
  * From https://github.com/oven-sh/WebKit releases.
  */
-export const WEBKIT_VERSION = "cf1b36ec8703d8e87436094d21d478d358c7d886";
+export const WEBKIT_VERSION = "autobuild-preview-pr-643-d4914100";
 
 /**
  * WebKit (JavaScriptCore) — the JS engine.

src/jsc/URLSearchParams.rs

@@ -18,7 +18,7 @@ unsafe extern "C" {
         self_: &mut URLSearchParams,
         ctx: *mut c_void,
         callback: extern "C" fn(ctx: *mut c_void, str: *const EncodedSlice),
-    );
+    ) -> bool;
 }
 
 impl URLSearchParams {
@@ -28,11 +28,13 @@ impl URLSearchParams {
         URLSearchParams__fromJS(value)
     }
 
+    /// Returns false, without a call of `callback`, when the result does not fit in a `WTF::String`.
+    #[must_use]
     pub fn to_string<Ctx>(
         &mut self,
         ctx: &mut Ctx,
         callback: fn(ctx: &mut Ctx, str: EncodedSlice),
-    ) {
+    ) -> bool {
         // A fn pointer cannot be a const generic, so pack (ctx, callback) on the
         // stack and pass the pair through the C trampoline's void* context.
         struct Wrap<'a, Ctx> {
@@ -53,6 +55,6 @@ impl URLSearchParams {
         let mut w = Wrap { ctx, callback };
         // `w` lives for the duration of the call (URLSearchParams__toString invokes
         // the callback synchronously, does not retain it).
-        URLSearchParams__toString(self, (&raw mut w).cast::<c_void>(), cb::<Ctx>);
+        URLSearchParams__toString(self, (&raw mut w).cast::<c_void>(), cb::<Ctx>)
     }
 }

src/jsc/VirtualMachine.rs

@@ -52,6 +52,18 @@ pub fn synthetic_allocation_limit() -> usize {
 // `Bun__stringSyntheticAllocationLimit`.
 pub use bun_core::STRING_ALLOCATION_LIMIT;
 
+unsafe extern "C" {
+    safe fn Bun__setURLMaximumLengthForTesting(limit: usize);
+}
+
+/// Sets the limit for Rust, Bun's C++ and WTF's URL parser. Returns the previous limit.
+pub(crate) fn set_synthetic_allocation_limit(limit: usize) -> usize {
+    let previous = SYNTHETIC_ALLOCATION_LIMIT.swap(limit, core::sync::atomic::Ordering::Relaxed);
+    STRING_ALLOCATION_LIMIT.store(limit, core::sync::atomic::Ordering::Relaxed);
+    Bun__setURLMaximumLengthForTesting(limit);
+    previous
+}
+
 // ──────────────────────────────────────────────────────────────────────────
 // Type aliases
 // ──────────────────────────────────────────────────────────────────────────
@@ -3812,9 +3824,7 @@ impl VirtualMachine {
             if let Some(value) = map.get(b"BUN_FEATURE_FLAG_SYNTHETIC_MEMORY_LIMIT") {
                 match bun_core::fmt::parse_int::<usize>(value, 10).ok() {
                     Some(limit) => {
-                        SYNTHETIC_ALLOCATION_LIMIT
-                            .store(limit, core::sync::atomic::Ordering::Relaxed);
-                        STRING_ALLOCATION_LIMIT.store(limit, core::sync::atomic::Ordering::Relaxed);
+                        set_synthetic_allocation_limit(limit);
                     }
                     None => bun_core::Output::panic(format_args!(
                         "BUN_FEATURE_FLAG_SYNTHETIC_MEMORY_LIMIT must be a positive integer"
@@ -5310,8 +5320,7 @@ impl VirtualMachine {
     /// Put the startup value back so a file's limit stays with that file.
     fn undo_synthetic_allocation_limit(&mut self) {
         if let Some(limit) = self.test_isolation_state.synthetic_allocation_limit {
-            SYNTHETIC_ALLOCATION_LIMIT.store(limit, core::sync::atomic::Ordering::Relaxed);
-            STRING_ALLOCATION_LIMIT.store(limit, core::sync::atomic::Ordering::Relaxed);
+            set_synthetic_allocation_limit(limit);
         }
     }
 

src/jsc/bindings/DOMFormData.cpp

@@ -57,18 +57,6 @@ Ref<DOMFormData> DOMFormData::create(ScriptExecutionContext* context, const Stri
     return newFormData;
 }
 
-String DOMFormData::toURLEncodedString()
-{
-    WTF::URLParser::URLEncodedForm form;
-    form.reserveInitialCapacity(m_items.size());
-    for (auto& item : m_items) {
-        if (auto value = std::get_if<String>(&item.data))
-            form.append({ item.name, *value });
-    }
-
-    return WTF::URLParser::serialize(form);
-}
-
 extern "C" void DOMFormData__forEach(DOMFormData* form, void* context, void (*callback)(void* context, EncodedSlice*, void*, EncodedSlice*, uint8_t))
 {
     for (auto& item : form->items()) {

src/jsc/bindings/DOMFormData.h

@@ -77,8 +77,6 @@ class DOMFormData : public RefCounted<DOMFormData>, public ContextDestructionObs
     size_t count() const { return m_items.size(); }
     size_t memoryCost() const;
 
-    String toURLEncodedString();
-
     class Iterator {
     public:
         explicit Iterator(DOMFormData&);

src/jsc/bindings/DOMURL.cpp

@@ -28,10 +28,25 @@
 
 #include "NodeURLHelpers.h"
 #include "URLSearchParams.h"
+#include <wtf/URLParser.h>
 #include <wtf/text/StringCommon.h>
 
+extern "C" size_t Bun__stringSyntheticAllocationLimit;
+
+// WTF's URL parser keeps its own copy of the limit that setSyntheticAllocationLimitForTesting lowers.
+extern "C" void Bun__setURLMaximumLengthForTesting(size_t limit)
+{
+    WTF::URLParser::setMaximumLengthForTesting(static_cast<unsigned>(std::min<size_t>(limit, String::MaxLength)));
+}
+
 namespace WebCore {
 
+// A null input parses to the null URL too. Input that is not a URL gives an invalid URL that keeps the input.
+static bool isTooLong(const URL& parsed, const String& input)
+{
+    return doesNotFitInString(parsed) && !input.isNull();
+}
+
 // The WHATWG parser (WebKit) fast-paths all-ASCII hosts without validating
 // xn-- labels; Node's ada rejects invalid punycode in special-scheme hosts.
 // `input` is the string the host was parsed from (a base URL's host was checked when the base was parsed).
@@ -94,6 +109,8 @@ inline DOMURL::DOMURL(URL&& completeURL)
 ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url)
 {
     URL completeURL { url };
+    if (isTooLong(completeURL, url)) [[unlikely]]
+        return Exception { OutOfMemoryError };
     if (!completeURL.isValid() || !hasValidParsedHost(completeURL, url))
         return Exception { InvalidURLError, url };
     return adoptRef(*new DOMURL(WTF::move(completeURL)));
@@ -103,17 +120,20 @@ ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url, const URL& base, cons
 {
     ASSERT(base.isValid() || base.isNull());
     URL completeURL { base, url };
+    if (isTooLong(completeURL, url)) [[unlikely]]
+        return Exception { OutOfMemoryError };
     if (!completeURL.isValid() || !hasValidParsedHost(completeURL, url))
         return Exception { InvalidURLError, url, baseInput };
     return adoptRef(*new DOMURL(WTF::move(completeURL)));
 }
 
-// A null URL means the base did not parse or has an invalid host.
-static URL parseBase(const String& base, DOMURL::BaseURLCache* cache)
+// A null URL means the base did not parse or has an invalid host. tooLong tells a base that does not fit in a String from those.
+static URL parseBase(const String& base, DOMURL::BaseURLCache* cache, bool& tooLong)
 {
     if (cache && cache->input == base) [[likely]]
         return cache->url;
     URL baseURL { base };
+    tooLong = isTooLong(baseURL, base);
     if (!baseURL.isValid() || !hasValidParsedHost(baseURL, base))
         return {};
     if (cache) {
@@ -125,7 +145,10 @@ static URL parseBase(const String& base, DOMURL::BaseURLCache* cache)
 
 ExceptionOr<Ref<DOMURL>> DOMURL::create(const String& url, const String& base, BaseURLCache* cache)
 {
-    URL baseURL = base.isNull() ? URL {} : parseBase(base, cache);
+    bool baseIsTooLong = false;
+    URL baseURL = base.isNull() ? URL {} : parseBase(base, cache, baseIsTooLong);
+    if (baseIsTooLong) [[unlikely]]
+        return Exception { OutOfMemoryError };
     if (!base.isNull() && !baseURL.isValid())
         return Exception { InvalidURLError, url, base };
     return create(url, baseURL, base);
@@ -135,7 +158,8 @@ DOMURL::~DOMURL() = default;
 
 static URL parseInternal(const String& url, const String& base, DOMURL::BaseURLCache* cache)
 {
-    URL baseURL = base.isNull() ? URL {} : parseBase(base, cache);
+    bool baseIsTooLong = false;
+    URL baseURL = base.isNull() ? URL {} : parseBase(base, cache, baseIsTooLong);
     if (!base.isNull() && !baseURL.isValid())
         return {};
     URL result { baseURL, url };
@@ -160,32 +184,82 @@ bool DOMURL::canParse(const String& url, const String& base, BaseURLCache* cache
 ExceptionOr<void> DOMURL::setHref(const String& url)
 {
     URL completeURL { URL {}, url };
+    if (isTooLong(completeURL, url)) [[unlikely]]
+        return Exception { OutOfMemoryError };
     if (!completeURL.isValid() || !hasValidParsedHost(completeURL, url))
         return Exception { InvalidURLError, url };
     m_url = WTF::move(completeURL);
     m_searchParamsDirty = false;
+    m_pendingSearchParamsLength = 0;
     if (m_searchParams)
         m_searchParams->updateFromAssociatedURL();
     return {};
 }
 
+// Per the URL spec the setters ignore a value that is not valid. They only report a URL that does not fit in a String.
+ExceptionOr<void> DOMURL::setFullURL(const URL& fullURL)
+{
+    if (doesNotFitInString(fullURL)) [[unlikely]]
+        return Exception { OutOfMemoryError };
+    auto result = setHref(fullURL.string());
+    if (result.hasException() && result.exception().code() != OutOfMemoryError)
+        return {};
+    return result;
+}
+
+static size_t maximumURLLength()
+{
+    return std::min<size_t>(String::MaxLength, Bun__stringSyntheticAllocationLimit);
+}
+
+// "(" in the query of the URL is "%28=" when the params serialize it.
+static constexpr uint64_t maximumGrowthOfSerializedQuery = 4;
+
+bool DOMURL::deferSearchParamsUpdate(uint64_t addedLength)
+{
+    uint64_t lengthBound = maximumGrowthOfSerializedQuery * m_url.string().length() + m_pendingSearchParamsLength + addedLength;
+    // Half of the limit leaves WTF::URLParser the room it reserves. Past it the URL takes the pairs at once, which is exact.
+    if (lengthBound > maximumURLLength() / 2) [[unlikely]]
+        return false;
+    m_pendingSearchParamsLength += static_cast<uint32_t>(addedLength);
+    m_searchParamsDirty = true;
+    return true;
+}
+
+bool DOMURL::updateFromSearchParams()
+{
+    bool wasDirty = std::exchange(m_searchParamsDirty, true);
+    if (flushPendingSearchParamsUpdate())
+        return true;
+    // URLSearchParams puts its pairs back, so the URL is as much behind them as it was.
+    m_searchParamsDirty = wasDirty;
+    return false;
+}
+
 // The update steps invoked on URLSearchParams::{append,set,delete,sort} set
 // m_searchParamsDirty instead of eagerly re-serializing m_url on every call so
 // that N appends through url.searchParams stay O(N) instead of O(N^2). All
 // reads of m_url (href/fullURL) call this first to reconcile.
-void DOMURL::flushPendingSearchParamsUpdate() const
+// False when the URL does not fit in a String with the new query. It then keeps the query it has.
+bool DOMURL::flushPendingSearchParamsUpdate() const
 {
     if (!m_searchParamsDirty) [[likely]]
-        return;
-    m_searchParamsDirty = false;
+        return true;
     auto* self = const_cast<DOMURL*>(this);
-    if (!self->m_searchParams)
-        return;
-    auto serialized = self->m_searchParams->toString();
-    if (serialized.isEmpty())
-        self->m_url.setQuery({});
-    else
-        self->m_url.setQuery(WTF::move(serialized));
+    if (self->m_searchParams) {
+        auto serialized = self->m_searchParams->toString();
+        if (serialized.hasException()) [[unlikely]]
+            return false;
+        String query = serialized.releaseReturnValue();
+        URL url = m_url;
+        url.setQuery(query.isEmpty() ? StringView() : StringView(query));
+        if (!url.isValid()) [[unlikely]]
+            return false;
+        self->m_url = WTF::move(url);
+    }
+    m_searchParamsDirty = false;
+    m_pendingSearchParamsLength = 0;
+    return true;
 }
 
 URLSearchParams& DOMURL::searchParams()

src/jsc/bindings/DOMURL.h

@@ -57,7 +57,10 @@ class DOMURL final : public RefCounted<DOMURL>, public CanMakeWeakPtr<DOMURL>, p
     ExceptionOr<void> setHref(const String&);
 
     URLSearchParams& searchParams();
-    void markSearchParamsDirty() { m_searchParamsDirty = true; }
+    // For a change of the searchParams that adds at most addedLength characters. True: the URL takes the pairs at its next read.
+    bool deferSearchParamsUpdate(uint64_t addedLength);
+    // Takes the pairs now. False if the URL does not fit in a String with them. It is then as it was.
+    bool updateFromSearchParams();
 
     size_t memoryCost() const
     {
@@ -77,11 +80,13 @@ class DOMURL final : public RefCounted<DOMURL>, public CanMakeWeakPtr<DOMURL>, p
         flushPendingSearchParamsUpdate();
         return m_url;
     }
-    void setFullURL(const URL& fullURL) final { setHref(fullURL.string()); }
-    void flushPendingSearchParamsUpdate() const;
+    ExceptionOr<void> setFullURL(const URL&) final;
+    bool flushPendingSearchParamsUpdate() const;
 
     URL m_url;
     RefPtr<URLSearchParams> m_searchParams;
+    // At least what the changes since the last flush add to the query. deferSearchParamsUpdate() keeps it under 2^30.
+    mutable uint32_t m_pendingSearchParamsLength { 0 };
     uint16_t m_initialURLCostForGC { 0 };
     mutable bool m_searchParamsDirty { false };
 };

src/jsc/bindings/ImportMetaObject.cpp

@@ -410,6 +410,11 @@ JSC_DEFINE_HOST_FUNCTION(functionImportMeta__resolve,
         }
 
         WTF::URL url(fromURL, specifier);
+        // The null URL is a URL that does not fit in a String.
+        if (url.isNull()) [[unlikely]] {
+            throwOutOfMemoryError(globalObject, scope);
+            return {};
+        }
         RELEASE_AND_RETURN(scope, JSValue::encode(jsString(vm, url.string())));
     }