diff --git a/src/runtime/webcore/FormData.rs b/src/runtime/webcore/FormData.rs index f6cee67879df..e3c338b43ff5 100644 --- a/src/runtime/webcore/FormData.rs +++ b/src/runtime/webcore/FormData.rs @@ -7,8 +7,8 @@ use bun_jsc::{ AnyPromise, CallFrame, DOMFormData, JSGlobalObject, JSValue, JsError, JsResult, JsTerminated, ZigStringJsc as _, }; -use bun_semver::{self, SlicedString}; use core::ffi::c_void; +use std::borrow::Cow; use crate::webcore::Blob; use crate::webcore::BlobExt as _; @@ -79,28 +79,14 @@ impl AsyncFormDataExt for AsyncFormData { } } -/// Raw slice into the input buffer. Not using `bun.Semver.String` because -/// file bodies are binary data that can contain null bytes, which -/// Semver.String's inline storage treats as terminators. +/// Raw byte slices: `value` and `content_type` borrow the caller-owned input +/// buffer; `filename` borrows either that or a per-entry percent-decode buffer. pub struct Field<'a> { - /// Borrows into the caller-owned input buffer (binary body slice). + /// Binary body slice (may contain null bytes). pub value: &'a [u8], - pub filename: bun_semver::String, - pub content_type: bun_semver::String, + pub filename: &'a [u8], + pub content_type: &'a [u8], pub is_file: bool, - pub zero_count: u8, -} - -impl Default for Field<'_> { - fn default() -> Self { - Field { - value: b"", - filename: bun_semver::String::default(), - content_type: bun_semver::String::default(), - is_file: false, - zero_count: 0, - } - } } pub enum FieldEntry<'a> { @@ -216,18 +202,18 @@ pub fn to_js_from_multipart_data( } impl<'a> Wrapper<'a> { - fn on_entry(wrap: &mut Self, name: bun_semver::String, field: &Field<'_>, buf: &[u8]) { + fn on_entry(wrap: &mut Self, name: &[u8], field: &Field<'_>) { let value_str: &[u8] = field.value; - let key = ZigString::init_utf8(name.slice(buf)); + let key = ZigString::init_utf8(name); if field.is_file { - let filename_str = field.filename.slice(buf); + let filename_str = field.filename; let mut blob = Blob::create(value_str, wrap.global, false); let filename = ZigString::init_utf8(filename_str); if !field.content_type.is_empty() { - let ct = field.content_type.slice(buf); + let ct = field.content_type; blob.content_type .set(crate::webcore::blob::BlobContentType::Owned(ct.into())); blob.content_type_was_set.set(true); @@ -292,10 +278,9 @@ pub fn for_each_multipart_entry( input: &[u8], boundary: &[u8], ctx: &mut C, - mut iterator: impl FnMut(&mut C, bun_semver::String, &Field<'_>, &[u8]), + mut iterator: impl FnMut(&mut C, &[u8], &Field<'_>), ) -> crate::Result<()> { let mut slice = input; - let subslicer = SlicedString::init(input, input); let mut buf = [0u8; 76]; { @@ -334,12 +319,12 @@ pub fn for_each_multipart_entry( let header = &remain[..header_end + 2]; remain = &remain[header_end + 4..]; - let mut field = Field::default(); - let mut name = bun_semver::String::default(); - let mut filename: Option = None; + let mut name: &[u8] = b""; + let mut filename: Option<&[u8]> = None; + let mut content_type: &[u8] = b""; let mut header_chunk = header; let mut is_file = false; - while !header_chunk.is_empty() && (filename.is_none() || name.len() == 0) { + while !header_chunk.is_empty() && (filename.is_none() || name.is_empty()) { let line_end = strings::index_of(header_chunk, b"\r\n") .ok_or(crate::Error::IsMissingHeaderLineEnd)?; let line = &header_chunk[..line_end]; @@ -389,9 +374,9 @@ pub fn for_each_multipart_entry( } if strings::eql_case_insensitive_ascii(eql_key, b"name", true) { - name = subslicer.sub(field_value).value(); + name = field_value; } else if strings::eql_case_insensitive_ascii(eql_key, b"filename", true) { - filename = Some(subslicer.sub(field_value).value()); + filename = Some(field_value); is_file = true; } @@ -406,7 +391,7 @@ pub fn for_each_multipart_entry( } } } else if !value.is_empty() - && field.content_type.is_empty() + && content_type.is_empty() && strings::eql_case_insensitive_ascii(key, b"content-type", true) { let trimmed = strings::trim(value, b"; \t"); @@ -420,12 +405,12 @@ pub fn for_each_multipart_entry( .iter() .all(|&b| b == b'\t' || (0x20..=0x7E).contains(&b)) { - field.content_type = subslicer.sub(trimmed).value(); + content_type = trimmed; } } } - if name.len() + field.zero_count as usize == 0 { + if name.is_empty() { continue; } @@ -433,12 +418,43 @@ pub fn for_each_multipart_entry( if body.ends_with(b"\r\n") { body = &body[..body.len() - 2]; } - field.value = body; - field.filename = filename.unwrap_or_default(); - field.is_file = is_file; - iterator(ctx, name, &field, input); + // The serializer percent-escapes `"`, CR and LF in entry names and + // filenames; the parse algorithm undoes that. + let name = decode_multipart_name(name); + let filename = filename.map(decode_multipart_name); + let field = Field { + value: body, + filename: filename.as_deref().unwrap_or_default(), + content_type, + is_file, + }; + + iterator(ctx, &name, &field); } Ok(()) } + +/// Replaces `%0A`/`%0D`/`%22` (hex digits case-insensitive) in a +/// `Content-Disposition` name or filename with LF, CR and `"`. +/// +fn decode_multipart_name(raw: &[u8]) -> Cow<'_, [u8]> { + if !strings::contains_char(raw, b'%') { + return Cow::Borrowed(raw); + } + + let mut decoded = Vec::with_capacity(raw.len()); + let mut remain = raw; + while !remain.is_empty() { + let (byte, advance) = match remain { + [b'%', b'0', b'A' | b'a', ..] => (b'\n', 3), + [b'%', b'0', b'D' | b'd', ..] => (b'\r', 3), + [b'%', b'2', b'2', ..] => (b'"', 3), + _ => (remain[0], 1), + }; + decoded.push(byte); + remain = &remain[advance..]; + } + Cow::Owned(decoded) +} diff --git a/test/js/web/html/FormData-multipart-serialization.test.ts b/test/js/web/html/FormData-multipart-serialization.test.ts index b12321a2b9a8..bdc7d8fa5be5 100644 --- a/test/js/web/html/FormData-multipart-serialization.test.ts +++ b/test/js/web/html/FormData-multipart-serialization.test.ts @@ -110,6 +110,22 @@ describe("multipart serialization (new Response(formData))", () => { ); }); + // The parser must undo the `"`/CR/LF percent-escaping shown above, or Bun + // cannot read back the names it wrote into its own multipart output. + test("round-trips escaped names and filenames through Response.formData()", async () => { + const formData = new FormData(); + formData.append('quote"name', "v1"); + formData.append("crlf\r\nname", "v2"); + formData.append("file", new File(["

hi

"], 'weird"file\r\nname.html', { type: "text/html" })); + + const parsed = await new Response(formData).formData(); + + expect([...parsed.keys()]).toEqual(['quote"name', "crlf\r\nname", "file"]); + expect(parsed.get('quote"name')).toBe("v1"); + expect(parsed.get("crlf\r\nname")).toBe("v2"); + expect((parsed.get("file") as File).name).toBe('weird"file\r\nname.html'); + }); + test("round-trips every entry kind through Response.formData()", async () => { const formData = new FormData(); formData.append("simple", "value"); diff --git a/test/js/web/html/FormData.test.ts b/test/js/web/html/FormData.test.ts index 0ba5a1649f7e..e39647bd3901 100644 --- a/test/js/web/html/FormData.test.ts +++ b/test/js/web/html/FormData.test.ts @@ -136,6 +136,20 @@ describe("FormData", () => { baz: "qux", }, }, + { + // `"`, CR and LF in names arrive percent-encoded (%22 / %0D / %0A, hex + // in either case) and are decoded; other `%` sequences stay literal. + name: "percent-encoded names", + body: '--foo\r\nContent-Disposition: form-data; name="a%22b"\r\n\r\nbar\r\n--foo\r\nContent-Disposition: form-data; name="quote%22crlf%0D%0Alower%0d%0aname"\r\n\r\nbaz\r\n--foo\r\nContent-Disposition: form-data; name="keep%41%25%"\r\n\r\nqux\r\n--foo--\r\n', + headers: { + "Content-Type": "multipart/form-data; boundary=foo", + }, + expected: { + 'a"b': "bar", + 'quote"crlf\r\nlower\r\nname': "baz", + "keep%41%25%": "qux", + }, + }, { name: "with name and filename", body: '--foo\r\nContent-Disposition: form-data; name="foo"; filename="bar"\r\n\r\nbaz\r\n--foo--\r\n', @@ -884,6 +898,37 @@ it("drops multipart part Content-Type values containing control characters", asy expect(tabbed.type).toBe("text/plain;\tcharset=utf-8"); }); +it("percent-decodes %22, %0D and %0A in multipart names and filenames", async () => { + // The multipart/form-data serializer writes `"`, CR and LF in entry names + // and filenames as %22, %0D and %0A; parsing applies the inverse mapping. + // https://andreubotella.github.io/multipart-form-data/#parse-a-multipart-form-data-name + const body = + "--formboundary\r\n" + + 'Content-Disposition: form-data; name="quoted"; filename="quote%22lf%0acr%0Dname.txt"\r\n' + + "Content-Type: text/plain\r\n" + + "\r\n" + + "first\r\n" + + "--formboundary\r\n" + + 'Content-Disposition: form-data; name="literal"; filename="keep%41%25%.txt"\r\n' + + "Content-Type: text/plain\r\n" + + "\r\n" + + "second\r\n" + + "--formboundary--\r\n"; + + const formData = await new Response(body, { + headers: { "Content-Type": "multipart/form-data; boundary=formboundary" }, + }).formData(); + + const quoted = formData.get("quoted") as File; + expect(quoted.name).toBe('quote"lf\ncr\rname.txt'); + expect(await quoted.text()).toBe("first"); + + // Only %22, %0D and %0A are decoded; unrelated `%` sequences are untouched. + const literal = formData.get("literal") as File; + expect(literal.name).toBe("keep%41%25%.txt"); + expect(await literal.text()).toBe("second"); +}); + test("FormData.toJSON merges duplicate numeric field names into an array", async () => { // Field names that parse as array indices ("0", "1", ...) are stored as indexed // properties on the serialized object; appending the same numeric name more than