Skip to content
Merged
Show file tree
Hide file tree
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
7 changes: 2 additions & 5 deletions bitreq/fuzz/src/url_parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub fn do_test(data: &[u8]) {
{
let mut url_clone = bitreq_url.clone();
// Use the input itself as both key and value to exercise encoding
url_clone.append_query_params([(input.to_string(), input.to_string())]);
url_clone.append_query_params([(&*input, &*input)]);
// Verify the URL is still valid by accessing its fields
let _ = url_clone.query();
let _ = url_clone.as_str();
Expand All @@ -98,10 +98,7 @@ pub fn do_test(data: &[u8]) {

// Test appending multiple params in one call
let mut url_clone3 = bitreq_url.clone();
url_clone3.append_query_params([
("key1".into(), "value1".into()),
("key2".into(), input.to_string()),
]);
url_clone3.append_query_params([("key1", "value1"), ("key2", &input)]);
let _ = url_clone3.as_str();
}

Expand Down
3 changes: 2 additions & 1 deletion bitreq/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,8 @@ impl ParsedRequest {
#[allow(unused_mut)]
pub(crate) fn new(mut config: Request) -> Result<ParsedRequest, Error> {
let mut url = Url::parse(&config.url)?;
url.append_query_params(config.params.drain(..));
let params = config.params.iter().map(|(a, b)| (a.as_str(), b.as_str()));
url.append_query_params(params);

#[cfg(all(feature = "proxy", feature = "std"))]
// Set default proxy from environment variables
Expand Down
152 changes: 65 additions & 87 deletions bitreq/src/url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,6 @@ use alloc::vec::Vec;
use core::fmt;
use core::ops::Range;

/// Macro to conditionally set visibility to `pub` when fuzzing, `pub(crate)` otherwise.
/// Used to expose internal methods for fuzz testing without making them part of the public API.
macro_rules! pub_if_fuzzing {
(
$(#[$attr:meta])*
fn $name:ident($($args:tt)*) $body:tt
) => {
$(#[$attr])*
#[cfg(fuzzing)]
pub fn $name($($args)*) $body

$(#[$attr])*
#[cfg(not(fuzzing))]
pub(crate) fn $name($($args)*) $body
};
}

/// Returns the default port for known schemes, or `None` for unknown schemes.
fn default_port_for_scheme(scheme: &str) -> Option<u16> {
match scheme {
Expand Down Expand Up @@ -428,71 +411,70 @@ impl Url {
}
}

pub_if_fuzzing! {
/// Appends query parameters from an iterator to the URL.
///
/// Keys and values are percent-encoded before being appended.
/// If the URL already has a query string, parameters are appended with `&`.
/// Otherwise, the first parameter is appended with `?`.
///
/// Only calls `parse_inner` once after all parameters have been appended.
fn append_query_params(&mut self, params: impl IntoIterator<Item = (String, String)>) {
let mut params = params.into_iter().peekable();
if params.peek().is_none() {
return;
}

// Extract fragment string and compute base (everything before '#')
let (mut new_serialization, fragment) = if let Some(ref frag_range) = self.fragment {
let frag = self.serialization[frag_range.clone()].to_string();
let base = self.serialization[..frag_range.start - 1].to_string(); // -1 for '#'
(base, Some(frag))
} else {
(self.serialization.clone(), None)
};
/// Appends query parameters from an iterator to the URL.
///
/// Keys and values are percent-encoded before being appended.
/// If the URL already has a query string, parameters are appended with `&`.
/// Otherwise, the first parameter is appended with `?`.
///
/// Only calls `parse_inner` once after all parameters have been appended.
pub fn append_query_params<'a>(
&mut self,
params: impl IntoIterator<Item = (&'a str, &'a str)>,
) {
let mut params = params.into_iter().peekable();
if params.peek().is_none() {
return;
}

let mut has_query = self.query.is_some();
// Extract fragment string and compute base (everything before '#')
let (mut new_serialization, fragment) = if let Some(ref frag_range) = self.fragment {
let frag = self.serialization[frag_range.clone()].to_string();
let base = self.serialization[..frag_range.start - 1].to_string(); // -1 for '#'
(base, Some(frag))
} else {
(self.serialization.clone(), None)
};

for (key, value) in params {
let encoded_key = percent_encode_string(&key);
let encoded_value = percent_encode_string(&value);
if has_query {
new_serialization.push('&');
} else {
new_serialization.push('?');
has_query = true;
}
new_serialization.push_str(&encoded_key);
new_serialization.push('=');
new_serialization.push_str(&encoded_value);
}
let mut has_query = self.query.is_some();

if let Some(frag) = fragment {
new_serialization.push('#');
new_serialization.push_str(&frag);
for (key, value) in params {
let encoded_key = percent_encode_string(key);
let encoded_value = percent_encode_string(value);
if has_query {
new_serialization.push('&');
} else {
new_serialization.push('?');
has_query = true;
}
new_serialization.push_str(&encoded_key);
new_serialization.push('=');
new_serialization.push_str(&encoded_value);
}

// Reparse to update all fields
*self =
Self::parse_inner(new_serialization).expect("append_query_params produced invalid URL");
if let Some(frag) = fragment {
new_serialization.push('#');
new_serialization.push_str(&frag);
}

// Reparse to update all fields
*self =
Self::parse_inner(new_serialization).expect("append_query_params produced invalid URL");
}

pub_if_fuzzing! {
/// If this URL has no fragment but `other` does, copies the fragment from `other`.
///
/// This implements RFC 7231 section 7.1.2 behavior for preserving fragments
/// across redirects.
fn preserve_fragment_from(&mut self, other: &Url) {
if self.fragment.is_some() {
return;
}
/// If this URL has no fragment but `other` does, copies the fragment from `other`.
///
/// This implements RFC 7231 section 7.1.2 behavior for preserving fragments
/// across redirects.
pub fn preserve_fragment_from(&mut self, other: &Url) {
if self.fragment.is_some() {
return;
}

if let Some(other_frag) = other.fragment() {
let new_serialization = format!("{}#{}", &self.serialization, other_frag);
*self = Self::parse_inner(new_serialization)
.expect("preserve_fragment_from produced invalid URL");
}
if let Some(other_frag) = other.fragment() {
let new_serialization = format!("{}#{}", &self.serialization, other_frag);
*self = Self::parse_inner(new_serialization)
.expect("preserve_fragment_from produced invalid URL");
}
}

Expand Down Expand Up @@ -999,37 +981,37 @@ mod tests {
#[test]
fn append_query_params_to_url_without_query() {
let mut url = Url::parse("http://example.com/path").unwrap();
url.append_query_params([("foo".into(), "bar".into())]);
url.append_query_params([("foo", "bar")]);
assert_eq!(url.query(), Some("foo=bar"));
assert_eq!(url.as_str(), "http://example.com/path?foo=bar");
}

#[test]
fn append_query_params_to_url_with_existing_query() {
let mut url = Url::parse("http://example.com/path?existing=value").unwrap();
url.append_query_params([("foo".into(), "bar".into())]);
url.append_query_params([("foo", "bar")]);
assert_eq!(url.query(), Some("existing=value&foo=bar"));
assert_eq!(url.as_str(), "http://example.com/path?existing=value&foo=bar");
}

#[test]
fn append_query_params_encodes_special_chars() {
let mut url = Url::parse("http://example.com").unwrap();
url.append_query_params([("key with spaces".into(), "value&special=chars".into())]);
url.append_query_params([("key with spaces", "value&special=chars")]);
assert_eq!(url.query(), Some("key%20with%20spaces=value%26special%3Dchars"));
}

#[test]
fn append_query_params_encodes_unicode() {
let mut url = Url::parse("http://example.com").unwrap();
url.append_query_params([("ówò".into(), "what's this? 👀".into())]);
url.append_query_params([("ówò", "what's this? 👀")]);
assert_eq!(url.query(), Some("%C3%B3w%C3%B2=what%27s%20this%3F%20%F0%9F%91%80"));
}

#[test]
fn append_query_params_preserves_fragment() {
let mut url = Url::parse("http://example.com/path#section").unwrap();
url.append_query_params([("foo".into(), "bar".into())]);
url.append_query_params([("foo", "bar")]);
assert_eq!(url.query(), Some("foo=bar"));
assert_eq!(url.fragment(), Some("section"));
assert_eq!(url.as_str(), "http://example.com/path?foo=bar#section");
Expand All @@ -1038,7 +1020,7 @@ mod tests {
#[test]
fn append_query_params_to_url_with_query_and_fragment() {
let mut url = Url::parse("http://example.com/path?existing=value#section").unwrap();
url.append_query_params([("foo".into(), "bar".into())]);
url.append_query_params([("foo", "bar")]);
assert_eq!(url.query(), Some("existing=value&foo=bar"));
assert_eq!(url.fragment(), Some("section"));
assert_eq!(url.as_str(), "http://example.com/path?existing=value&foo=bar#section");
Expand All @@ -1047,18 +1029,14 @@ mod tests {
#[test]
fn append_query_params_multiple_params() {
let mut url = Url::parse("http://example.com").unwrap();
url.append_query_params([
("a".into(), "1".into()),
("b".into(), "2".into()),
("c".into(), "3".into()),
]);
url.append_query_params([("a", "1"), ("b", "2"), ("c", "3")]);
assert_eq!(url.query(), Some("a=1&b=2&c=3"));
}

#[test]
fn append_query_params_empty_iterator() {
let mut url = Url::parse("http://example.com/path").unwrap();
url.append_query_params(std::iter::empty::<(String, String)>());
url.append_query_params(std::iter::empty::<(&str, &str)>());
assert_eq!(url.query(), None);
assert_eq!(url.as_str(), "http://example.com/path");
}
Expand All @@ -1074,7 +1052,7 @@ mod tests {
assert_eq!(url.query(), Some("query=%7B%22id%22%7D"));

// Add a new param
url.append_query_params([("foo".into(), "bar".into())]);
url.append_query_params([("foo", "bar")]);

// The existing encoded query should still be preserved, not double-encoded
// i.e., %7B should NOT become %257B
Expand All @@ -1092,7 +1070,7 @@ mod tests {
assert_eq!(url.query(), Some("filter=%7B%22name%22%3A%22test%22%7D"));

// Add multiple new params in one call
url.append_query_params([("page".into(), "1".into()), ("sort".into(), "name".into())]);
url.append_query_params([("page", "1"), ("sort", "name")]);

// Verify no double encoding occurred
assert_eq!(url.query(), Some("filter=%7B%22name%22%3A%22test%22%7D&page=1&sort=name"));
Expand Down