Merge branch 'master' into various-refactorings

This commit is contained in:
Igor Aleksanov 2021-06-26 08:21:48 +03:00
commit cd5322ddea
10 changed files with 345 additions and 124 deletions

View File

@ -48,7 +48,6 @@ jobs:
uses: actions-rs/cargo@v1
with:
command: generate-lockfile
- name: Cache Dependencies
uses: Swatinem/rust-cache@v1.2.0
@ -96,7 +95,6 @@ jobs:
run: |
cargo install cargo-tarpaulin --vers "^0.13"
cargo tarpaulin --out Xml --verbose
- name: Upload to Codecov
if: >
matrix.target.os == 'ubuntu-latest'

View File

@ -10,6 +10,7 @@
### Changed
* Change compression algorithm features flags. [#2250]
* Deprecate `App::data` and `App::data_factory`. [#2271]
* Smarter extraction of `ConnectionInfo` parts. [#2282]
### Fixed
@ -20,6 +21,7 @@
[#2271]: https://github.com/actix/actix-web/pull/2271
[#2262]: https://github.com/actix/actix-web/pull/2262
[#2263]: https://github.com/actix/actix-web/pull/2263
[#2282]: https://github.com/actix/actix-web/pull/2282
[#2288]: https://github.com/actix/actix-web/pull/2288

View File

@ -107,7 +107,7 @@ actix-test = { version = "0.1.0-beta.3", features = ["openssl", "rustls"] }
awc = { version = "3.0.0-beta.6", features = ["openssl"] }
brotli2 = "0.3.2"
criterion = "0.3"
criterion = { version = "0.3", features = ["html_reports"] }
env_logger = "0.8"
flate2 = "1.0.13"
zstd = "0.7"

View File

@ -1,6 +1,9 @@
# Changes
## Unreleased - 2021-xx-xx
* Added `Files::path_filter()`. [#2274]
[#2274]: https://github.com/actix/actix-web/pull/2274
## 0.6.0-beta.5 - 2021-06-17

View File

@ -1,9 +1,17 @@
use std::{cell::RefCell, fmt, io, path::PathBuf, rc::Rc};
use std::{
cell::RefCell,
fmt, io,
path::{Path, PathBuf},
rc::Rc,
};
use actix_service::{boxed, IntoServiceFactory, ServiceFactory, ServiceFactoryExt};
use actix_utils::future::ok;
use actix_web::{
dev::{AppService, HttpServiceFactory, ResourceDef, ServiceRequest, ServiceResponse},
dev::{
AppService, HttpServiceFactory, RequestHead, ResourceDef, ServiceRequest,
ServiceResponse,
},
error::Error,
guard::Guard,
http::header::DispositionType,
@ -13,7 +21,7 @@ use futures_core::future::LocalBoxFuture;
use crate::{
directory_listing, named, Directory, DirectoryRenderer, FilesService, HttpNewService,
MimeOverride,
MimeOverride, PathFilter,
};
/// Static files handling service.
@ -36,6 +44,7 @@ pub struct Files {
default: Rc<RefCell<Option<Rc<HttpNewService>>>>,
renderer: Rc<DirectoryRenderer>,
mime_override: Option<Rc<MimeOverride>>,
path_filter: Option<Rc<PathFilter>>,
file_flags: named::Flags,
use_guards: Option<Rc<dyn Guard>>,
guards: Vec<Rc<dyn Guard>>,
@ -60,6 +69,7 @@ impl Clone for Files {
file_flags: self.file_flags,
path: self.path.clone(),
mime_override: self.mime_override.clone(),
path_filter: self.path_filter.clone(),
use_guards: self.use_guards.clone(),
guards: self.guards.clone(),
hidden_files: self.hidden_files,
@ -104,6 +114,7 @@ impl Files {
default: Rc::new(RefCell::new(None)),
renderer: Rc::new(directory_listing),
mime_override: None,
path_filter: None,
file_flags: named::Flags::default(),
use_guards: None,
guards: Vec::new(),
@ -149,6 +160,38 @@ impl Files {
self
}
/// Sets path filtering closure.
///
/// The path provided to the closure is relative to `serve_from` path.
/// You can safely join this path with the `serve_from` path to get the real path.
/// However, the real path may not exist since the filter is called before checking path existence.
///
/// When a path doesn't pass the filter, [`Files::default_handler`] is called if set, otherwise,
/// `404 Not Found` is returned.
///
/// # Examples
/// ```
/// use std::path::Path;
/// use actix_files::Files;
///
/// // prevent searching subdirectories and following symlinks
/// let files_service = Files::new("/", "./static").path_filter(|path, _| {
/// path.components().count() == 1
/// && Path::new("./static")
/// .join(path)
/// .symlink_metadata()
/// .map(|m| !m.file_type().is_symlink())
/// .unwrap_or(false)
/// });
/// ```
pub fn path_filter<F>(mut self, f: F) -> Self
where
F: Fn(&Path, &RequestHead) -> bool + 'static,
{
self.path_filter = Some(Rc::new(f));
self
}
/// Set index file
///
/// Shows specific index file for directories instead of
@ -318,6 +361,7 @@ impl ServiceFactory<ServiceRequest> for Files {
default: None,
renderer: self.renderer.clone(),
mime_override: self.mime_override.clone(),
path_filter: self.path_filter.clone(),
file_flags: self.file_flags,
guards: self.use_guards.clone(),
hidden_files: self.hidden_files,

View File

@ -16,11 +16,12 @@
use actix_service::boxed::{BoxService, BoxServiceFactory};
use actix_web::{
dev::{ServiceRequest, ServiceResponse},
dev::{RequestHead, ServiceRequest, ServiceResponse},
error::Error,
http::header::DispositionType,
};
use mime_guess::from_ext;
use std::path::Path;
mod chunked;
mod directory;
@ -56,6 +57,8 @@ pub fn file_extension_to_mime(ext: &str) -> mime::Mime {
type MimeOverride = dyn Fn(&mime::Name<'_>) -> DispositionType;
type PathFilter = dyn Fn(&Path, &RequestHead) -> bool;
#[cfg(test)]
mod tests {
use std::{
@ -901,4 +904,40 @@ mod tests {
let bytes = test::read_body(resp).await;
assert!(format!("{:?}", bytes).contains("/tests/test.png"));
}
#[actix_rt::test]
async fn test_path_filter() {
// prevent searching subdirectories
let st = Files::new("/", ".")
.path_filter(|path, _| path.components().count() == 1)
.new_service(())
.await
.unwrap();
let req = TestRequest::with_uri("/Cargo.toml").to_srv_request();
let resp = test::call_service(&st, req).await;
assert_eq!(resp.status(), StatusCode::OK);
let req = TestRequest::with_uri("/src/lib.rs").to_srv_request();
let resp = test::call_service(&st, req).await;
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
}
#[actix_rt::test]
async fn test_default_handler_filter() {
let st = Files::new("/", ".")
.default_handler(|req: ServiceRequest| {
ok(req.into_response(HttpResponse::Ok().body("default content")))
})
.path_filter(|path, _| path.extension() == Some("png".as_ref()))
.new_service(())
.await
.unwrap();
let req = TestRequest::with_uri("/Cargo.toml").to_srv_request();
let resp = test::call_service(&st, req).await;
assert_eq!(resp.status(), StatusCode::OK);
let bytes = test::read_body(resp).await;
assert_eq!(bytes, web::Bytes::from_static(b"default content"));
}
}

View File

@ -13,7 +13,7 @@ use futures_core::future::LocalBoxFuture;
use crate::{
named, Directory, DirectoryRenderer, FilesError, HttpService, MimeOverride, NamedFile,
PathBufWrap,
PathBufWrap, PathFilter,
};
/// Assembled file serving service.
@ -25,6 +25,7 @@ pub struct FilesService {
pub(crate) default: Option<HttpService>,
pub(crate) renderer: Rc<DirectoryRenderer>,
pub(crate) mime_override: Option<Rc<MimeOverride>>,
pub(crate) path_filter: Option<Rc<PathFilter>>,
pub(crate) file_flags: named::Flags,
pub(crate) guards: Option<Rc<dyn Guard>>,
pub(crate) hidden_files: bool,
@ -82,6 +83,18 @@ impl Service<ServiceRequest> for FilesService {
Err(e) => return Box::pin(ok(req.error_response(e))),
};
if let Some(filter) = &self.path_filter {
if !filter(real_path.as_ref(), req.head()) {
if let Some(ref default) = self.default {
return Box::pin(default.call(req));
} else {
return Box::pin(ok(
req.into_response(actix_web::HttpResponse::NotFound().finish())
));
}
}
}
// full file path
let path = self.directory.join(&real_path);
if let Err(err) = path.canonicalize() {

View File

@ -144,6 +144,11 @@ impl AppConfig {
pub fn local_addr(&self) -> SocketAddr {
self.addr
}
#[cfg(test)]
pub(crate) fn set_host(&mut self, host: &str) {
self.host = host.to_owned();
}
}
impl Default for AppConfig {

View File

@ -2,6 +2,7 @@ use std::{cell::Ref, convert::Infallible, net::SocketAddr};
use actix_utils::future::{err, ok, Ready};
use derive_more::{Display, Error};
use once_cell::sync::Lazy;
use crate::{
dev::{AppConfig, Payload, RequestHead},
@ -12,9 +13,24 @@ use crate::{
FromRequest, HttpRequest, ResponseError,
};
const X_FORWARDED_FOR: &[u8] = b"x-forwarded-for";
const X_FORWARDED_HOST: &[u8] = b"x-forwarded-host";
const X_FORWARDED_PROTO: &[u8] = b"x-forwarded-proto";
static X_FORWARDED_FOR: Lazy<HeaderName> =
Lazy::new(|| HeaderName::from_static("x-forwarded-for"));
static X_FORWARDED_HOST: Lazy<HeaderName> =
Lazy::new(|| HeaderName::from_static("x-forwarded-host"));
static X_FORWARDED_PROTO: Lazy<HeaderName> =
Lazy::new(|| HeaderName::from_static("x-forwarded-proto"));
/// Trim whitespace then any quote marks.
fn unquote(val: &str) -> &str {
val.trim().trim_start_matches('"').trim_end_matches('"')
}
/// Extracts and trims first value for given header name.
fn first_header_value<'a>(req: &'a RequestHead, name: &'_ HeaderName) -> Option<&'a str> {
let hdr = req.headers.get(name)?.to_str().ok()?;
let val = hdr.split(',').next()?.trim();
Some(val)
}
/// HTTP connection information.
///
@ -34,6 +50,19 @@ const X_FORWARDED_PROTO: &[u8] = b"x-forwarded-proto";
/// }
/// # let _svc = actix_web::web::to(handler);
/// ```
///
/// # Implementation Notes
/// Parses `Forwarded` header information according to [RFC 7239][rfc7239] but does not try to
/// interpret the values for each property. As such, the getter methods on `ConnectionInfo` return
/// strings instead of IP addresses or other types to acknowledge that they may be
/// [obfuscated][rfc7239-63] or [unknown][rfc7239-62].
///
/// If the older, related headers are also present (eg. `X-Forwarded-For`), then `Forwarded`
/// is preferred.
///
/// [rfc7239]: https://datatracker.ietf.org/doc/html/rfc7239
/// [rfc7239-62]: https://datatracker.ietf.org/doc/html/rfc7239#section-6.2
/// [rfc7239-63]: https://datatracker.ietf.org/doc/html/rfc7239#section-6.3
#[derive(Debug, Clone, Default)]
pub struct ConnectionInfo {
scheme: String,
@ -51,105 +80,75 @@ impl ConnectionInfo {
Ref::map(req.extensions(), |e| e.get().unwrap())
}
#[allow(clippy::cognitive_complexity, clippy::borrow_interior_mutable_const)]
fn new(req: &RequestHead, cfg: &AppConfig) -> ConnectionInfo {
let mut host = None;
let mut scheme = None;
let mut realip_remote_addr = None;
// load forwarded header
for hdr in req.headers.get_all(&header::FORWARDED) {
if let Ok(val) = hdr.to_str() {
for pair in val.split(';') {
for el in pair.split(',') {
let mut items = el.trim().splitn(2, '=');
if let Some(name) = items.next() {
if let Some(val) = items.next() {
match &name.to_lowercase() as &str {
"for" => {
if realip_remote_addr.is_none() {
realip_remote_addr = Some(val.trim());
}
}
"proto" => {
if scheme.is_none() {
scheme = Some(val.trim());
}
}
"host" => {
if host.is_none() {
host = Some(val.trim());
}
}
_ => {}
}
}
}
}
}
}
}
// scheme
if scheme.is_none() {
if let Some(h) = req
for (name, val) in req
.headers
.get(&HeaderName::from_lowercase(X_FORWARDED_PROTO).unwrap())
.get_all(&header::FORWARDED)
.into_iter()
.filter_map(|hdr| hdr.to_str().ok())
// "for=1.2.3.4, for=5.6.7.8; scheme=https"
.flat_map(|val| val.split(';'))
// ["for=1.2.3.4, for=5.6.7.8", " scheme=https"]
.flat_map(|vals| vals.split(','))
// ["for=1.2.3.4", " for=5.6.7.8", " scheme=https"]
.flat_map(|pair| {
let mut items = pair.trim().splitn(2, '=');
Some((items.next()?, items.next()?))
})
{
if let Ok(h) = h.to_str() {
scheme = h.split(',').next().map(str::trim);
}
}
if scheme.is_none() {
scheme = req.uri.scheme().map(Scheme::as_str);
if scheme.is_none() && cfg.secure() {
scheme = Some("https")
}
// [(name , val ), ... ]
// [("for", "1.2.3.4"), ("for", "5.6.7.8"), ("scheme", "https")]
// taking the first value for each property is correct because spec states that first
// "for" value is client and rest are proxies; multiple values other properties have
// no defined semantics
//
// > In a chain of proxy servers where this is fully utilized, the first
// > "for" parameter will disclose the client where the request was first
// > made, followed by any subsequent proxy identifiers.
// --- https://datatracker.ietf.org/doc/html/rfc7239#section-5.2
match name.trim().to_lowercase().as_str() {
"for" => realip_remote_addr.get_or_insert_with(|| unquote(val)),
"proto" => scheme.get_or_insert_with(|| unquote(val)),
"host" => host.get_or_insert_with(|| unquote(val)),
"by" => {
// TODO: implement https://datatracker.ietf.org/doc/html/rfc7239#section-5.1
continue;
}
_ => continue,
};
}
// host
if host.is_none() {
if let Some(h) = req
.headers
.get(&HeaderName::from_lowercase(X_FORWARDED_HOST).unwrap())
{
if let Ok(h) = h.to_str() {
host = h.split(',').next().map(str::trim);
}
}
if host.is_none() {
if let Some(h) = req.headers.get(&header::HOST) {
host = h.to_str().ok();
}
if host.is_none() {
host = req.uri.authority().map(Authority::as_str);
if host.is_none() {
host = Some(cfg.host());
}
}
}
}
let scheme = scheme
.or_else(|| first_header_value(req, &*X_FORWARDED_PROTO))
.or_else(|| req.uri.scheme().map(Scheme::as_str))
.or_else(|| Some("https").filter(|_| cfg.secure()))
.unwrap_or("http")
.to_owned();
// get remote_addraddr from socketaddr
let remote_addr = req.peer_addr.map(|addr| format!("{}", addr));
let host = host
.or_else(|| first_header_value(req, &*X_FORWARDED_HOST))
.or_else(|| req.headers.get(&header::HOST)?.to_str().ok())
.or_else(|| req.uri.authority().map(Authority::as_str))
.unwrap_or(cfg.host())
.to_owned();
if realip_remote_addr.is_none() {
if let Some(h) = req
.headers
.get(&HeaderName::from_lowercase(X_FORWARDED_FOR).unwrap())
{
if let Ok(h) = h.to_str() {
realip_remote_addr = h.split(',').next().map(str::trim);
}
}
}
let realip_remote_addr = realip_remote_addr
.or_else(|| first_header_value(req, &*X_FORWARDED_FOR))
.map(str::to_owned);
let remote_addr = req.peer_addr.map(|addr| addr.to_string());
ConnectionInfo {
remote_addr,
scheme: scheme.unwrap_or("http").to_owned(),
host: host.unwrap_or("localhost").to_owned(),
realip_remote_addr: realip_remote_addr.map(str::to_owned),
scheme,
host,
realip_remote_addr,
}
}
@ -178,15 +177,16 @@ impl ConnectionInfo {
&self.host
}
/// remote_addr address of the request.
/// Remote address of the connection.
///
/// Get remote_addr address from socket address
/// Get remote_addr address from socket address.
pub fn remote_addr(&self) -> Option<&str> {
self.remote_addr.as_ref().map(String::as_ref)
self.remote_addr.as_deref()
}
/// Real ip remote addr of client initiated HTTP request.
/// Real IP (remote address) of client that initiated request.
///
/// The addr is resolved through the following headers, in this order:
/// The address is resolved through the following headers, in this order:
///
/// - Forwarded
/// - X-Forwarded-For
@ -195,15 +195,14 @@ impl ConnectionInfo {
/// # Security
/// Do not use this function for security purposes, unless you can ensure the Forwarded and
/// X-Forwarded-For headers cannot be spoofed by the client. If you want the client's socket
/// address explicitly, use
/// [`HttpRequest::peer_addr()`](super::web::HttpRequest::peer_addr()) instead.
/// address explicitly, use [`HttpRequest::peer_addr()`][peer_addr] instead.
///
/// [peer_addr]: crate::web::HttpRequest::peer_addr()
#[inline]
pub fn realip_remote_addr(&self) -> Option<&str> {
if let Some(ref r) = self.realip_remote_addr {
Some(r)
} else {
self.remote_addr.as_ref().map(String::as_ref)
}
self.realip_remote_addr
.as_deref()
.or_else(|| self.remote_addr.as_deref())
}
}
@ -271,13 +270,60 @@ mod tests {
use super::*;
use crate::test::TestRequest;
const X_FORWARDED_FOR: &str = "x-forwarded-for";
const X_FORWARDED_HOST: &str = "x-forwarded-host";
const X_FORWARDED_PROTO: &str = "x-forwarded-proto";
#[test]
fn test_forwarded() {
fn info_default() {
let req = TestRequest::default().to_http_request();
let info = req.connection_info();
assert_eq!(info.scheme(), "http");
assert_eq!(info.host(), "localhost:8080");
}
#[test]
fn host_header() {
let req = TestRequest::default()
.insert_header((header::HOST, "rust-lang.org"))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.scheme(), "http");
assert_eq!(info.host(), "rust-lang.org");
assert_eq!(info.realip_remote_addr(), None);
}
#[test]
fn x_forwarded_for_header() {
let req = TestRequest::default()
.insert_header((X_FORWARDED_FOR, "192.0.2.60"))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.realip_remote_addr(), Some("192.0.2.60"));
}
#[test]
fn x_forwarded_host_header() {
let req = TestRequest::default()
.insert_header((X_FORWARDED_HOST, "192.0.2.60"))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.host(), "192.0.2.60");
assert_eq!(info.realip_remote_addr(), None);
}
#[test]
fn x_forwarded_proto_header() {
let req = TestRequest::default()
.insert_header((X_FORWARDED_PROTO, "https"))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.scheme(), "https");
}
#[test]
fn forwarded_header() {
let req = TestRequest::default()
.insert_header((
header::FORWARDED,
@ -291,45 +337,111 @@ mod tests {
assert_eq!(info.realip_remote_addr(), Some("192.0.2.60"));
let req = TestRequest::default()
.insert_header((header::HOST, "rust-lang.org"))
.insert_header((
header::FORWARDED,
"for=192.0.2.60; proto=https; by=203.0.113.43; host=rust-lang.org",
))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.scheme(), "http");
assert_eq!(info.scheme(), "https");
assert_eq!(info.host(), "rust-lang.org");
assert_eq!(info.realip_remote_addr(), None);
assert_eq!(info.realip_remote_addr(), Some("192.0.2.60"));
}
#[test]
fn forwarded_case_sensitivity() {
let req = TestRequest::default()
.insert_header((X_FORWARDED_FOR, "192.0.2.60"))
.insert_header((header::FORWARDED, "For=192.0.2.60"))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.realip_remote_addr(), Some("192.0.2.60"));
}
#[test]
fn forwarded_weird_whitespace() {
let req = TestRequest::default()
.insert_header((X_FORWARDED_HOST, "192.0.2.60"))
.insert_header((header::FORWARDED, "for= 1.2.3.4; proto= https"))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.host(), "192.0.2.60");
assert_eq!(info.realip_remote_addr(), None);
assert_eq!(info.realip_remote_addr(), Some("1.2.3.4"));
assert_eq!(info.scheme(), "https");
let req = TestRequest::default()
.insert_header((X_FORWARDED_PROTO, "https"))
.insert_header((header::FORWARDED, " for = 1.2.3.4 "))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.realip_remote_addr(), Some("1.2.3.4"));
}
#[test]
fn forwarded_for_quoted() {
let req = TestRequest::default()
.insert_header((header::FORWARDED, r#"for="192.0.2.60:8080""#))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.realip_remote_addr(), Some("192.0.2.60:8080"));
}
#[test]
fn forwarded_for_ipv6() {
let req = TestRequest::default()
.insert_header((header::FORWARDED, r#"for="[2001:db8:cafe::17]:4711""#))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.realip_remote_addr(), Some("[2001:db8:cafe::17]:4711"));
}
#[test]
fn forwarded_for_multiple() {
let req = TestRequest::default()
.insert_header((header::FORWARDED, "for=192.0.2.60, for=198.51.100.17"))
.to_http_request();
let info = req.connection_info();
// takes the first value
assert_eq!(info.realip_remote_addr(), Some("192.0.2.60"));
}
#[test]
fn scheme_from_uri() {
let req = TestRequest::get()
.uri("https://actix.rs/test")
.to_http_request();
let info = req.connection_info();
assert_eq!(info.scheme(), "https");
}
#[actix_rt::test]
async fn test_conn_info() {
let req = TestRequest::default()
.uri("http://actix.rs/")
#[test]
fn host_from_uri() {
let req = TestRequest::get()
.uri("https://actix.rs/test")
.to_http_request();
let conn_info = ConnectionInfo::extract(&req).await.unwrap();
assert_eq!(conn_info.scheme(), "http");
let info = req.connection_info();
assert_eq!(info.host(), "actix.rs");
}
#[test]
fn host_from_server_hostname() {
let mut req = TestRequest::get();
req.set_server_hostname("actix.rs");
let req = req.to_http_request();
let info = req.connection_info();
assert_eq!(info.host(), "actix.rs");
}
#[actix_rt::test]
async fn test_peer_addr() {
async fn conn_info_extract() {
let req = TestRequest::default()
.uri("https://actix.rs/test")
.to_http_request();
let conn_info = ConnectionInfo::extract(&req).await.unwrap();
assert_eq!(conn_info.scheme(), "https");
assert_eq!(conn_info.host(), "actix.rs");
}
#[actix_rt::test]
async fn peer_addr_extract() {
let addr = "127.0.0.1:8080".parse().unwrap();
let req = TestRequest::default().peer_addr(addr).to_http_request();
let peer_addr = PeerAddr::extract(&req).await.unwrap();

View File

@ -613,6 +613,11 @@ impl TestRequest {
let req = self.to_request();
call_service(app, req).await
}
#[cfg(test)]
pub fn set_server_hostname(&mut self, host: &str) {
self.config.set_host(host)
}
}
#[cfg(test)]