Skip to content

Commit ccc3507

Browse files
committed
feat: update recursion depth limit to 60 and enhance deep recursion safety tests
1 parent 05d716c commit ccc3507

File tree

7 files changed

+211
-110
lines changed

7 files changed

+211
-110
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
/target
2+
/vendor
23
/benchmark/vendor
34
/benchmark/composer.lock

Dockerfile.prod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ COPY --from=composer:latest /usr/bin/composer /usr/bin/composer
1616
WORKDIR /app
1717

1818
# Download and install the php-rs-toon extension from GitHub release
19-
ARG TOON_VERSION=1.0.0
19+
ARG TOON_VERSION=1.0.1
2020
RUN apt-get update && apt-get install -y unzip && rm -rf /var/lib/apt/lists/* && \
2121
curl -L https://github.com/mesak/php-rs-toon/releases/download/v${TOON_VERSION}/php-rs-toon-linux-x86_64.zip \
2222
-o /tmp/extension.zip && \

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ docker run --rm php-rs-toon:bench
143143
**Performance Results**:
144144
- **10-30x faster** than pure PHP implementation
145145
- **Optimized memory usage** with pre-allocation
146-
- **Recursion depth protection** (max depth: 100)
146+
- **Recursion depth protection** (max depth: 60)
147147

148148
---
149149

README.zh_TW.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ docker run --rm php-rs-toon:bench
143143
**性能結果**
144144
- **快 10-30 倍** 相較於純 PHP 實作
145145
- **最佳化記憶體使用** 採用預分配策略
146-
- **遞迴深度保護** (最大深度: 100)
146+
- **遞迴深度保護** (最大深度: 60)
147147

148148
---
149149

@@ -270,7 +270,7 @@ try {
270270

271271
**回傳:** TOON 格式字串
272272

273-
**例外:** 超過遞迴深度限制 (>100) 時拋出例外
273+
**例外:** 超過遞迴深度限制 (>60) 時拋出例外
274274

275275
---
276276

src/lib.rs

Lines changed: 124 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
1-
use ext_php_rs::prelude::*;
2-
use ext_php_rs::types::{Zval, ArrayKey};
1+
use std::ffi::CString;
2+
use std::mem;
3+
4+
use ext_php_rs::boxed::ZBox;
5+
use ext_php_rs::ffi::{zend_hash_index_update, zend_hash_next_index_insert, zend_hash_str_update};
36
use ext_php_rs::internal::function::PhpFunction;
7+
use ext_php_rs::prelude::*;
8+
use ext_php_rs::types::{ArrayKey, ZendHashTable, Zval};
49

510
pub mod toon;
611
use toon::ToonValue;
@@ -30,17 +35,19 @@ pub fn get_module(module: ModuleBuilder) -> ModuleBuilder {
3035

3136
// --- Helpers ---
3237

33-
const MAX_RECURSION_DEPTH: usize = 100;
38+
const MAX_RECURSION_DEPTH: usize = 60;
3439

3540
fn toon_value_to_zval(val: ToonValue) -> PhpResult<Zval> {
3641
toon_value_to_zval_impl(val, 0)
3742
}
3843

3944
fn toon_value_to_zval_impl(val: ToonValue, depth: usize) -> PhpResult<Zval> {
4045
if depth > MAX_RECURSION_DEPTH {
41-
return Err(PhpException::default("Recursion depth limit exceeded".to_string()));
46+
return Err(PhpException::default(
47+
"Recursion depth limit exceeded".to_string(),
48+
));
4249
}
43-
50+
4451
let mut zval = Zval::new();
4552
match val {
4653
ToonValue::Null => zval.set_null(),
@@ -49,18 +56,12 @@ fn toon_value_to_zval_impl(val: ToonValue, depth: usize) -> PhpResult<Zval> {
4956
ToonValue::Float(f) => zval.set_double(f),
5057
ToonValue::String(s) => zval.set_string(&s, false)?,
5158
ToonValue::Array(arr) => {
52-
let mut vec = Vec::with_capacity(arr.len());
53-
for item in arr {
54-
vec.push(toon_value_to_zval_impl(item, depth + 1)?);
55-
}
56-
zval.set_array(vec).map_err(|e| PhpException::default(e.to_string()))?;
59+
let ht = build_php_list(arr, depth + 1)?;
60+
zval.set_hashtable(ht);
5761
}
5862
ToonValue::Map(map) => {
59-
let mut entries = Vec::with_capacity(map.len());
60-
for (k, v) in map {
61-
entries.push((k, toon_value_to_zval_impl(v, depth + 1)?));
62-
}
63-
zval.set_array(entries).map_err(|e| PhpException::default(e.to_string()))?;
63+
let ht = build_php_map(map, depth + 1)?;
64+
zval.set_hashtable(ht);
6465
}
6566
}
6667
Ok(zval)
@@ -72,9 +73,11 @@ fn zval_to_toon_value(zval: &Zval) -> PhpResult<ToonValue> {
7273

7374
fn zval_to_toon_value_impl(zval: &Zval, depth: usize) -> PhpResult<ToonValue> {
7475
if depth > MAX_RECURSION_DEPTH {
75-
return Err(PhpException::default("Recursion depth limit exceeded".to_string()));
76+
return Err(PhpException::default(
77+
"Recursion depth limit exceeded".to_string(),
78+
));
7679
}
77-
80+
7881
if zval.is_null() {
7982
return Ok(ToonValue::Null);
8083
}
@@ -96,65 +99,121 @@ fn zval_to_toon_value_impl(zval: &Zval, depth: usize) -> PhpResult<ToonValue> {
9699
if zval.is_array() {
97100
let ht = zval.array().unwrap();
98101
let len = ht.len();
99-
100-
// Check if it's a sequential array (list) or associative (map)
101-
// PHP arrays are always ordered maps, but TOON distinguishes between lists and maps.
102-
// Heuristic: if all keys are sequential integers starting from 0, treat as Array.
103-
// Otherwise treat as Map.
104-
105-
let mut is_list = true;
106-
let mut expected_idx = 0;
107-
let mut entries = Vec::with_capacity(len);
102+
103+
// Detect list candidacy and defer allocating map storage until required
104+
let mut expected_idx = 0usize;
105+
let mut is_list_candidate = true;
108106
let mut has_complex = false;
109-
110-
// Single pass: check list conditions and complex types simultaneously
111-
// Optimized: single pattern match for key checking and conversion
107+
let mut list_items: Vec<ToonValue> = Vec::with_capacity(len);
108+
let mut map_entries: Option<Vec<(String, ToonValue)>> = None;
109+
112110
for (k, v) in ht.iter() {
113-
let val = zval_to_toon_value_impl(v, depth + 1)?;
114-
115-
// Check if value is complex (Map or Array) during iteration
116-
if !has_complex && matches!(val, ToonValue::Map(_) | ToonValue::Array(_)) {
117-
has_complex = true;
118-
}
119-
120-
// Single pattern match combines list check and key conversion
121-
let key_str = match k {
122-
ArrayKey::Long(idx) => {
123-
// Check if array is sequential during key extraction
124-
if is_list {
125-
if idx != expected_idx as i64 {
126-
is_list = false;
127-
} else {
128-
expected_idx += 1;
129-
}
111+
let mut treat_as_list_entry = is_list_candidate;
112+
113+
if treat_as_list_entry {
114+
match k {
115+
ArrayKey::Long(idx) if idx == expected_idx as i64 => {
116+
expected_idx += 1;
117+
}
118+
_ => {
119+
treat_as_list_entry = false;
120+
is_list_candidate = false;
130121
}
131-
idx.to_string()
132122
}
133-
ArrayKey::String(s) => {
134-
is_list = false;
135-
s
123+
}
124+
125+
let val = zval_to_toon_value_impl(v, depth + 1)?;
126+
let value_is_complex = matches!(val, ToonValue::Map(_) | ToonValue::Array(_));
127+
if value_is_complex {
128+
has_complex = true;
129+
if treat_as_list_entry {
130+
treat_as_list_entry = false;
131+
is_list_candidate = false;
136132
}
137-
ArrayKey::Str(s) => {
138-
is_list = false;
139-
s
133+
}
134+
135+
if treat_as_list_entry {
136+
list_items.push(val);
137+
continue;
138+
}
139+
140+
let entries = map_entries.get_or_insert_with(|| {
141+
let mut vec = Vec::with_capacity(len);
142+
for (idx, prev_val) in list_items.drain(..).enumerate() {
143+
vec.push((idx.to_string(), prev_val));
140144
}
145+
vec
146+
});
147+
148+
let key_str = match k {
149+
ArrayKey::Long(idx) => idx.to_string(),
150+
ArrayKey::String(s) => s,
151+
ArrayKey::Str(s) => s.to_string(),
141152
};
142153
entries.push((key_str, val));
143154
}
144-
145-
// Decision: return Array or Map
146-
if is_list && !entries.is_empty() && !has_complex {
147-
// Extract values without cloning (move ownership)
148-
let items = entries.into_iter().map(|(_, v)| v).collect();
149-
return Ok(ToonValue::Array(items));
150-
} else if entries.is_empty() {
151-
// Empty PHP array defaults to empty list
152-
return Ok(ToonValue::Array(Vec::new()));
155+
156+
if is_list_candidate && !has_complex {
157+
return Ok(ToonValue::Array(list_items));
153158
}
154-
155-
return Ok(ToonValue::Map(entries));
159+
160+
if let Some(entries) = map_entries {
161+
return Ok(ToonValue::Map(entries));
162+
}
163+
164+
// Empty PHP arrays default to empty lists
165+
return Ok(ToonValue::Array(Vec::new()));
156166
}
157-
167+
158168
// Fallback
159169
Ok(ToonValue::String(zval.string().unwrap_or_default()))
160170
}
171+
172+
fn build_php_list(items: Vec<ToonValue>, depth: usize) -> PhpResult<ZBox<ZendHashTable>> {
173+
let mut ht = ZendHashTable::with_capacity(clamped_capacity(items.len()));
174+
for item in items {
175+
let mut child = toon_value_to_zval_impl(item, depth)?;
176+
unsafe {
177+
zend_hash_next_index_insert(&mut *ht, std::ptr::addr_of_mut!(child));
178+
}
179+
mem::forget(child);
180+
}
181+
Ok(ht)
182+
}
183+
184+
fn build_php_map(
185+
entries: Vec<(String, ToonValue)>,
186+
depth: usize,
187+
) -> PhpResult<ZBox<ZendHashTable>> {
188+
let mut ht = ZendHashTable::with_capacity(clamped_capacity(entries.len()));
189+
for (key, value) in entries {
190+
let mut child = toon_value_to_zval_impl(value, depth)?;
191+
let maybe_index = key.parse::<i64>().ok();
192+
unsafe {
193+
if let Some(idx) = maybe_index {
194+
#[allow(clippy::cast_sign_loss)]
195+
zend_hash_index_update(&mut *ht, idx as u64, std::ptr::addr_of_mut!(child));
196+
} else {
197+
let c_key = CString::new(key.as_str())
198+
.map_err(|_| PhpException::default("Map key contains null byte".to_string()))?;
199+
zend_hash_str_update(
200+
&mut *ht,
201+
c_key.as_ptr(),
202+
key.len(),
203+
std::ptr::addr_of_mut!(child),
204+
);
205+
}
206+
}
207+
mem::forget(child);
208+
}
209+
Ok(ht)
210+
}
211+
212+
fn clamped_capacity(len: usize) -> u32 {
213+
let max = u32::MAX as usize;
214+
if len > max {
215+
u32::MAX
216+
} else {
217+
len as u32
218+
}
219+
}

0 commit comments

Comments
 (0)