From f8745d50766a2c34e3c75f351c3523a92b2db33f Mon Sep 17 00:00:00 2001 From: Inseok Lee Date: Sat, 25 Jul 2026 18:25:50 +0900 Subject: [PATCH] Match the JDK on regionMatches and drop leftover io copies regionMatches returned false for a negative len, but the JDK's bounds test widens to long and its comparison loop simply never runs, so an otherwise in-range region reports a match. Widening the check here too keeps a len near i32::MAX from overflowing the offset arithmetic. BufferedReader.readLine() and DataInputStream.readUTF() built an exactly sized char array and then handed it to a copying constructor. They now go through JavaLangString::from_utf16, which takes the sharing constructor, so each line and each UTF entry costs one array copy instead of two. --- .../src/classes/java/io/buffered_reader.rs | 14 ++---- .../src/classes/java/io/data_input_stream.rs | 8 ++-- java_runtime/src/classes/java/lang/string.rs | 11 +++-- .../tests/classes/java/lang/test_string.rs | 45 ++++++++++++++++++- 4 files changed, 57 insertions(+), 21 deletions(-) diff --git a/java_runtime/src/classes/java/io/buffered_reader.rs b/java_runtime/src/classes/java/io/buffered_reader.rs index 478c1c0b..5b1770e3 100644 --- a/java_runtime/src/classes/java/io/buffered_reader.rs +++ b/java_runtime/src/classes/java/io/buffered_reader.rs @@ -4,7 +4,7 @@ use alloc::{vec, vec::Vec}; use java_class_proto::{JavaFieldProto, JavaMethodProto}; use java_constants::{ClassAccessFlags, FieldAccessFlags, MethodAccessFlags}; -use jvm::{Array, ClassInstanceRef, JavaChar, Jvm, Result}; +use jvm::{Array, ClassInstanceRef, JavaChar, Jvm, Result, runtime::JavaLangString}; use crate::{ RuntimeClassProto, RuntimeContext, @@ -336,11 +336,7 @@ impl BufferedReader { return Ok(None.into()); } - let line_length = line.len(); - let mut chars = jvm.instantiate_array("C", line_length).await?; - jvm.store_array(&mut chars, 0, line).await?; - let value = jvm.new_class("java/lang/String", "([CII)V", (chars, 0, line_length as i32)).await?; - return Ok(value.into()); + return Ok(JavaLangString::from_utf16(jvm, line).await?.into()); } next_char = jvm.get_field(&this, "nextChar", "I").await?; n_chars = jvm.get_field(&this, "nChars", "I").await?; @@ -367,11 +363,7 @@ impl BufferedReader { jvm.put_field(&mut this, "skipLF", "Z", true).await?; } - let line_length = line.len(); - let mut chars = jvm.instantiate_array("C", line_length).await?; - jvm.store_array(&mut chars, 0, line).await?; - let value = jvm.new_class("java/lang/String", "([CII)V", (chars, 0, line_length as i32)).await?; - return Ok(value.into()); + return Ok(JavaLangString::from_utf16(jvm, line).await?.into()); } line.extend(buffered); diff --git a/java_runtime/src/classes/java/io/data_input_stream.rs b/java_runtime/src/classes/java/io/data_input_stream.rs index 480c6f9e..1d2eb426 100644 --- a/java_runtime/src/classes/java/io/data_input_stream.rs +++ b/java_runtime/src/classes/java/io/data_input_stream.rs @@ -2,7 +2,7 @@ use alloc::{vec, vec::Vec}; use java_class_proto::JavaMethodProto; use java_constants::MethodAccessFlags; -use jvm::{Array, ClassInstanceRef, JavaChar, Jvm, Result}; +use jvm::{Array, ClassInstanceRef, JavaChar, Jvm, Result, runtime::JavaLangString}; use crate::{ RuntimeClassProto, RuntimeContext, @@ -156,7 +156,7 @@ impl DataInputStream { tracing::debug!("java.io.DataInputStream::readUTF({this:?})"); let length: i32 = jvm.invoke_virtual(&this, "readUnsignedShort", "()I", ()).await?; - let mut java_array = jvm.instantiate_array("B", length as usize).await?; + let java_array = jvm.instantiate_array("B", length as usize).await?; let _: () = jvm.invoke_virtual(&this, "readFully", "([BII)V", (java_array.clone(), 0, length)).await?; let bytes: Vec = jvm.load_array(&java_array, 0, length as usize).await?; let bytes: Vec = bytes.into_iter().map(|value| value as u8).collect(); @@ -192,9 +192,7 @@ impl DataInputStream { } } - java_array = jvm.instantiate_array("C", chars.len()).await?; - jvm.store_array(&mut java_array, 0, chars).await?; - Ok(jvm.new_class("java/lang/String", "([C)V", (java_array,)).await?.into()) + Ok(JavaLangString::from_utf16(jvm, chars).await?.into()) } async fn read_utf_from_input(jvm: &Jvm, _: &mut RuntimeContext, input: ClassInstanceRef) -> Result> { diff --git a/java_runtime/src/classes/java/lang/string.rs b/java_runtime/src/classes/java/lang/string.rs index cf7a9b12..41d084d8 100644 --- a/java_runtime/src/classes/java/lang/string.rs +++ b/java_runtime/src/classes/java/lang/string.rs @@ -922,17 +922,20 @@ impl String { return Err(jvm.exception("java/lang/NullPointerException", "other is null").await); } - if toffset < 0 || ooffset < 0 || len < 0 { + if toffset < 0 || ooffset < 0 { return Ok(false); } let (this_value, this_offset, this_count) = Self::value_range(jvm, &this).await?; let (other_value, other_offset, other_count) = Self::value_range(jvm, &other).await?; - let end_t = toffset as usize + len as usize; - let end_o = ooffset as usize + len as usize; - if end_t > this_count || end_o > other_count { + // widened like the jdk does, so a len near i32::MAX fails the bounds test instead of overflowing + if toffset as i64 > this_count as i64 - len as i64 || ooffset as i64 > other_count as i64 - len as i64 { return Ok(false); } + // the jdk's comparison loop never runs for a non-positive len, so an in-range region trivially matches + if len <= 0 { + return Ok(true); + } let this_chars: Vec = jvm.load_array(&this_value, this_offset + toffset as usize, len as usize).await?; let other_chars: Vec = jvm.load_array(&other_value, other_offset + ooffset as usize, len as usize).await?; diff --git a/java_runtime/tests/classes/java/lang/test_string.rs b/java_runtime/tests/classes/java/lang/test_string.rs index 2ffde214..10e9dbf1 100644 --- a/java_runtime/tests/classes/java/lang/test_string.rs +++ b/java_runtime/tests/classes/java/lang/test_string.rs @@ -1182,7 +1182,7 @@ async fn test_str_06_region_matches_without_ignore_case() -> Result<()> { .await? ); assert!( - !jvm.invoke_virtual::<_, bool>(&source, "regionMatches", "(ILjava/lang/String;II)Z", (1, same, 1, -1)) + jvm.invoke_virtual::<_, bool>(&source, "regionMatches", "(ILjava/lang/String;II)Z", (1, same, 1, -1)) .await? ); @@ -1209,6 +1209,49 @@ async fn test_str_06_region_matches_without_ignore_case() -> Result<()> { Ok(()) } +#[tokio::test] +async fn test_region_matches_non_positive_len() -> Result<()> { + let jvm = test_jvm().await?; + let source = JavaLangString::from_rust_string(&jvm, "Hello").await?; + let other = JavaLangString::from_rust_string(&jvm, "World").await?; + + for len in [0, -1, i32::MIN] { + assert!( + jvm.invoke_virtual::<_, bool>(&source, "regionMatches", "(ILjava/lang/String;II)Z", (1, other.clone(), 2, len)) + .await? + ); + assert!( + jvm.invoke_virtual::<_, bool>(&source, "regionMatches", "(ZILjava/lang/String;II)Z", (true, 1, other.clone(), 2, len)) + .await? + ); + } + + assert!( + jvm.invoke_virtual::<_, bool>(&source, "regionMatches", "(ILjava/lang/String;II)Z", (5, other.clone(), 5, 0)) + .await? + ); + assert!( + !jvm.invoke_virtual::<_, bool>(&source, "regionMatches", "(ILjava/lang/String;II)Z", (6, other.clone(), 0, 0)) + .await? + ); + assert!( + !jvm.invoke_virtual::<_, bool>(&source, "regionMatches", "(ILjava/lang/String;II)Z", (0, other.clone(), 0, i32::MAX)) + .await? + ); + + let sub: ClassInstanceRef = jvm.invoke_virtual(&source, "substring", "(II)Ljava/lang/String;", (1, 3)).await?; + assert!( + jvm.invoke_virtual::<_, bool>(&sub, "regionMatches", "(ILjava/lang/String;II)Z", (2, other, 0, -1)) + .await? + ); + assert!( + !jvm.invoke_virtual::<_, bool>(&sub, "regionMatches", "(ILjava/lang/String;II)Z", (3, source, 0, 0)) + .await? + ); + + Ok(()) +} + #[tokio::test] async fn test_str_07_locale_case_overloads_and_float_formatting() -> Result<()> { let jvm = test_jvm().await?;