Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions src/stdlib/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,25 @@ use super::helpers::{
use crate::interpreter::environment::Environment;
use crate::interpreter::error::RuntimeError;
use crate::interpreter::value::Value;
use std::borrow::Cow;
use std::cell::RefCell;
use std::rc::Rc;
use std::sync::Arc;

/// Decode percent-encoded URL string
/// Converts '+' to space and decodes %HH hex sequences
/// Invalid sequences are left as-is
fn percent_decode(s: &str) -> String {
let mut result = Vec::new();
fn percent_decode(s: &str) -> Cow<'_, str> {
let bytes = s.as_bytes();

// Optimization: avoid string allocation and decoding overhead if the string
// doesn't contain any encoded characters ('%' or '+').
// We scan bytes in a single pass to be efficient.
if !bytes.iter().any(|&b| b == b'%' || b == b'+') {
return Cow::Borrowed(s);
}
Comment on lines +16 to +24

Copilot AI Mar 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the return type/behavioral path of URL decoding (borrowed fast-path vs owned decoding) but there are no Rust tests covering parse_query_string/parse_key_value_pairs behavior (including '+' handling and invalid % sequences). Since other text stdlib features are covered by integration tests, adding coverage here would help prevent regressions when optimizing this hot path.

Copilot uses AI. Check for mistakes.

let mut result = Vec::with_capacity(bytes.len());
let mut i = 0;

while i < bytes.len() {
Expand Down Expand Up @@ -43,8 +52,10 @@ fn percent_decode(s: &str) -> String {
}

// Convert bytes to String, replacing invalid UTF-8 with replacement character
String::from_utf8(result)
.unwrap_or_else(|e| String::from_utf8_lossy(&e.into_bytes()).into_owned())
Cow::Owned(
String::from_utf8(result)
.unwrap_or_else(|e| String::from_utf8_lossy(&e.into_bytes()).into_owned()),
)
}

/// Parse key-value pairs with URL decoding
Expand Down Expand Up @@ -73,13 +84,13 @@ fn parse_key_value_pairs(
let key = if trim_parts { key.trim() } else { key };
let value = if trim_parts { value.trim() } else { value };

let decoded_key = percent_decode(key);
let decoded_key = percent_decode(key).into_owned();
let decoded_value = percent_decode(value);
params.insert(decoded_key, Value::Text(Arc::from(decoded_value)));
params.insert(decoded_key, Value::Text(Arc::from(decoded_value.as_ref())));

Copilot AI Mar 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

decoded_value is a Cow<str>, but converting it with Arc::from(decoded_value.as_ref()) forces an extra allocation+copy for the Cow::Owned case (the owned String gets dropped after being copied into a new Arc<str>). Prefer converting in a way that reuses the owned String buffer when available (e.g., branch on the Cow and use Arc::from(String) for Owned).

Suggested change
params.insert(decoded_key, Value::Text(Arc::from(decoded_value.as_ref())));
let value_arc: Arc<str> = match decoded_value {
Cow::Borrowed(s) => Arc::from(s),
Cow::Owned(s) => Arc::from(s),
};
params.insert(decoded_key, Value::Text(value_arc));

Copilot uses AI. Check for mistakes.
} else if !ignore_empty_values {
// Key without value
let key = if trim_parts { pair.trim() } else { pair };
let decoded_key = percent_decode(key);
let decoded_key = percent_decode(key).into_owned();
params.insert(decoded_key, Value::Text(Arc::from("")));
}
}
Expand Down
Loading