Skip to content

feat: add remote address extractor - #673

Open
Wervice wants to merge 5 commits into
cot-rs:masterfrom
Wervice:ip-extractor
Open

Wervice wants to merge 5 commits into
cot-rs:masterfrom
Wervice:ip-extractor

Conversation

@Wervice

@Wervice Wervice commented Sep 15, 2026

Copy link
Copy Markdown

I added an extractor RemoteAddr in the extractors.rs. This extractor works by getting an extension from the HTTP headers which contains the remote address.
For this extension to be set, axum has to be started with handler.into_make_service_with_connect_info.
The extractor then provides the IP address. Furthermore, when a proxied connection was declared using one of the following headers, the proxied IP will be used:

  • CF-Connecting-IP
  • CloudFront-Viewer-Address
  • Fly-Client-IP
  • Forwarded
  • X-Forwarded-For
  • True-Client-IP
  • X-Envoy-External-Address
  • X-Real-Ip

In order to implement proxied HTTP header detection the client_ip crate was used.
In order to comply with the orphan rules, cot-core was given access to tokio via tokio.workspace = true.

Related issue or discussion

Description

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Refactor / cleanup
  • Performance improvement
  • Other (describe above)

Checklist

  • I've read the contributing guide
  • Tests pass locally (just test-all)
  • Code passes clippy (just clippy)
  • Code is properly formatted (cargo fmt)
  • New tests added (regression test for bugs, coverage for new features)
  • Documentation (both code and site) updated (if applicable)

I added an extractor `RemoteAddr` in the `extractors.rs`.
This extractor works by getting an extension from the HTTP headers which
contains the remote address.
For this extension to be set, axum has to be started with
`handler.into_make_service_with_connect_info`.
The extractor then provides the IP address. Furthermore, when a proxied
connection was declared using one of the following headers, the proxied
IP will be used:
- CF-Connecting-IP
- CloudFront-Viewer-Address
- Fly-Client-IP
- Forwarded
- X-Forwarded-For
- True-Client-IP
- X-Envoy-External-Address
- X-Real-Ip

In order to implement proxied HTTP header detection the `client_ip`
crate was used.
In order to comply with the orphan rules, `cot-core` was given access to
`tokio` via `tokio.workspace = true`.
@github-actions github-actions Bot added C-lib Crate: cot (main library crate) C-core labels Sep 15, 2026
@Wervice Wervice changed the title Added IP address extractor feat: add remote address extractor Sep 15, 2026
@Wervice
Wervice marked this pull request as draft September 16, 2026 12:08
I added support for configuring a proxy via a new trusted_proxy field in
the configuration file.
The field supports CIDR notated subnets and several headers.
I cut the `client-ip` dependency.
Currently supported headers are:
- Forwarded
- X-Forwarded-For
- CF-Connecting-IP
- X-Real-IP
@Wervice
Wervice marked this pull request as ready for review September 20, 2026 16:22

@ElijahAhianyo ElijahAhianyo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey! Thanks for your contribution. Please address the comments and this should be in a good shape


#[cfg(test)]
mod tests {
use serde::Deserialize;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why was this moved?

use crate::request::extractors::{FromRequest, Json, Path, UrlQuery};

use serde::Deserialize;
use std::net::{Ipv4Addr, Ipv6Addr};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't use this import anywhere, no?

Comment thread cot/src/config.rs
Comment on lines +1256 to +1281
pub struct IpWithSubnet {
ip: IpAddr,
mask: Option<u8>,
}

impl Serialize for IpWithSubnet {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
if let Some(mask) = self.mask {
serializer.serialize_str(&format!("{}/{}", self.ip, mask))
} else {
serializer.serialize_str(&self.ip.to_string())
}
}
}

struct IpWithSubnetVistor;

impl Visitor<'_> for IpWithSubnetVistor {
type Value = String;

fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter.write_str("a CIDR notation")
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this; doing this manually is a bit too finicky and error-prone. If the goal is to support Cidr, we should delegate this to a battle-tested dependency like ipnet

Comment thread cot/src/remote_addr.rs
Comment on lines +95 to +105
/// An extractor that extracts the IP address of the remote.
/// This automatically checks for proxy IP headers and contains their IP if one such is specified.
///
/// # Examples
/// ```rust
/// pub async fn example_handler(ip: RemoteAddr) -> cot::Result<Html> {
/// dbg!(ip.ip()); // Prints the IP as a debug statement
/// dbg!(ip.closest_ip()); // Prints the closest IP as a debug statement
/// ///...
/// }
/// ```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The extractor should live in requests/extractor.rs

Comment thread cot/src/config.rs
Comment on lines +1224 to +1249
pub struct ProxyConfig {
trusted_proxies: Vec<IpWithSubnet>,
trusted_headers: Vec<String>,
}

impl ProxyConfig {
#[must_use]
/// Get the proxies that have been configured to be trusted.
pub fn get_trusted_proxies(&self) -> Vec<IpWithSubnet> {
self.trusted_proxies.clone()
}

#[must_use]
/// Get the proxy headers that have been configured to be trusted.
pub fn get_trusted_headers(&self) -> Vec<HeaderName> {
self.trusted_headers
.iter()
.filter_map(|h| HeaderName::from_str(h).ok())
.collect()
}

#[must_use]
/// Create a new [`ProxyConfig`] with trusted proxies and trusted headers.
pub fn new(proxies: Vec<IpWithSubnet>, headers: Vec<String>) -> Self {
Self {
trusted_proxies: proxies,

@ElijahAhianyo ElijahAhianyo Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should aim for some consistency in the API here. The config struct here should be a builder(we do that by using the derive_builder::Builder macro).

We should aim for something like this:

#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize, derive_builder::Builder)]
#[builder(build_fn(name = "build_impl"))]
pub struct ClientIpConfig {
    #[serde(default)]
    #[builder(default)]
    pub trusted_proxies: TrustedProxies,

    #[serde(default = "default_client_ip_headers")]
    #[builder(default = "default_client_ip_headers()")]
    pub headers: Vec<ClientIpHeader>,
}

fn default_client_ip_headers() -> Vec<ClientIpHeader> {
    vec![ClientIpHeader::XForwardedFor]
}

impl Default for ClientIpConfig {
    fn default() -> Self {
        Self {
            trusted_proxies: TrustedProxies::default(),
            headers: default_client_ip_headers(),
        }
    }
}

While we're here, I think we need some kind of type-safety with deserializing proxy IPs and headers. For headers, we want to provide an enum(ClientIpHeader in the case of the example above), with supported headers (x-forwarded-for, true-client-ip, etc) as variant but also an escape hatch for custom headers. This should look somewhat like this:

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum ClientIpHeader {
    /// `X-Forwarded-For`, the de facto standard, a comma-separated list of
    /// hops, closest-to-origin first.
    XForwardedFor,
    /// `X-Real-IP`,  commonly set by nginx, a single address.
    XRealIp,
    /// `True-Client-IP`, used by Akamai and Cloudflare Enterprise.
    TrueClientIp,
    /// `CF-Connecting-IP` set by Cloudflare.
    CfConnectingIp,
    /// Any other header name.
    Custom(HeaderName),
}

I would also expect TrustedProxies (in the example above) to look something like this:

#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct TrustedProxies(Vec<TrustedProxyEntry>);

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum TrustedProxyEntry {
    /// Trusts every proxy address. (we should have a security note that states the risk of using this)
    Any,
    /// Trusts every address in this network (a single address is a network
    /// with a full-length prefix). 
    Network(ipnet::IpNet), 
}

In terms of design, i would expect to abstract the header and trusted proxy structs into the cot-core crate (cot-core/src/request/client_ip.rs or equivalent) since they dont need axum or the project config

Comment thread cot/src/remote_addr.rs
@@ -0,0 +1,586 @@
use axum::{extract::connect_info::Connected, serve::IncomingStream};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should improve the abstraction design here. Core components, code, or data structures that do not integrate with third-party dependencies in cot (like axum) or ProjectConfig should move into cot-core. In other words, the contents of this file should move into the appropriate modules

Comment thread cot/src/remote_addr.rs
Comment on lines +131 to +140
impl<'a> Connected<IncomingStream<'a, tokio::net::TcpListener>> for RemoteAddr {
fn connect_info(stream: IncomingStream<'a, tokio::net::TcpListener>) -> Self {
let closest_ip = stream.remote_addr().ip();
RemoteAddr {
direct: closest_ip,
proxied: None,
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why do we need this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-core C-lib Crate: cot (main library crate)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants