From 58e76098e6325ad16d552924d58c33bad6573c07 Mon Sep 17 00:00:00 2001 From: Geert-Jan Zwiers Date: Thu, 1 Sep 2022 22:20:11 +0200 Subject: [PATCH] fix(serde_v8): no panic on reading large text file (#15494) Co-authored-by: Nayeem Rahman --- cli/tests/unit/read_text_file_test.ts | 42 +++++++++++++++++++++++++++ core/runtime.rs | 7 ++++- serde_v8/ser.rs | 15 ++++++---- 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/cli/tests/unit/read_text_file_test.ts b/cli/tests/unit/read_text_file_test.ts index 169972cb42..48d246dad1 100644 --- a/cli/tests/unit/read_text_file_test.ts +++ b/cli/tests/unit/read_text_file_test.ts @@ -145,3 +145,45 @@ Deno.test( assert(data.length > 0); }, ); + +Deno.test( + { permissions: { read: true, write: true } }, + function readTextFileSyncV8LimitError() { + const kStringMaxLengthPlusOne = 536870888 + 1; + const bytes = new Uint8Array(kStringMaxLengthPlusOne); + const filePath = "cli/tests/testdata/too_big_a_file.txt"; + + Deno.writeFileSync(filePath, bytes); + + assertThrows( + () => { + Deno.readTextFileSync(filePath); + }, + TypeError, + "buffer exceeds maximum length", + ); + + Deno.removeSync(filePath); + }, +); + +Deno.test( + { permissions: { read: true, write: true } }, + async function readTextFileV8LimitError() { + const kStringMaxLengthPlusOne = 536870888 + 1; + const bytes = new Uint8Array(kStringMaxLengthPlusOne); + const filePath = "cli/tests/testdata/too_big_a_file_2.txt"; + + await Deno.writeFile(filePath, bytes); + + await assertRejects( + async () => { + await Deno.readTextFile(filePath); + }, + TypeError, + "buffer exceeds maximum length", + ); + + await Deno.remove(filePath); + }, +); diff --git a/core/runtime.rs b/core/runtime.rs index 47777099cc..d640e3e049 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -1946,7 +1946,12 @@ impl JsRuntime { for (promise_id, mut resp) in results.into_iter() { args.push(v8::Integer::new(scope, promise_id).into()); - args.push(resp.to_v8(scope).unwrap()); + args.push(match resp.to_v8(scope) { + Ok(v) => v, + Err(e) => OpResult::Err(OpError::new(&|_| "TypeError", e.into())) + .to_v8(scope) + .unwrap(), + }); } let tc_scope = &mut v8::TryCatch::new(scope); diff --git a/serde_v8/ser.rs b/serde_v8/ser.rs index 31d3e31442..3f8ad3b321 100644 --- a/serde_v8/ser.rs +++ b/serde_v8/ser.rs @@ -447,11 +447,16 @@ impl<'a, 'b, 'c> ser::Serializer for Serializer<'a, 'b, 'c> { } fn serialize_str(self, v: &str) -> JsResult<'a> { - Ok( - v8::String::new(&mut self.scope.borrow_mut(), v) - .unwrap() - .into(), - ) + let maybe_str = v8::String::new(&mut self.scope.borrow_mut(), v); + + // v8 string can return 'None' if buffer length > kMaxLength. + if let Some(str) = maybe_str { + Ok(str.into()) + } else { + Err(Error::Message(String::from( + "Cannot allocate String: buffer exceeds maximum length.", + ))) + } } fn serialize_bytes(self, v: &[u8]) -> JsResult<'a> {