diff --git a/core/src/avm2/globals/string.rs b/core/src/avm2/globals/string.rs index 2f64725dc..d9340b726 100644 --- a/core/src/avm2/globals/string.rs +++ b/core/src/avm2/globals/string.rs @@ -5,7 +5,7 @@ use crate::avm2::class::{Class, ClassAttributes}; use crate::avm2::error::make_error_1004; use crate::avm2::method::{Method, NativeMethodImpl}; use crate::avm2::object::{primitive_allocator, FunctionObject, Object, TObject}; -use crate::avm2::regexp::RegExpFlags; +use crate::avm2::regexp::{RegExp, RegExpFlags}; use crate::avm2::value::Value; use crate::avm2::Error; use crate::avm2::QName; @@ -355,18 +355,18 @@ fn replace<'gc>( let this = Value::from(this).coerce_to_string(activation)?; let pattern = args.get(0).unwrap_or(&Value::Undefined); let replacement = args.get(1).unwrap_or(&Value::Undefined); - // Handles regex patterns. - if let Some(mut regexp) = pattern + + if let Some(regexp) = pattern .as_object() .as_ref() - .and_then(|o| o.as_regexp_mut(activation.context.gc_context)) + .and_then(|o| o.as_regexp_object()) { // Replacement is either a function or treatable as string. if let Some(f) = replacement.as_object().and_then(|o| o.as_function_object()) { - return Ok(regexp.replace_fn(activation, this, &f)?.into()); + return Ok(RegExp::replace_fn(regexp, activation, this, &f)?.into()); } else { let replacement = replacement.coerce_to_string(activation)?; - return Ok(regexp.replace_string(activation, this, replacement)?.into()); + return Ok(RegExp::replace_string(regexp, activation, this, replacement)?.into()); } } diff --git a/core/src/avm2/regexp.rs b/core/src/avm2/regexp.rs index 1812de25b..1982e2516 100644 --- a/core/src/avm2/regexp.rs +++ b/core/src/avm2/regexp.rs @@ -13,6 +13,8 @@ use bitflags::bitflags; use gc_arena::Collect; use ruffle_wstr::WStr; +use super::object::RegExpObject; + #[derive(Collect, Debug)] #[collect(no_drop)] pub struct RegExp<'gc> { @@ -214,12 +216,12 @@ impl<'gc> RegExp<'gc> { /// Implements string.replace(regex, replacement) where the replacement is /// a function. pub fn replace_fn( - &mut self, + regexp: RegExpObject<'gc>, activation: &mut Activation<'_, 'gc>, text: AvmString<'gc>, f: &FunctionObject<'gc>, ) -> Result, Error<'gc>> { - self.replace_with_fn(activation, &text, |activation, txt, m| { + Self::replace_with_fn(regexp, activation, &text, |activation, txt, m| { let args = std::iter::once(Some(&m.range)) .chain((m.captures.iter()).map(|x| x.as_ref())) .map(|o| match o { @@ -241,12 +243,12 @@ impl<'gc> RegExp<'gc> { /// Implements string.replace(regex, replacement) where the replacement may be /// a string with $-sequences. pub fn replace_string( - &mut self, + regexp: RegExpObject<'gc>, activation: &mut Activation<'_, 'gc>, text: AvmString<'gc>, replacement: AvmString<'gc>, ) -> Result, Error<'gc>> { - self.replace_with_fn(activation, &text, |_activation, txt, m| { + RegExp::replace_with_fn(regexp, activation, &text, |_activation, txt, m| { Ok(Self::effective_replacement(&replacement, txt, m)) }) } @@ -254,8 +256,9 @@ impl<'gc> RegExp<'gc> { // Helper for replace_string and replace_function. // // Replaces occurrences of regex with results of f(activation, &text, &match) + // Panics if regexp isn't a regexp fn replace_with_fn<'a, F>( - &mut self, + regexp: RegExpObject<'gc>, activation: &mut Activation<'_, 'gc>, text: &AvmString<'gc>, mut f: F, @@ -268,8 +271,16 @@ impl<'gc> RegExp<'gc> { ) -> Result, Error<'gc>>, { let mut start = 0; - let mut m = self.find_utf16_match(*text, start); + let (is_global, mut m) = { + // we only hold onto a mutable lock on the regular expression + // for a small window, because f might refer to the RegExp + // (See https://github.com/ruffle-rs/ruffle/issues/17899) + let mut re = regexp.as_regexp_mut(activation.gc()).unwrap(); + let global_flag = re.flags().contains(RegExpFlags::GLOBAL); + + (global_flag, re.find_utf16_match(*text, start)) + }; if m.is_none() { // Nothing to do; short circuit and just return the original string, to avoid any allocs or functions return Ok(*text); @@ -290,10 +301,16 @@ impl<'gc> RegExp<'gc> { start += 1; } - if !self.flags().contains(RegExpFlags::GLOBAL) { + if !is_global { break; } - m = self.find_utf16_match(*text, start); + // Again, here we only hold onto a mutable lock for + // the RegExp long enough to do our matching, so that + // when we call f we don't have a lock + m = regexp + .as_regexp_mut(activation.gc()) + .unwrap() + .find_utf16_match(*text, start); } ret.push_str(&text[start..]); @@ -347,7 +364,11 @@ impl<'gc> RegExp<'gc> { ArrayObject::from_storage(activation, storage) } - fn find_utf16_match(&mut self, text: AvmString<'gc>, start: usize) -> Option { + pub fn find_utf16_match( + &mut self, + text: AvmString<'gc>, + start: usize, + ) -> Option { self.find_utf8_match_at(text, start, |text, mut re_match| { // Sort the capture endpoints by increasing index, so that CachedText::utf16_index is efficient. let mut utf8_indices = re_match diff --git a/tests/tests/swfs/avm2/string_replace/Test.as b/tests/tests/swfs/avm2/string_replace/Test.as index c9831d77e..73b37f2ed 100644 --- a/tests/tests/swfs/avm2/string_replace/Test.as +++ b/tests/tests/swfs/avm2/string_replace/Test.as @@ -93,3 +93,10 @@ trace("<>".replace(/(a)(b)?|(c)/, rFN)) // The pattern is string and the replacement is a function trace("<>".replace("a", rFN)) + +trace("// regex calling into itself") + +var pattern = /simple/g +trace("this is simple, really simple.".replace(pattern, function (match) { + return match.replace(pattern, "complicated") +})) diff --git a/tests/tests/swfs/avm2/string_replace/output.txt b/tests/tests/swfs/avm2/string_replace/output.txt index 37c72163d..fae905eb4 100644 --- a/tests/tests/swfs/avm2/string_replace/output.txt +++ b/tests/tests/swfs/avm2/string_replace/output.txt @@ -39,3 +39,5 @@ undefinedbbbb // replace a regex with function, check arguments <>>> <>>> +// regex calling into itself +this is complicated, really complicated. diff --git a/tests/tests/swfs/avm2/string_replace/test.swf b/tests/tests/swfs/avm2/string_replace/test.swf index b061b576f..18dc8363c 100644 Binary files a/tests/tests/swfs/avm2/string_replace/test.swf and b/tests/tests/swfs/avm2/string_replace/test.swf differ