Skip to content

Commit d27b2bb

Browse files
fixed small issues with tests and added better client orchestration and seperation
1 parent f3ffb74 commit d27b2bb

8 files changed

Lines changed: 706 additions & 78 deletions

File tree

src/client/http_client.rs

Lines changed: 15 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
use crate::body::Body;
22
use crate::client::policy::{PolicyDecision, RequestPolicy};
3+
use crate::client::request_executor::RequestExecutor;
34
use crate::config::Config;
45
use crate::dns::DnsResolver;
56
use crate::error::Error;
7+
use crate::parser::Response;
68
use crate::parser::uri::Uri;
7-
use crate::parser::{RequestBuilder as ParserRequestBuilder, Response};
89
use crate::request_builder::ClientRequestBuilder;
910
use crate::socket::BlockingSocket;
10-
use crate::transport::{ConnectionPool, Connector, PoolKey, ResponseBodyExpectation};
11+
use crate::transport::ConnectionPool;
1112
use alloc::string::String;
1213
use alloc::vec::Vec;
1314

@@ -247,7 +248,12 @@ where
247248
self.request(method, &url, &headers, body.map(Body::into_bytes))
248249
}
249250

250-
/// Internal request execution with thin orchestration
251+
/// Internal request execution with clean orchestration
252+
///
253+
/// This method orchestrates the high-level request flow:
254+
/// - Redirect loop handling
255+
/// - Policy validation and decisions
256+
/// - Delegation to `RequestExecutor` for actual HTTP execution
251257
///
252258
/// # Errors
253259
/// Returns an error if URL parsing, DNS resolution, socket connection, or HTTP communication fails.
@@ -265,85 +271,16 @@ where
265271
let mut policy = RequestPolicy::new(&self.config);
266272

267273
loop {
274+
// Parse and validate URL
268275
let uri = Uri::parse(&current_url).map_err(Error::Parse)?;
269276
policy.validate_protocol(&uri)?;
270277

271-
// RFC 9112 Section 3.2: Host header handling
272-
let authority = uri.authority();
273-
let host_str = if let Some(auth) = authority {
274-
match auth.host() {
275-
crate::parser::uri::Host::RegName(name) => name,
276-
crate::parser::uri::Host::IpAddr(_) => {
277-
return Err(Error::IpAddressNotSupported);
278-
}
279-
}
280-
} else {
281-
// RFC 9112 Section 3.2: If authority missing, send empty Host
282-
""
283-
};
284-
285-
let port = authority
286-
.and_then(super::super::parser::uri::Authority::port)
287-
.unwrap_or_else(|| if uri.scheme() == "https" { 443 } else { 80 });
288-
289-
let pool_key = PoolKey::new(String::from(host_str), port);
290-
291-
// Get socket from pool or create new
292-
let mut socket = if self.config.connection_pooling {
293-
match self.pool.get(&pool_key) {
294-
Some(s) => s,
295-
None => S::new().map_err(Error::Socket)?,
296-
}
297-
} else {
298-
S::new().map_err(Error::Socket)?
299-
};
300-
301-
let connector = Connector::new(&mut socket, &self.dns);
302-
let mut conn = connector.connect(&uri, &self.config)?;
303-
304-
let mut builder =
305-
ParserRequestBuilder::new(current_method.as_str(), &uri.path_and_query())
306-
.header("Host", host_str);
307-
308-
// RFC 9112 Section 9.3: Send Connection: close if pooling is disabled
309-
if !self.config.connection_pooling {
310-
builder = builder.header("Connection", "close");
311-
}
312-
313-
if let Some(ref user_agent) = self.config.user_agent {
314-
builder = builder.header("User-Agent", user_agent.as_str());
315-
}
316-
317-
if let Some(ref accept) = self.config.accept {
318-
builder = builder.header("Accept", accept.as_str());
319-
}
320-
321-
for (name, value) in custom_headers {
322-
builder = builder.header(name.as_str(), value.as_str());
323-
}
324-
325-
if let Some(ref body_data) = current_body {
326-
builder = builder.body(body_data.clone());
327-
}
328-
329-
let request_bytes = builder.build()?;
330-
conn.send_request(&request_bytes)?;
331-
332-
let expectation = if current_method == crate::method::Method::Head {
333-
ResponseBodyExpectation::NoBody
334-
} else {
335-
ResponseBodyExpectation::Normal
336-
};
337-
let raw = conn.read_raw_response(expectation)?;
338-
339-
// Check if connection can be reused
340-
let is_reusable = conn.is_reusable();
341-
342-
// Return socket to pool if pooling is enabled and connection is reusable
343-
if self.config.connection_pooling && is_reusable {
344-
self.pool.return_connection(pool_key, socket);
345-
}
278+
// Execute single HTTP request
279+
let mut executor = RequestExecutor::new(&mut self.pool, &self.dns, &self.config);
280+
let body_slice = current_body.as_deref();
281+
let raw = executor.execute(&uri, current_method, custom_headers, body_slice)?;
346282

283+
// Process response and make policy decision
347284
match policy.process_raw_response(
348285
raw,
349286
&uri,

src/client/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
mod http_client;
22
mod policy;
3+
mod request_executor;
34

45
pub use http_client::HttpClient;
56

src/client/request_executor.rs

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
/// Single HTTP request execution logic
2+
///
3+
/// This module handles the low-level details of executing a single HTTP request:
4+
/// - Socket management (pooling/creation)
5+
/// - Connection establishment
6+
/// - Request serialization
7+
/// - Response reading
8+
/// - Connection reuse logic
9+
use crate::config::Config;
10+
use crate::dns::DnsResolver;
11+
use crate::error::Error;
12+
use crate::headers::Headers;
13+
use crate::method::Method;
14+
use crate::parser::RequestBuilder as ParserRequestBuilder;
15+
use crate::parser::uri::Uri;
16+
use crate::socket::BlockingSocket;
17+
use crate::transport::{
18+
ConnectionPool, Connector, PoolKey, RawResponse, ResponseBodyExpectation,
19+
};
20+
use alloc::string::String;
21+
use alloc::vec::Vec;
22+
23+
/// Executes a single HTTP request without redirect handling
24+
pub struct RequestExecutor<'a, S, D> {
25+
pool: &'a mut ConnectionPool<S>,
26+
dns: &'a D,
27+
config: &'a Config,
28+
}
29+
30+
impl<'a, S, D> RequestExecutor<'a, S, D>
31+
where
32+
S: BlockingSocket,
33+
D: DnsResolver,
34+
{
35+
pub const fn new(
36+
pool: &'a mut ConnectionPool<S>,
37+
dns: &'a D,
38+
config: &'a Config,
39+
) -> Self {
40+
Self { pool, dns, config }
41+
}
42+
43+
/// Execute a single HTTP request and return raw response
44+
pub fn execute(
45+
&mut self,
46+
uri: &Uri,
47+
method: Method,
48+
custom_headers: &Headers,
49+
body: Option<&[u8]>,
50+
) -> Result<RawResponse, Error> {
51+
// Extract host information from URI (copy to avoid lifetime issues)
52+
let host_str = Self::extract_host_from_uri(uri)?;
53+
let port = Self::extract_port_from_uri(uri);
54+
let pool_key = PoolKey::new(host_str.clone(), port);
55+
56+
// Get or create socket
57+
let mut socket = self.get_or_create_socket(&pool_key)?;
58+
59+
// Establish connection
60+
let connector = Connector::new(&mut socket, self.dns);
61+
let mut conn = connector.connect(uri, self.config)?;
62+
63+
// Build and send request
64+
let request_bytes =
65+
self.build_request(uri, method, &host_str, custom_headers, body)?;
66+
conn.send_request(&request_bytes)?;
67+
68+
// Read response
69+
let expectation = if method == Method::Head {
70+
ResponseBodyExpectation::NoBody
71+
} else {
72+
ResponseBodyExpectation::Normal
73+
};
74+
let raw = conn.read_raw_response(expectation)?;
75+
76+
// Handle connection pooling
77+
self.handle_connection_reuse(conn.is_reusable(), pool_key, socket);
78+
79+
Ok(raw)
80+
}
81+
82+
/// Extract hostname from URI
83+
fn extract_host_from_uri(uri: &Uri) -> Result<String, Error> {
84+
let authority = uri.authority();
85+
authority.map_or_else(
86+
|| Ok(String::new()),
87+
|auth| match auth.host() {
88+
crate::parser::uri::Host::RegName(name) => Ok(String::from(*name)),
89+
crate::parser::uri::Host::IpAddr(_) => Err(Error::IpAddressNotSupported),
90+
},
91+
)
92+
}
93+
94+
/// Extract port from URI with defaults
95+
fn extract_port_from_uri(uri: &Uri) -> u16 {
96+
uri
97+
.authority()
98+
.and_then(super::super::parser::uri::Authority::port)
99+
.unwrap_or_else(|| if uri.scheme() == "https" { 443 } else { 80 })
100+
}
101+
102+
/// Get socket from pool or create new one
103+
fn get_or_create_socket(&mut self, pool_key: &PoolKey) -> Result<S, Error> {
104+
if self.config.connection_pooling {
105+
self
106+
.pool
107+
.get(pool_key)
108+
.map_or_else(|| S::new().map_err(Error::Socket), |s| Ok(s))
109+
} else {
110+
S::new().map_err(Error::Socket)
111+
}
112+
}
113+
114+
/// Build HTTP request bytes
115+
fn build_request(
116+
&self,
117+
uri: &Uri,
118+
method: Method,
119+
host_str: &str,
120+
custom_headers: &Headers,
121+
body: Option<&[u8]>,
122+
) -> Result<Vec<u8>, Error> {
123+
let mut builder = ParserRequestBuilder::new(method.as_str(), &uri.path_and_query())
124+
.header("Host", host_str);
125+
126+
// RFC 9112 Section 9.3: Send Connection: close if pooling is disabled
127+
if !self.config.connection_pooling {
128+
builder = builder.header("Connection", "close");
129+
}
130+
131+
// Add default headers from config
132+
if let Some(ref user_agent) = self.config.user_agent {
133+
builder = builder.header("User-Agent", user_agent.as_str());
134+
}
135+
136+
if let Some(ref accept) = self.config.accept {
137+
builder = builder.header("Accept", accept.as_str());
138+
}
139+
140+
// Add custom headers
141+
for (name, value) in custom_headers {
142+
builder = builder.header(name.as_str(), value.as_str());
143+
}
144+
145+
// Add body if present
146+
if let Some(body_data) = body {
147+
builder = builder.body(body_data.to_vec());
148+
}
149+
150+
builder.build().map_err(Error::Parse)
151+
}
152+
153+
/// Handle connection reuse based on pooling config
154+
fn handle_connection_reuse(&mut self, is_reusable: bool, pool_key: PoolKey, socket: S) {
155+
if self.config.connection_pooling && is_reusable {
156+
self.pool.return_connection(pool_key, socket);
157+
}
158+
}
159+
}

src/config.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ pub struct Config {
7171
pub max_idle_per_host: usize,
7272
/// Timeout for idle connections in the pool (in seconds)
7373
pub idle_timeout: Option<Duration>,
74+
/// Maximum allowed URI length in bytes (RFC 9112 Section 3)
75+
/// Server should respond with 414 (URI Too Long) if exceeded
76+
/// None means no limit
77+
pub max_uri_length: Option<usize>,
7478
}
7579

7680
impl Default for Config {
@@ -90,6 +94,7 @@ impl Default for Config {
9094
connection_pooling: true,
9195
max_idle_per_host: 5,
9296
idle_timeout: Some(Duration::from_secs(90)),
97+
max_uri_length: Some(8192), // RFC 9112 Section 3: reasonable default
9398
}
9499
}
95100
}

src/error/parse.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,16 @@ pub enum ParseError {
5555
ChunkedInTeHeader,
5656
/// TE header present but Connection header missing "TE" (RFC 9112 Section 7.4)
5757
TeHeaderMissingConnection,
58+
/// Multiple Host headers present (RFC 9112 Section 3.2)
59+
MultipleHostHeaders,
60+
/// Invalid Host header value format (RFC 9112 Section 3.2)
61+
InvalidHostHeaderValue,
62+
/// Request-target URI exceeds maximum allowed length (RFC 9112 Section 3)
63+
UriTooLong,
64+
/// Transfer-Encoding used with HTTP version < 1.1 (RFC 9112 Section 6.1)
65+
TransferEncodingRequiresHttp11,
66+
/// Chunked appears multiple times in Transfer-Encoding (RFC 9112 Section 6.1)
67+
ChunkedAppliedMultipleTimes,
5868
}
5969

6070
impl ParseError {
@@ -122,6 +132,15 @@ impl core::fmt::Display for ParseError {
122132
Self::TeHeaderMissingConnection => {
123133
write!(f, "TE header requires 'TE' in Connection header")
124134
}
135+
Self::MultipleHostHeaders => write!(f, "multiple Host headers present"),
136+
Self::InvalidHostHeaderValue => write!(f, "invalid Host header value format"),
137+
Self::UriTooLong => write!(f, "request-target URI exceeds maximum allowed length"),
138+
Self::TransferEncodingRequiresHttp11 => {
139+
write!(f, "Transfer-Encoding requires HTTP/1.1 or higher")
140+
}
141+
Self::ChunkedAppliedMultipleTimes => {
142+
write!(f, "chunked transfer coding applied multiple times")
143+
}
125144
}
126145
}
127146
}

0 commit comments

Comments
 (0)