Deuxfleurs/garage
48
56
Fork 73
Code Issues 100 Pull requests 13 Releases 30 Wiki Activity

web_server.rs: Log X-Forwarded-For IP #504

Merged
lx merged 3 commits from jpds/garage:web_server-log-x-forwarded-for into main 2023-03-06 12:33:07 +00:00
Conversation 3 Commits 3 Files changed 4 +4 -9
1 changed files with 4 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit e4e5196066 - Show all commits

13
src/api/generic_server.rs
View file

@ -19,6 +19,7 @@ use opentelemetry::{
}; };
use garage_util::error::Error as GarageError; use garage_util::error::Error as GarageError;
use garage_util::forwarded_headers;
use garage_util::metrics::{gen_trace_id, RecordDuration}; use garage_util::metrics::{gen_trace_id, RecordDuration};
pub(crate) trait ApiEndpoint: Send + Sync + 'static { pub(crate) trait ApiEndpoint: Send + Sync + 'static {
@ -126,15 +127,9 @@ impl<A: ApiHandler> ApiServer<A> {
) -> Result<Response<Body>, GarageError> { ) -> Result<Response<Body>, GarageError> {
let uri = req.uri().clone(); let uri = req.uri().clone();
let has_forwarded_for_header = req.headers().contains_key("x-forwarded-for"); if let Ok(forwarded_for_ip_addr) =
if has_forwarded_for_header { forwarded_headers::handle_forwarded_for_headers(&req.headers())
lx commented 2023-03-06 10:18:42 +00:00
Outdated
Review
Copy link

You don't need lines 130-131, just use the second if (the if let statement) with the else branch of the first one.

You don't need lines 130-131, just use the second if (the `if let` statement) with the else branch of the first one.
let forwarded_for_ip_addr = &req {
.headers()
.get("x-forwarded-for")
.expect("Could not parse X-Forwarded-For header")
.to_str()
.unwrap_or_default();
info!( info!(
jpds marked this conversation as resolved Outdated
lx commented 2023-03-01 19:03:54 +00:00
Outdated
Review
Copy link

With handle_forwarded_for_headers returning a Result (see below), lines 130-133 should be rewritten as follows:

if let Ok(forwarded_for_ip_addr) = forwarded_headers::handle_forwarded_for_headers(&req.headers()) {

  info!(...)
With `handle_forwarded_for_headers` returning a `Result` (see below), lines 130-133 should be rewritten as follows: ```rust if let Ok(forwarded_for_ip_addr) = forwarded_headers::handle_forwarded_for_headers(&req.headers()) { info!(...) ```
"{} (via {}) {} {}", "{} (via {}) {} {}",
forwarded_for_ip_addr, forwarded_for_ip_addr,