Skip to content

Commit 2eaa17f

Browse files
committed
Merge branch 'main' into cursor/sanitize-http-header-values-to-prevent-injection-2d93
Resolved conflicts in src/render.rs by: - Keeping header value sanitization with Cow optimization - Using the new into_response_builder() method from main - Maintaining has_status flag updates from main
2 parents 8afa988 + c803214 commit 2eaa17f

File tree

5 files changed

+67
-19
lines changed

5 files changed

+67
-19
lines changed

src/render.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,7 @@ impl HeaderContext {
247247
let sanitized_link = sanitize_header_value(link);
248248
self.response
249249
.insert_header((header::LOCATION, sanitized_link.as_ref()));
250-
let response = self.response.body(());
251-
Ok(response)
250+
Ok(self.into_response_builder().body(()))
252251
}
253252

254253
/// Answers to the HTTP request with a single json object
@@ -261,7 +260,9 @@ impl HeaderContext {
261260
} else {
262261
serde_json::to_vec(contents)?
263262
};
264-
Ok(PageContext::Close(self.response.body(json_response)))
263+
Ok(PageContext::Close(
264+
self.into_response_builder().body(json_response),
265+
))
265266
} else {
266267
let body_type = get_object_str(data, "type");
267268
let json_renderer = match body_type {
@@ -319,22 +320,21 @@ impl HeaderContext {
319320
}
320321
log::debug!("Authentication failed");
321322
// The authentication failed
322-
let http_response: HttpResponse = if let Some(link) = get_object_str(&data, "link") {
323+
if let Some(link) = get_object_str(&data, "link") {
323324
let sanitized_link = sanitize_header_value(link);
324325
self.response
325326
.status(StatusCode::FOUND)
326-
.insert_header((header::LOCATION, sanitized_link.as_ref()))
327-
.body(
328-
"Sorry, but you are not authorized to access this page. \
329-
Redirecting to the login page...",
330-
)
327+
.insert_header((header::LOCATION, sanitized_link.as_ref()));
328+
self.has_status = true;
329+
Ok(PageContext::Close(self.into_response_builder().body(
330+
"Sorry, but you are not authorized to access this page. \
331+
Redirecting to the login page...",
332+
)))
331333
} else {
332334
anyhow::bail!(ErrorWithStatus {
333335
status: StatusCode::UNAUTHORIZED
334336
})
335-
};
336-
self.has_status = true;
337-
Ok(PageContext::Close(http_response))
337+
}
338338
}
339339

340340
fn download(mut self, options: &JsonValue) -> anyhow::Result<PageContext> {
@@ -366,7 +366,7 @@ impl HeaderContext {
366366
.insert_header((header::CONTENT_TYPE, content_type));
367367
}
368368
Ok(PageContext::Close(
369-
self.response.body(body_bytes.into_owned()),
369+
self.into_response_builder().body(body_bytes.into_owned()),
370370
))
371371
}
372372

@@ -381,6 +381,11 @@ impl HeaderContext {
381381
}
382382
}
383383

384+
fn into_response_builder(mut self) -> HttpResponseBuilder {
385+
self.add_server_timing_header();
386+
self.response
387+
}
388+
384389
async fn start_body(mut self, data: JsonValue) -> anyhow::Result<PageContext> {
385390
self.add_server_timing_header();
386391
let html_renderer =
@@ -395,9 +400,8 @@ impl HeaderContext {
395400
})
396401
}
397402

398-
pub fn close(mut self) -> HttpResponse {
399-
self.add_server_timing_header();
400-
self.response.finish()
403+
pub fn close(self) -> HttpResponse {
404+
self.into_response_builder().finish()
401405
}
402406
}
403407

src/webserver/database/execute_queries.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ pub fn stream_query_results_with_conn<'a>(
5858
},
5959
ParsedStatement::StmtWithParams(stmt) => {
6060
let query = bind_parameters(stmt, request, db_connection).await?;
61+
request.server_timing.record("bind_params");
6162
let connection = take_connection(&request.app_state.db, db_connection, request).await?;
6263
log::trace!("Executing query {:?}", query.sql);
6364
let mut stream = connection.fetch_many(query);

src/webserver/server_timing.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,10 @@ impl ServerTiming {
6161
if i > 0 {
6262
s.push_str(", ");
6363
}
64-
let millis = time.saturating_duration_since(last).as_millis();
65-
write!(&mut s, "{name};dur={millis}").ok()?;
64+
let micros = time.saturating_duration_since(last).as_micros();
65+
let millis = micros / 1000;
66+
let micros = micros % 1000;
67+
write!(&mut s, "{name};dur={millis}.{micros:03}").ok()?;
6668
last = *time;
6769
}
6870
Some(s)

tests/server_timing/mod.rs

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ async fn test_server_timing_enabled_in_development() -> actix_web::Result<()> {
5454
header_value.contains("parse_req;dur="),
5555
"Should contain parse_req timing: {header_value}"
5656
);
57+
assert!(
58+
header_value.contains("bind_params;dur="),
59+
"Should contain bind_params timing: {header_value}"
60+
);
5761
assert!(
5862
header_value.contains("db_conn;dur="),
5963
"Should contain db_conn timing: {header_value}"
@@ -78,7 +82,7 @@ async fn test_server_timing_format() -> actix_web::Result<()> {
7882
let header_value = server_timing_header.to_str().unwrap();
7983

8084
let parts: Vec<&str> = header_value.split(", ").collect();
81-
assert!(parts.len() >= 4, "Should have at least 4 timing events");
85+
assert!(parts.len() >= 5, "Should have at least 5 timing events");
8286

8387
for part in parts {
8488
assert!(
@@ -98,3 +102,38 @@ async fn test_server_timing_format() -> actix_web::Result<()> {
98102

99103
Ok(())
100104
}
105+
106+
#[actix_web::test]
107+
async fn test_server_timing_in_redirect() -> actix_web::Result<()> {
108+
let mut config = test_config();
109+
config.environment = sqlpage::app_config::DevOrProd::Development;
110+
let app_data = make_app_data_from_config(config).await;
111+
112+
let req =
113+
crate::common::get_request_to_with_data("/tests/server_timing/redirect_test.sql", app_data)
114+
.await?
115+
.to_srv_request();
116+
let resp = main_handler(req).await?;
117+
118+
assert_eq!(
119+
resp.status(),
120+
StatusCode::FOUND,
121+
"Response should be a redirect"
122+
);
123+
let server_timing_header = resp
124+
.headers()
125+
.get("Server-Timing")
126+
.expect("Server-Timing header should be present in redirect responses");
127+
let header_value = server_timing_header.to_str().unwrap();
128+
129+
assert!(
130+
!header_value.is_empty(),
131+
"Server-Timing header should not be empty: {header_value}"
132+
);
133+
assert!(
134+
header_value.contains(";dur="),
135+
"Server-Timing header should contain timing events: {header_value}"
136+
);
137+
138+
Ok(())
139+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
select 'redirect' as component, '/destination.sql' as link;
2+

0 commit comments

Comments
 (0)