Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

extract more config types from Data<T> as well #1641

Merged
merged 1 commit into from Sep 2, 2020
Merged

extract more config types from Data<T> as well #1641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions CHANGES.md
View file
Open in desktop
Expand Up @@ -3,11 +3,16 @@
## Unreleased - 2020-xx-xx
### Added
* `middleware::NormalizePath` now has configurable behaviour for either always having a trailing
slash, or as the new addition, always trimming trailing slashes.
slash, or as the new addition, always trimming trailing slashes. [#1639]

### Changed
* Update actix-codec and actix-utils dependencies.
* Update actix-codec and actix-utils dependencies. [#1634]
* `FormConfig` and `JsonConfig` configurations are now also considered when set
using `App::data`. [#1641]

[#1639]: https://github.com/actix/actix-web/pull/1639
[#1641]: https://github.com/actix/actix-web/pull/1641
[#1634]: https://github.com/actix/actix-web/pull/1634

## 3.0.0-beta.3 - 2020-08-17
### Changed
Expand Down
38 changes: 30 additions & 8 deletions src/types/form.rs
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@ use crate::http::{
StatusCode,
};
use crate::request::HttpRequest;
use crate::responder::Responder;
use crate::{responder::Responder, web};

/// Form data helper (`application/x-www-form-urlencoded`)
///
Expand Down Expand Up @@ -121,8 +121,12 @@ where
fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future {
let req2 = req.clone();
let (limit, err) = req
.app_data::<FormConfig>()
.map(|c| (c.limit, c.ehandler.clone()))
.app_data::<Self::Config>()
.or_else(|| {
req.app_data::<web::Data<Self::Config>>()
.map(|d| d.as_ref())
})
.map(|c| (c.limit, c.err_handler.clone()))
.unwrap_or((16384, None));

UrlEncoded::new(req, payload)
Expand Down Expand Up @@ -200,7 +204,7 @@ impl<T: Serialize> Responder for Form<T> {
#[derive(Clone)]
pub struct FormConfig {
limit: usize,
ehandler: Option<Rc<dyn Fn(UrlencodedError, &HttpRequest) -> Error>>,
err_handler: Option<Rc<dyn Fn(UrlencodedError, &HttpRequest) -> Error>>,
}

impl FormConfig {
Expand All @@ -215,16 +219,16 @@ impl FormConfig {
where
F: Fn(UrlencodedError, &HttpRequest) -> Error + 'static,
{
self.ehandler = Some(Rc::new(f));
self.err_handler = Some(Rc::new(f));
self
}
}

impl Default for FormConfig {
fn default() -> Self {
FormConfig {
limit: 16384,
ehandler: None,
limit: 16_384, // 2^14 bytes (~16kB)
err_handler: None,
}
}
}
Expand Down Expand Up @@ -378,7 +382,7 @@ mod tests {
use serde::{Deserialize, Serialize};

use super::*;
use crate::http::header::{HeaderValue, CONTENT_TYPE};
use crate::http::header::{HeaderValue, CONTENT_LENGTH, CONTENT_TYPE};
use crate::test::TestRequest;

#[derive(Deserialize, Serialize, Debug, PartialEq)]
Expand Down Expand Up @@ -499,4 +503,22 @@ mod tests {
use crate::responder::tests::BodyTest;
assert_eq!(resp.body().bin_ref(), b"hello=world&counter=123");
}

#[actix_rt::test]
async fn test_with_config_in_data_wrapper() {
let ctype = HeaderValue::from_static("application/x-www-form-urlencoded");

let (req, mut pl) = TestRequest::default()
.header(CONTENT_TYPE, ctype)
.header(CONTENT_LENGTH, HeaderValue::from_static("20"))
.set_payload(Bytes::from_static(b"hello=test&counter=4"))
.app_data(web::Data::new(FormConfig::default().limit(10)))
.to_http_parts();

let s = Form::<Info>::from_request(&req, &mut pl).await;
assert!(s.is_err());

let err_str = s.err().unwrap().to_string();
assert!(err_str.contains("Urlencoded payload size is bigger"));
}
}
58 changes: 44 additions & 14 deletions src/types/json.rs
View file
Open in desktop
Expand Up @@ -20,7 +20,7 @@ use crate::dev::Decompress;
use crate::error::{Error, JsonPayloadError};
use crate::extract::FromRequest;
use crate::request::HttpRequest;
use crate::responder::Responder;
use crate::{responder::Responder, web};

/// Json helper
///
Expand Down Expand Up @@ -179,10 +179,11 @@ where
#[inline]
fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future {
let req2 = req.clone();
let (limit, err, ctype) = req
.app_data::<Self::Config>()
.map(|c| (c.limit, c.ehandler.clone(), c.content_type.clone()))
.unwrap_or((32768, None, None));
let config = JsonConfig::from_req(req);

let limit = config.limit;
let ctype = config.content_type.clone();
let err_handler = config.err_handler.clone();

JsonBody::new(req, payload, ctype)
.limit(limit)
Expand All @@ -193,7 +194,8 @@ where
Request path: {}",
req2.path()
);
if let Some(err) = err {

if let Some(err) = err_handler {
Err((*err)(e, &req2))
} else {
Err(e.into())
Expand Down Expand Up @@ -255,7 +257,8 @@ where
#[derive(Clone)]
pub struct JsonConfig {
limit: usize,
ehandler: Option<Arc<dyn Fn(JsonPayloadError, &HttpRequest) -> Error + Send + Sync>>,
err_handler:
Option<Arc<dyn Fn(JsonPayloadError, &HttpRequest) -> Error + Send + Sync>>,
content_type: Option<Arc<dyn Fn(mime::Mime) -> bool + Send + Sync>>,
}

Expand All @@ -271,7 +274,7 @@ impl JsonConfig {
where
F: Fn(JsonPayloadError, &HttpRequest) -> Error + Send + Sync + 'static,
{
self.ehandler = Some(Arc::new(f));
self.err_handler = Some(Arc::new(f));
self
}

Expand All @@ -283,15 +286,26 @@ impl JsonConfig {
self.content_type = Some(Arc::new(predicate));
self
}

/// Extract payload config from app data. Check both `T` and `Data<T>`, in that order, and fall
/// back to the default payload config.
fn from_req(req: &HttpRequest) -> &Self {
req.app_data::<Self>()
.or_else(|| req.app_data::<web::Data<Self>>().map(|d| d.as_ref()))
.unwrap_or_else(|| &DEFAULT_CONFIG)
}
}

// Allow shared refs to default.
const DEFAULT_CONFIG: JsonConfig = JsonConfig {
limit: 32_768, // 2^15 bytes, (~32kB)
err_handler: None,
content_type: None,
};

impl Default for JsonConfig {
fn default() -> Self {
JsonConfig {
limit: 32768,
ehandler: None,
content_type: None,
}
DEFAULT_CONFIG.clone()
}
}

Expand Down Expand Up @@ -422,7 +436,7 @@ mod tests {

use super::*;
use crate::error::InternalError;
use crate::http::header;
use crate::http::header::{self, HeaderValue, CONTENT_LENGTH, CONTENT_TYPE};
use crate::test::{load_stream, TestRequest};
use crate::HttpResponse;

Expand Down Expand Up @@ -659,4 +673,20 @@ mod tests {
let s = Json::<MyObject>::from_request(&req, &mut pl).await;
assert!(s.is_err())
}

#[actix_rt::test]
async fn test_with_config_in_data_wrapper() {
let (req, mut pl) = TestRequest::default()
.header(CONTENT_TYPE, HeaderValue::from_static("application/json"))
.header(CONTENT_LENGTH, HeaderValue::from_static("16"))
.set_payload(Bytes::from_static(b"{\"name\": \"test\"}"))
.app_data(web::Data::new(JsonConfig::default().limit(10)))
.to_http_parts();

let s = Json::<MyObject>::from_request(&req, &mut pl).await;
assert!(s.is_err());

let err_str = s.err().unwrap().to_string();
assert!(err_str.contains("Json payload size is bigger than allowed"));
}
}
19 changes: 8 additions & 11 deletions src/types/payload.rs
View file
Open in desktop
Expand Up @@ -279,27 +279,24 @@ impl PayloadConfig {
Ok(())
}

/// Allow payload config extraction from app data checking both `T` and `Data<T>`, in that
/// order, and falling back to the default payload config.
fn from_req(req: &HttpRequest) -> &PayloadConfig {
req.app_data::<PayloadConfig>()
.or_else(|| {
req.app_data::<web::Data<PayloadConfig>>()
.map(|d| d.as_ref())
})
.unwrap_or_else(|| &DEFAULT_PAYLOAD_CONFIG)
/// Extract payload config from app data. Check both `T` and `Data<T>`, in that order, and fall
/// back to the default payload config.
fn from_req(req: &HttpRequest) -> &Self {
req.app_data::<Self>()
.or_else(|| req.app_data::<web::Data<Self>>().map(|d| d.as_ref()))
.unwrap_or_else(|| &DEFAULT_CONFIG)
}
}

// Allow shared refs to default.
static DEFAULT_PAYLOAD_CONFIG: PayloadConfig = PayloadConfig {
const DEFAULT_CONFIG: PayloadConfig = PayloadConfig {
limit: 262_144, // 2^18 bytes (~256kB)
mimetype: None,
};

impl Default for PayloadConfig {
fn default() -> Self {
DEFAULT_PAYLOAD_CONFIG.clone()
DEFAULT_CONFIG.clone()
}
}

Expand Down