Implement url encoded FormData parser

builtins/web/fetch/request-response.cpp

@@ -2251,8 +2251,11 @@ bool Response::redirect(JSContext *cx, unsigned argc, Value *vp) {
   if (!url_str.data) {
     return false;
   }
-  auto parsedURL =
-      new_jsurl_with_base(&url_str, url::URL::url(worker_location::WorkerLocation::url));
+
+  auto deleter = [&](auto *url) { jsurl::free_jsurl(url); };
+  std::unique_ptr<jsurl::JSUrl, decltype(deleter)> parsedURL(
+      new_jsurl_with_base(&url_str, url::URL::url(worker_location::WorkerLocation::url)), deleter);
+
   if (!parsedURL) {
     return api::throw_error(cx, api::Errors::TypeError, "Response.redirect", "url",
                             "be a valid URL");

builtins/web/form-data/form-data-parser.cpp

@@ -4,6 +4,7 @@
 #include "form-data.h"
 #include "rust-encoding.h"
 #include "rust-multipart-ffi.h"
+#include "rust-url.h"
 
 #include "../file.h"
 
@@ -217,6 +218,61 @@ JSObject *MultipartParser::parse(JSContext *cx, std::string_view body) {
   return formdata;
 }
 
+class UrlParser : public FormDataParser {
+  virtual JSObject *parse(JSContext *cx, std::string_view body) override;
+};
+
+JSObject *UrlParser::parse(JSContext *cx, std::string_view body) {
+  RootedObject formdata(cx, FormData::create(cx));
+  if (!formdata) {
+    return nullptr;
+  }
+
+  if (body.empty()) {
+    return formdata;
+  }
+
+  jsurl::SpecString spec((uint8_t *)body.data(), body.size(), body.size());
+
+  auto deleter = [&](auto *params) { jsurl::free_params(params); };
+  std::unique_ptr<jsurl::JSUrlSearchParams, decltype(deleter)> params(jsurl::new_params(), deleter);
+  if (!params) {
+    JS_ReportOutOfMemory(cx);
+    return nullptr;
+  }
+
+  jsurl::params_init(params.get(), &spec);
+  auto index = 0;
+
+  jsurl::JSSearchParam param{};
+
+  while (true) {
+    jsurl::params_at(params.get(), index, &param);
+    if (param.done || param.name.data == nullptr || param.value.data == nullptr) {
+      break;
+    }
+
+    auto val_chars = JS::UTF8Chars((char *)param.value.data, param.value.len);
+    JS::RootedString val_str(cx, JS_NewStringCopyUTF8N(cx, val_chars));
+    if (!val_str) {
+      JS_ReportOutOfMemory(cx);
+      return nullptr;
+    }
+
+    auto name = std::string_view((char *)param.name.data, param.name.len);
+    JS::RootedValue value_val(cx, JS::StringValue(val_str));
+
+    auto res = FormData::append(cx, formdata, name, value_val, UndefinedHandleValue);
+    if (!res) {
+      return nullptr;
+    }
+
+    index += 1;
+  }
+
+  return formdata;
+}
+
 std::unique_ptr<FormDataParser> FormDataParser::create(std::string_view content_type) {
   if (content_type.starts_with("multipart/form-data")) {
     jsmultipart::Slice content_slice{(uint8_t *)(content_type.data()), content_type.size()};
@@ -230,7 +286,7 @@ std::unique_ptr<FormDataParser> FormDataParser::create(std::string_view content_
     std::string_view boundary((char *)boundary_slice.data, boundary_slice.len);
     return std::make_unique<MultipartParser>(boundary);
   } else if (content_type.starts_with("application/x-www-form-urlencoded")) {
-    // TODO: add form url encoded content parser
+    return std::make_unique<UrlParser>();
   } else if (content_type.starts_with("text/plain")) {
     // TODO: add plain text content parser
   }

builtins/web/form-data/form-data.h

@@ -54,6 +54,7 @@ class FormData : public TraceableBuiltinImpl<FormData> {
   using EntryList = JS::GCVector<FormDataEntry, 0, js::SystemAllocPolicy>;
   static EntryList *entry_list(JSObject *self);
 
+  friend class UrlParser;
   friend class FormDataIterator;
   friend class MultipartParser;
   friend class MultipartFormData;

builtins/web/url.cpp

@@ -728,7 +728,7 @@ JSObject *URL::create(JSContext *cx, JS::HandleObject self, JS::HandleValue url_
 void URL::finalize(JS::GCContext *gcx, JSObject *self) {
   jsurl::JSUrl *url =
       static_cast<jsurl::JSUrl *>(JS::GetReservedSlot(self, Slots::Url).toPrivate());
-  free(url);
+  jsurl::free_jsurl(url);
 }
 
 JSObject *URL::create(JSContext *cx, JS::HandleObject self, JS::HandleValue url_val,

crates/rust-url/rust-url.h

@@ -111,6 +111,8 @@ JSUrl *new_jsurl(const SpecString *spec);
 
 JSUrl *new_jsurl_with_base(const SpecString *spec, const JSUrl *base);
 
+void free_jsurl(JSUrl *url);
+
 SpecSlice authority(const JSUrl *url);
 
 SpecSlice path_with_query(const JSUrl *url);
@@ -161,6 +163,8 @@ JSUrlSearchParams *url_search_params(JSUrl *url);
 
 JSUrlSearchParams *new_params();
 
+void free_params(JSUrlSearchParams *params);
+
 void params_init(JSUrlSearchParams *params, const SpecString *init);
 
 void params_append(JSUrlSearchParams *params, SpecString name, SpecString value);
@@ -181,8 +185,8 @@ void params_sort(JSUrlSearchParams *params);
 
 SpecSlice params_to_string(const JSUrlSearchParams *params);
 
-} // extern "C"
+}  // extern "C"
 
-} // namespace jsurl
+}  // namespace jsurl
 
-#endif // rust_url_bindings_h
+#endif  // rust_url_bindings_h

crates/rust-url/src/lib.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::missing_safety_doc)]
+
 /// Wrapper for the Url crate and a URLSearchParams implementation that enables use from C++.
 use std::marker::PhantomData;
 use std::slice;
@@ -45,14 +47,13 @@ impl JSUrlSearchParams {
                 }
             }
             UrlOrString::Str(_) => {
-                let str;
-                if self.list.is_empty() {
-                    str = form_urlencoded::Serializer::new(String::new()).finish();
+                let str = if self.list.is_empty() {
+                    form_urlencoded::Serializer::new(String::new()).finish()
                 } else {
-                    str = form_urlencoded::Serializer::new(String::new())
+                    form_urlencoded::Serializer::new(String::new())
                         .extend_pairs(&*self.list)
-                        .finish();
-                }
+                        .finish()
+                };
                 self.url_or_str = UrlOrString::Str(str);
             }
         }
@@ -81,6 +82,19 @@ pub unsafe extern "C" fn new_jsurl_with_base(spec: &SpecString, base: &JSUrl) ->
     }
 }
 
+#[no_mangle]
+pub unsafe extern "C" fn free_jsurl(url: *mut JSUrl) {
+    if url.is_null() {
+        return;
+    }
+
+    if !(*url).params.is_null() {
+        free_params((*url).params);
+    }
+
+    let _ = Box::from_raw(url);
+}
+
 #[no_mangle]
 pub extern "C" fn authority(url: &JSUrl) -> SpecSlice {
     url.url.authority().into()
@@ -188,7 +202,7 @@ pub extern "C" fn search(url: &JSUrl) -> SpecSlice {
 
 #[no_mangle]
 pub extern "C" fn set_search(url: &mut JSUrl, search: &SpecString) {
-    let _ = quirks::set_search(&mut url.url, search.into());
+    quirks::set_search(&mut url.url, search.into());
     url.update_params();
 }
 
@@ -222,13 +236,22 @@ pub unsafe extern "C" fn new_params() -> *mut JSUrlSearchParams {
     }))
 }
 
+#[no_mangle]
+pub unsafe extern "C" fn free_params(params: *mut JSUrlSearchParams) {
+    if params.is_null() {
+        return;
+    }
+
+    let _ = Box::from_raw(params);
+}
+
 #[no_mangle]
 pub extern "C" fn params_init(params: &mut JSUrlSearchParams, init: &SpecString) {
     let init = unsafe { slice::from_raw_parts(init.data, init.len) };
 
     // https://url.spec.whatwg.org/#dom-urlsearchparams-urlsearchparams
     // Step 1
-    let init = if init.len() > 0 && init[0] == '?' as u8 {
+    let init = if !init.is_empty() && init[0] == b'?' {
         &init[1..]
     } else {
         init
@@ -251,14 +274,14 @@ pub extern "C" fn params_append(
 #[no_mangle]
 pub extern "C" fn params_delete(params: &mut JSUrlSearchParams, name: &SpecString) {
     let name: &str = name.into();
-    params.list.retain(|&(ref k, _)| k != &name);
+    params.list.retain(|(k, _)| k != name);
     params.update_url_or_str();
 }
 
 #[no_mangle]
 pub extern "C" fn params_has(params: &JSUrlSearchParams, name: &SpecString) -> bool {
     let name: &str = name.into();
-    params.list.iter().find(|&kv| kv.0 == name).is_some()
+    params.list.iter().any(|kv| kv.0 == name)
 }
 
 #[no_mangle]
@@ -271,7 +294,7 @@ pub extern "C" fn params_get<'a>(
         .list
         .iter()
         .find(|&kv| kv.0 == name)
-        .map(|ref kv| SpecSlice::from(kv.1.as_str()))
+        .map(|kv| SpecSlice::from(kv.1.as_str()))
         .unwrap_or_else(|| SpecSlice::new(std::ptr::null(), 0))
 }
 
@@ -299,8 +322,8 @@ pub extern "C" fn params_get_all<'a>(
     let mut values: Vec<SpecSlice> = params
         .list
         .iter()
-        .filter_map(|&(ref k, ref v)| {
-            if k == &name {
+        .filter_map(|(k, v)| {
+            if k == name {
                 Some(SpecSlice::from(v.as_str()))
             } else {
                 None
@@ -324,7 +347,7 @@ pub extern "C" fn params_set(params: &mut JSUrlSearchParams, name: SpecString, v
 
     let mut index = None;
     let mut i = 0;
-    params.list.retain(|&(ref k, _)| {
+    params.list.retain(|(k, _)| {
         if index.is_none() {
             if k == &name {
                 index = Some(i);
@@ -353,7 +376,7 @@ pub extern "C" fn params_sort(params: &mut JSUrlSearchParams) {
 }
 
 #[no_mangle]
-pub extern "C" fn params_to_string<'a>(params: &'a JSUrlSearchParams) -> SpecSlice<'a> {
+pub extern "C" fn params_to_string(params: &JSUrlSearchParams) -> SpecSlice<'_> {
     match &params.url_or_str {
         UrlOrString::Url(url) => {
             let url = unsafe { url.as_mut().unwrap() };
@@ -439,15 +462,15 @@ impl SpecSlice<'_> {
     fn new(data: *const u8, len: usize) -> Self {
         SpecSlice {
             data,
-            len: len,
+            len,
             _marker: PhantomData,
         }
     }
 }
 
 impl<'a> From<&'a str> for SpecSlice<'a> {
     fn from(s: &'a str) -> SpecSlice<'a> {
-        let ptr = if s.len() > 0 {
+        let ptr = if !s.is_empty() {
             s.as_ptr()
         } else {
             std::ptr::null()

tests/wpt-harness/expectations/fetch/api/request/request-consume-empty.any.js.json

@@ -15,7 +15,7 @@
     "status": "FAIL"
   },
   "Consume request's body as formData with correct urlencoded type": {
-    "status": "FAIL"
+    "status": "PASS"
   },
   "Consume request's body as formData without correct type (error case)": {
     "status": "PASS"

tests/wpt-harness/expectations/fetch/api/response/response-consume-empty.any.js.json

@@ -15,7 +15,7 @@
     "status": "FAIL"
   },
   "Consume response's body as formData with correct urlencoded type": {
-    "status": "FAIL"
+    "status": "PASS"
   },
   "Consume response's body as formData without correct type (error case)": {
     "status": "PASS"