fix(native): resolve memory leaks in glob, ttsr, and image overflow (#2170)
Address three critical safety issues found during codebase audit: - glob.rs: Explicitly drop ThreadsafeFunction after glob operation completes to release the N-API reference immediately instead of relying on implicit drop ordering. - ttsr.rs: Add handle bounds validation in ttsrCheckBuffer, recover from mutex poisoning via unwrap_or_else instead of returning errors, cap live handles at 10,000 to prevent unbounded growth, and add ttsrClearAll for bulk cleanup. - image.rs: Replace unchecked (w * h * N) as usize casts with checked_mul arithmetic that returns a descriptive error instead of panicking on overflow.
This commit is contained in:
parent
620f840210
commit
efebd29857
3 changed files with 53 additions and 19 deletions
|
|
@ -254,7 +254,7 @@ pub fn glob(
|
|||
let ct = task::CancelToken::new(timeout_ms);
|
||||
|
||||
task::blocking("glob", ct, move |ct| {
|
||||
run_glob(
|
||||
let result = run_glob(
|
||||
GlobConfig {
|
||||
root: fs_cache::resolve_search_path(&path)?,
|
||||
include_hidden: hidden.unwrap_or(false),
|
||||
|
|
@ -270,6 +270,10 @@ pub fn glob(
|
|||
},
|
||||
on_match.as_ref(),
|
||||
ct,
|
||||
)
|
||||
);
|
||||
// Explicitly drop the ThreadsafeFunction to release the N-API reference
|
||||
// immediately rather than relying on implicit drop ordering.
|
||||
drop(on_match);
|
||||
result
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -103,31 +103,42 @@ fn decode_image_from_bytes(bytes: &[u8]) -> Result<DynamicImage> {
|
|||
.map_err(|e| Error::from_reason(format!("Failed to decode image: {e}")))
|
||||
}
|
||||
|
||||
/// Compute a capacity hint for the encode buffer using checked arithmetic.
|
||||
///
|
||||
/// Returns an error instead of panicking when `w * h * bytes_per_pixel`
|
||||
/// overflows `usize`.
|
||||
fn encode_capacity(w: u32, h: u32, bytes_per_pixel: usize) -> Result<usize> {
|
||||
(w as usize)
|
||||
.checked_mul(h as usize)
|
||||
.and_then(|wh| wh.checked_mul(bytes_per_pixel))
|
||||
.ok_or_else(|| Error::from_reason("Image dimensions too large for encode buffer"))
|
||||
}
|
||||
|
||||
fn encode_image(img: &DynamicImage, format: u8, quality: u8) -> Result<Vec<u8>> {
|
||||
let (w, h) = (img.width(), img.height());
|
||||
match format {
|
||||
0 => {
|
||||
let mut buffer = Vec::with_capacity((w * h * 4) as usize);
|
||||
let mut buffer = Vec::with_capacity(encode_capacity(w, h, 4)?);
|
||||
img.write_to(&mut Cursor::new(&mut buffer), ImageFormat::Png)
|
||||
.map_err(|e| Error::from_reason(format!("Failed to encode PNG: {e}")))?;
|
||||
Ok(buffer)
|
||||
},
|
||||
1 => {
|
||||
let mut buffer = Vec::with_capacity((w * h * 3) as usize);
|
||||
let mut buffer = Vec::with_capacity(encode_capacity(w, h, 3)?);
|
||||
let encoder = JpegEncoder::new_with_quality(&mut buffer, quality);
|
||||
img.write_with_encoder(encoder)
|
||||
.map_err(|e| Error::from_reason(format!("Failed to encode JPEG: {e}")))?;
|
||||
Ok(buffer)
|
||||
},
|
||||
2 => {
|
||||
let mut buffer = Vec::with_capacity((w * h * 4) as usize);
|
||||
let mut buffer = Vec::with_capacity(encode_capacity(w, h, 4)?);
|
||||
let encoder = WebPEncoder::new_lossless(&mut buffer);
|
||||
img.write_with_encoder(encoder)
|
||||
.map_err(|e| Error::from_reason(format!("Failed to encode WebP: {e}")))?;
|
||||
Ok(buffer)
|
||||
},
|
||||
3 => {
|
||||
let mut buffer = Vec::with_capacity((w * h) as usize);
|
||||
let mut buffer = Vec::with_capacity(encode_capacity(w, h, 1)?);
|
||||
img.write_to(&mut Cursor::new(&mut buffer), ImageFormat::Gif)
|
||||
.map_err(|e| Error::from_reason(format!("Failed to encode GIF: {e}")))?;
|
||||
Ok(buffer)
|
||||
|
|
|
|||
|
|
@ -34,6 +34,15 @@ pub struct NapiTtsrRuleInput {
|
|||
pub conditions: Vec<String>,
|
||||
}
|
||||
|
||||
/// Maximum number of live handles allowed before we refuse to allocate more.
|
||||
/// Prevents unbounded memory growth if JS callers forget to free handles.
|
||||
const MAX_LIVE_HANDLES: usize = 10_000;
|
||||
|
||||
/// Lock the global STORE, recovering gracefully from mutex poisoning.
|
||||
fn lock_store() -> std::sync::MutexGuard<'static, HashMap<u64, CompiledRuleSet>> {
|
||||
STORE.lock().unwrap_or_else(|e| e.into_inner())
|
||||
}
|
||||
|
||||
/// Compile a set of TTSR rules into an optimized regex engine.
|
||||
///
|
||||
/// Returns an opaque numeric handle. Each rule has one or more regex condition
|
||||
|
|
@ -69,10 +78,13 @@ pub fn ttsr_compile_rules(rules: Vec<NapiTtsrRuleInput>) -> Result<f64> {
|
|||
mappings,
|
||||
};
|
||||
|
||||
STORE
|
||||
.lock()
|
||||
.map_err(|e| Error::from_reason(format!("Lock poisoned: {e}")))?
|
||||
.insert(handle, compiled);
|
||||
let mut store = lock_store();
|
||||
if store.len() >= MAX_LIVE_HANDLES {
|
||||
return Err(Error::from_reason(format!(
|
||||
"TTSR handle limit reached ({MAX_LIVE_HANDLES}). Free unused handles before compiling more rules."
|
||||
)));
|
||||
}
|
||||
store.insert(handle, compiled);
|
||||
|
||||
// Return as f64 since napi BigInt interop is awkward; handles won't exceed 2^53.
|
||||
Ok(handle as f64)
|
||||
|
|
@ -86,9 +98,13 @@ pub fn ttsr_compile_rules(rules: Vec<NapiTtsrRuleInput>) -> Result<f64> {
|
|||
pub fn ttsr_check_buffer(handle: f64, buffer: String) -> Result<Vec<String>> {
|
||||
let handle_key = handle as u64;
|
||||
|
||||
let store = STORE
|
||||
.lock()
|
||||
.map_err(|e| Error::from_reason(format!("Lock poisoned: {e}")))?;
|
||||
// Bounds-check: reject handles that were never allocated.
|
||||
let upper_bound = NEXT_HANDLE.load(Ordering::Relaxed);
|
||||
if handle_key == 0 || handle_key >= upper_bound {
|
||||
return Err(Error::from_reason(format!("Invalid TTSR handle: {handle}")));
|
||||
}
|
||||
|
||||
let store = lock_store();
|
||||
|
||||
let compiled = store
|
||||
.get(&handle_key)
|
||||
|
|
@ -114,11 +130,14 @@ pub fn ttsr_check_buffer(handle: f64, buffer: String) -> Result<Vec<String>> {
|
|||
#[napi(js_name = "ttsrFreeRules")]
|
||||
pub fn ttsr_free_rules(handle: f64) -> Result<()> {
|
||||
let handle_key = handle as u64;
|
||||
|
||||
STORE
|
||||
.lock()
|
||||
.map_err(|e| Error::from_reason(format!("Lock poisoned: {e}")))?
|
||||
.remove(&handle_key);
|
||||
|
||||
lock_store().remove(&handle_key);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Free all compiled TTSR rule sets, releasing all memory.
|
||||
///
|
||||
/// Useful for process cleanup or tests that need a fresh state.
|
||||
#[napi(js_name = "ttsrClearAll")]
|
||||
pub fn ttsr_clear_all() {
|
||||
lock_store().clear();
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue