From ce24630d31fc85e301bbb401111df56573964286 Mon Sep 17 00:00:00 2001 From: Huy Date: Wed, 22 Apr 2020 11:39:09 -0700 Subject: [PATCH 1/7] Fix typos in MIGRATION.md (#1470) * Fix typos in MIGRATION.md Those are `crate` not `create` * Update MIGRATION.md --- MIGRATION.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 86721e0eb..672aa253e 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -360,7 +360,7 @@ * `actix_web::server` module has been removed. To start http server use `actix_web::HttpServer` type -* StaticFiles and NamedFile has been move to separate create. +* StaticFiles and NamedFile have been moved to a separate crate. instead of `use actix_web::fs::StaticFile` @@ -370,7 +370,7 @@ use `use actix_files::NamedFile` -* Multipart has been move to separate create. +* Multipart has been moved to a separate crate. instead of `use actix_web::multipart::Multipart` From b047413b39fa98daf6a94e999512d295200d80ef Mon Sep 17 00:00:00 2001 From: Huston Bokinsky Date: Tue, 28 Apr 2020 19:13:09 -0700 Subject: [PATCH 2/7] Small ws codec fix (#1465) * Small ws codec fix * Update actix-http/Changes.md Co-authored-by: Huston Bokinsky --- actix-http/CHANGES.md | 2 ++ actix-http/src/ws/codec.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 140d78e95..43f189afc 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -7,6 +7,8 @@ * Implement `std::error::Error` for our custom errors [#1422] * Remove `failure` support for `ResponseError` since that crate will be deprecated in the near future. +* Fix a mistake in the encoding of websocket continuation messages wherein + Item::FirstText and Item::FirstBinary are each encoded as the other. [#1422]: https://github.com/actix/actix-web/pull/1422 diff --git a/actix-http/src/ws/codec.rs b/actix-http/src/ws/codec.rs index a37208a2b..733976a78 100644 --- a/actix-http/src/ws/codec.rs +++ b/actix-http/src/ws/codec.rs @@ -137,7 +137,7 @@ impl Encoder for Codec { Parser::write_message( dst, &data[..], - OpCode::Binary, + OpCode::Text, false, !self.flags.contains(Flags::SERVER), ) @@ -151,7 +151,7 @@ impl Encoder for Codec { Parser::write_message( dst, &data[..], - OpCode::Text, + OpCode::Binary, false, !self.flags.contains(Flags::SERVER), ) From bb17280f512927cbeed95a6ce99dd1d42e463add Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Wed, 29 Apr 2020 07:38:53 +0100 Subject: [PATCH 3/7] simplify data factory future polling (#1473) Co-authored-by: Yuki Okushi --- src/app.rs | 15 +++++- src/app_service.rs | 51 ++++++++++++-------- src/test.rs | 118 ++++++++++++++++++++++++++------------------- 3 files changed, 113 insertions(+), 71 deletions(-) diff --git a/src/app.rs b/src/app.rs index ed2aff8e6..c611f2657 100644 --- a/src/app.rs +++ b/src/app.rs @@ -476,13 +476,13 @@ where mod tests { use actix_service::Service; use bytes::Bytes; - use futures::future::ok; + use futures::future::{ok, err}; use super::*; use crate::http::{header, HeaderValue, Method, StatusCode}; use crate::middleware::DefaultHeaders; use crate::service::ServiceRequest; - use crate::test::{call_service, init_service, read_body, TestRequest}; + use crate::test::{call_service, init_service, try_init_service, read_body, TestRequest}; use crate::{web, HttpRequest, HttpResponse}; #[actix_rt::test] @@ -551,6 +551,17 @@ mod tests { assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); } + #[actix_rt::test] + async fn test_data_factory_errors() { + let srv = + try_init_service(App::new().data_factory(|| err::(())).service( + web::resource("/").to(|_: web::Data| HttpResponse::Ok()), + )) + .await; + + assert!(srv.is_err()); + } + #[actix_rt::test] async fn test_extension() { let mut srv = init_service(App::new().app_data(10usize).service( diff --git a/src/app_service.rs b/src/app_service.rs index ccfefbc68..67fa4dc2c 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -9,7 +9,7 @@ use actix_http::{Extensions, Request, Response}; use actix_router::{Path, ResourceDef, ResourceInfo, Router, Url}; use actix_service::boxed::{self, BoxService, BoxServiceFactory}; use actix_service::{fn_service, Service, ServiceFactory}; -use futures::future::{ok, FutureExt, LocalBoxFuture}; +use futures::future::{join_all, ok, FutureExt, LocalBoxFuture}; use crate::config::{AppConfig, AppService}; use crate::data::DataFactory; @@ -109,12 +109,15 @@ where let rmap = Rc::new(rmap); rmap.finish(rmap.clone()); + // start all data factory futures + let factory_futs = join_all(self.data_factories.iter().map(|f| f())); + AppInitResult { endpoint: None, endpoint_fut: self.endpoint.new_service(()), data: self.data.clone(), - data_factories: Vec::new(), - data_factories_fut: self.data_factories.iter().map(|f| f()).collect(), + data_factories: None, + data_factories_fut: factory_futs.boxed_local(), extensions: Some( self.extensions .borrow_mut() @@ -133,15 +136,21 @@ pub struct AppInitResult where T: ServiceFactory, { - endpoint: Option, #[pin] endpoint_fut: T::Future, + // a Some signals completion of endpoint creation + endpoint: Option, + + #[pin] + data_factories_fut: LocalBoxFuture<'static, Vec, ()>>>, + // a Some signals completion of factory futures + data_factories: Option>>, + rmap: Rc, config: AppConfig, data: Rc>>, - data_factories: Vec>, - data_factories_fut: Vec, ()>>>, extensions: Option, + _t: PhantomData, } @@ -161,44 +170,46 @@ where let this = self.project(); // async data factories - let mut idx = 0; - while idx < this.data_factories_fut.len() { - match Pin::new(&mut this.data_factories_fut[idx]).poll(cx)? { - Poll::Ready(f) => { - this.data_factories.push(f); - let _ = this.data_factories_fut.remove(idx); - } - Poll::Pending => idx += 1, + if let Poll::Ready(factories) = this.data_factories_fut.poll(cx) { + let factories: Result, ()> = factories.into_iter().collect(); + + if let Ok(factories) = factories { + this.data_factories.replace(factories); + } else { + return Poll::Ready(Err(())); } } + // app service and middleware if this.endpoint.is_none() { if let Poll::Ready(srv) = this.endpoint_fut.poll(cx)? { *this.endpoint = Some(srv); } } - if this.endpoint.is_some() && this.data_factories_fut.is_empty() { + // not using if let so condition only needs shared ref + if this.endpoint.is_some() && this.data_factories.is_some() { // create app data container let mut data = this.extensions.take().unwrap(); + for f in this.data.iter() { f.create(&mut data); } - for f in this.data_factories.iter() { + for f in this.data_factories.take().unwrap().iter() { f.create(&mut data); } - Poll::Ready(Ok(AppInitService { + return Poll::Ready(Ok(AppInitService { service: this.endpoint.take().unwrap(), rmap: this.rmap.clone(), config: this.config.clone(), data: Rc::new(data), pool: HttpRequestPool::create(), - })) - } else { - Poll::Pending + })); } + + Poll::Pending } } diff --git a/src/test.rs b/src/test.rs index 19ea8bbef..c8a738d83 100644 --- a/src/test.rs +++ b/src/test.rs @@ -78,6 +78,26 @@ pub fn default_service( pub async fn init_service( app: R, ) -> impl Service, Error = E> +where + R: IntoServiceFactory, + S: ServiceFactory< + Config = AppConfig, + Request = Request, + Response = ServiceResponse, + Error = E, + >, + S::InitError: std::fmt::Debug, +{ + try_init_service(app).await.expect("service initilization failed") +} + +/// Fallible version of init_service that allows testing data factory errors. +pub(crate) async fn try_init_service( + app: R, +) -> Result< + impl Service, Error = E>, + S::InitError, +> where R: IntoServiceFactory, S: ServiceFactory< @@ -89,7 +109,7 @@ where S::InitError: std::fmt::Debug, { let srv = app.into_factory(); - srv.new_service(AppConfig::default()).await.unwrap() + srv.new_service(AppConfig::default()).await } /// Calls service and waits for response future completion. @@ -580,7 +600,7 @@ impl TestRequest { pub async fn send_request(self, app: &mut S) -> S::Response where S: Service, Error = E>, - E: std::fmt::Debug + E: std::fmt::Debug, { let req = self.to_request(); call_service(app, req).await @@ -1125,8 +1145,8 @@ mod tests { #[actix_rt::test] async fn test_response_json() { let mut app = init_service(App::new().service(web::resource("/people").route( - web::post().to(|person: web::Json| { - async { HttpResponse::Ok().json(person.into_inner()) } + web::post().to(|person: web::Json| async { + HttpResponse::Ok().json(person.into_inner()) }), ))) .await; @@ -1146,8 +1166,8 @@ mod tests { #[actix_rt::test] async fn test_body_json() { let mut app = init_service(App::new().service(web::resource("/people").route( - web::post().to(|person: web::Json| { - async { HttpResponse::Ok().json(person.into_inner()) } + web::post().to(|person: web::Json| async { + HttpResponse::Ok().json(person.into_inner()) }), ))) .await; @@ -1168,8 +1188,8 @@ mod tests { #[actix_rt::test] async fn test_request_response_form() { let mut app = init_service(App::new().service(web::resource("/people").route( - web::post().to(|person: web::Form| { - async { HttpResponse::Ok().json(person.into_inner()) } + web::post().to(|person: web::Form| async { + HttpResponse::Ok().json(person.into_inner()) }), ))) .await; @@ -1194,8 +1214,8 @@ mod tests { #[actix_rt::test] async fn test_request_response_json() { let mut app = init_service(App::new().service(web::resource("/people").route( - web::post().to(|person: web::Json| { - async { HttpResponse::Ok().json(person.into_inner()) } + web::post().to(|person: web::Json| async { + HttpResponse::Ok().json(person.into_inner()) }), ))) .await; @@ -1259,53 +1279,53 @@ mod tests { assert!(res.status().is_success()); } -/* + /* - Comment out until actix decoupled of actix-http: - https://github.com/actix/actix/issues/321 + Comment out until actix decoupled of actix-http: + https://github.com/actix/actix/issues/321 - use futures::FutureExt; + use futures::FutureExt; - #[actix_rt::test] - async fn test_actor() { - use actix::Actor; + #[actix_rt::test] + async fn test_actor() { + use actix::Actor; - struct MyActor; + struct MyActor; - struct Num(usize); - impl actix::Message for Num { - type Result = usize; - } - impl actix::Actor for MyActor { - type Context = actix::Context; - } - impl actix::Handler for MyActor { - type Result = usize; - fn handle(&mut self, msg: Num, _: &mut Self::Context) -> Self::Result { - msg.0 + struct Num(usize); + impl actix::Message for Num { + type Result = usize; + } + impl actix::Actor for MyActor { + type Context = actix::Context; + } + impl actix::Handler for MyActor { + type Result = usize; + fn handle(&mut self, msg: Num, _: &mut Self::Context) -> Self::Result { + msg.0 + } } - } - let mut app = init_service(App::new().service(web::resource("/index.html").to( - move || { - addr.send(Num(1)).map(|res| match res { - Ok(res) => { - if res == 1 { - Ok(HttpResponse::Ok()) - } else { - Ok(HttpResponse::BadRequest()) + let mut app = init_service(App::new().service(web::resource("/index.html").to( + move || { + addr.send(Num(1)).map(|res| match res { + Ok(res) => { + if res == 1 { + Ok(HttpResponse::Ok()) + } else { + Ok(HttpResponse::BadRequest()) + } } - } - Err(err) => Err(err), - }) - }, - ))) - .await; + Err(err) => Err(err), + }) + }, + ))) + .await; - let req = TestRequest::post().uri("/index.html").to_request(); - let res = app.call(req).await.unwrap(); - assert!(res.status().is_success()); - } -*/ + let req = TestRequest::post().uri("/index.html").to_request(); + let res = app.call(req).await.unwrap(); + assert!(res.status().is_success()); + } + */ } From c27d3fad8ece1e1440153f0abb47b96927c66f97 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Wed, 29 Apr 2020 18:20:47 +0100 Subject: [PATCH 4/7] clarify resource/scope app data overriding (#1476) * relocate FnDataFactory * clarify app data overriding in Scope and Resource Co-authored-by: Yuki Okushi --- actix-http/src/extensions.rs | 193 ++++++++++++++++++----------------- src/app.rs | 6 +- src/app_service.rs | 4 +- src/data.rs | 5 +- src/resource.rs | 85 ++++++++++++++- src/scope.rs | 6 +- 6 files changed, 191 insertions(+), 108 deletions(-) diff --git a/actix-http/src/extensions.rs b/actix-http/src/extensions.rs index 5114ce140..6a4a034a4 100644 --- a/actix-http/src/extensions.rs +++ b/actix-http/src/extensions.rs @@ -67,108 +67,113 @@ impl fmt::Debug for Extensions { } } -#[test] -fn test_remove() { - let mut map = Extensions::new(); +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_remove() { + let mut map = Extensions::new(); - map.insert::(123); - assert!(map.get::().is_some()); + map.insert::(123); + assert!(map.get::().is_some()); - map.remove::(); - assert!(map.get::().is_none()); -} - -#[test] -fn test_clear() { - let mut map = Extensions::new(); - - map.insert::(8); - map.insert::(16); - map.insert::(32); - - assert!(map.contains::()); - assert!(map.contains::()); - assert!(map.contains::()); - - map.clear(); - - assert!(!map.contains::()); - assert!(!map.contains::()); - assert!(!map.contains::()); - - map.insert::(10); - assert_eq!(*map.get::().unwrap(), 10); -} - -#[test] -fn test_integers() { - let mut map = Extensions::new(); - - map.insert::(8); - map.insert::(16); - map.insert::(32); - map.insert::(64); - map.insert::(128); - map.insert::(8); - map.insert::(16); - map.insert::(32); - map.insert::(64); - map.insert::(128); - assert!(map.get::().is_some()); - assert!(map.get::().is_some()); - assert!(map.get::().is_some()); - assert!(map.get::().is_some()); - assert!(map.get::().is_some()); - assert!(map.get::().is_some()); - assert!(map.get::().is_some()); - assert!(map.get::().is_some()); - assert!(map.get::().is_some()); - assert!(map.get::().is_some()); -} - -#[test] -fn test_composition() { - struct Magi(pub T); - - struct Madoka { - pub god: bool, + map.remove::(); + assert!(map.get::().is_none()); } - struct Homura { - pub attempts: usize, + #[test] + fn test_clear() { + let mut map = Extensions::new(); + + map.insert::(8); + map.insert::(16); + map.insert::(32); + + assert!(map.contains::()); + assert!(map.contains::()); + assert!(map.contains::()); + + map.clear(); + + assert!(!map.contains::()); + assert!(!map.contains::()); + assert!(!map.contains::()); + + map.insert::(10); + assert_eq!(*map.get::().unwrap(), 10); } - struct Mami { - pub guns: usize, + #[test] + fn test_integers() { + let mut map = Extensions::new(); + + map.insert::(8); + map.insert::(16); + map.insert::(32); + map.insert::(64); + map.insert::(128); + map.insert::(8); + map.insert::(16); + map.insert::(32); + map.insert::(64); + map.insert::(128); + assert!(map.get::().is_some()); + assert!(map.get::().is_some()); + assert!(map.get::().is_some()); + assert!(map.get::().is_some()); + assert!(map.get::().is_some()); + assert!(map.get::().is_some()); + assert!(map.get::().is_some()); + assert!(map.get::().is_some()); + assert!(map.get::().is_some()); + assert!(map.get::().is_some()); } - let mut map = Extensions::new(); + #[test] + fn test_composition() { + struct Magi(pub T); - map.insert(Magi(Madoka { god: false })); - map.insert(Magi(Homura { attempts: 0 })); - map.insert(Magi(Mami { guns: 999 })); + struct Madoka { + pub god: bool, + } - assert!(!map.get::>().unwrap().0.god); - assert_eq!(0, map.get::>().unwrap().0.attempts); - assert_eq!(999, map.get::>().unwrap().0.guns); -} - -#[test] -fn test_extensions() { - #[derive(Debug, PartialEq)] - struct MyType(i32); - - let mut extensions = Extensions::new(); - - extensions.insert(5i32); - extensions.insert(MyType(10)); - - assert_eq!(extensions.get(), Some(&5i32)); - assert_eq!(extensions.get_mut(), Some(&mut 5i32)); - - assert_eq!(extensions.remove::(), Some(5i32)); - assert!(extensions.get::().is_none()); - - assert_eq!(extensions.get::(), None); - assert_eq!(extensions.get(), Some(&MyType(10))); + struct Homura { + pub attempts: usize, + } + + struct Mami { + pub guns: usize, + } + + let mut map = Extensions::new(); + + map.insert(Magi(Madoka { god: false })); + map.insert(Magi(Homura { attempts: 0 })); + map.insert(Magi(Mami { guns: 999 })); + + assert!(!map.get::>().unwrap().0.god); + assert_eq!(0, map.get::>().unwrap().0.attempts); + assert_eq!(999, map.get::>().unwrap().0.guns); + } + + #[test] + fn test_extensions() { + #[derive(Debug, PartialEq)] + struct MyType(i32); + + let mut extensions = Extensions::new(); + + extensions.insert(5i32); + extensions.insert(MyType(10)); + + assert_eq!(extensions.get(), Some(&5i32)); + assert_eq!(extensions.get_mut(), Some(&mut 5i32)); + + assert_eq!(extensions.remove::(), Some(5i32)); + assert!(extensions.get::().is_none()); + + assert_eq!(extensions.get::(), None); + assert_eq!(extensions.get(), Some(&MyType(10))); + } } diff --git a/src/app.rs b/src/app.rs index c611f2657..7d3100db7 100644 --- a/src/app.rs +++ b/src/app.rs @@ -10,11 +10,11 @@ use actix_service::boxed::{self, BoxServiceFactory}; use actix_service::{ apply, apply_fn_factory, IntoServiceFactory, ServiceFactory, Transform, }; -use futures::future::{FutureExt, LocalBoxFuture}; +use futures::future::FutureExt; use crate::app_service::{AppEntry, AppInit, AppRoutingFactory}; use crate::config::ServiceConfig; -use crate::data::{Data, DataFactory}; +use crate::data::{Data, DataFactory, FnDataFactory}; use crate::dev::ResourceDef; use crate::error::Error; use crate::resource::Resource; @@ -25,8 +25,6 @@ use crate::service::{ }; type HttpNewService = BoxServiceFactory<(), ServiceRequest, ServiceResponse, Error, ()>; -type FnDataFactory = - Box LocalBoxFuture<'static, Result, ()>>>; /// Application builder - structure that follows the builder pattern /// for building application instances. diff --git a/src/app_service.rs b/src/app_service.rs index 67fa4dc2c..808592e58 100644 --- a/src/app_service.rs +++ b/src/app_service.rs @@ -12,7 +12,7 @@ use actix_service::{fn_service, Service, ServiceFactory}; use futures::future::{join_all, ok, FutureExt, LocalBoxFuture}; use crate::config::{AppConfig, AppService}; -use crate::data::DataFactory; +use crate::data::{FnDataFactory, DataFactory}; use crate::error::Error; use crate::guard::Guard; use crate::request::{HttpRequest, HttpRequestPool}; @@ -23,8 +23,6 @@ type Guards = Vec>; type HttpService = BoxService; type HttpNewService = BoxServiceFactory<(), ServiceRequest, ServiceResponse, Error, ()>; type BoxResponse = LocalBoxFuture<'static, Result>; -type FnDataFactory = - Box LocalBoxFuture<'static, Result, ()>>>; /// Service factory to convert `Request` to a `ServiceRequest`. /// It also executes data factories. diff --git a/src/data.rs b/src/data.rs index c36418942..0c04e1d90 100644 --- a/src/data.rs +++ b/src/data.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use actix_http::error::{Error, ErrorInternalServerError}; use actix_http::Extensions; -use futures::future::{err, ok, Ready}; +use futures::future::{err, ok, LocalBoxFuture, Ready}; use crate::dev::Payload; use crate::extract::FromRequest; @@ -14,6 +14,9 @@ pub(crate) trait DataFactory { fn create(&self, extensions: &mut Extensions) -> bool; } +pub(crate) type FnDataFactory = + Box LocalBoxFuture<'static, Result, ()>>>; + /// Application data. /// /// Application data is an arbitrary data attached to the app. diff --git a/src/resource.rs b/src/resource.rs index dba32b43c..634294cc2 100644 --- a/src/resource.rs +++ b/src/resource.rs @@ -196,9 +196,11 @@ where self.app_data(Data::new(data)) } - /// Set or override application data. + /// Add resource data. /// - /// This method overrides data stored with [`App::app_data()`](#method.app_data) + /// If used, this method will create a new data context used for extracting + /// from requests. Data added here is *not* merged with data added on App + /// or containing scopes. pub fn app_data(mut self, data: U) -> Self { if self.data.is_none() { self.data = Some(Extensions::new()); @@ -393,6 +395,7 @@ where if let Some(ref mut ext) = self.data { config.set_service_data(ext); } + config.register_service(rdef, guards, self, None) } } @@ -587,13 +590,14 @@ mod tests { use actix_rt::time::delay_for; use actix_service::Service; + use bytes::Bytes; use futures::future::ok; use crate::http::{header, HeaderValue, Method, StatusCode}; use crate::middleware::DefaultHeaders; use crate::service::ServiceRequest; - use crate::test::{call_service, init_service, TestRequest}; - use crate::{guard, web, App, Error, HttpResponse}; + use crate::test::{call_service, init_service, read_body, TestRequest}; + use crate::{guard, web, App, Error, HttpRequest, HttpResponse}; #[actix_rt::test] async fn test_middleware() { @@ -619,6 +623,79 @@ mod tests { ); } + #[actix_rt::test] + async fn test_overwritten_data() { + #[allow(dead_code)] + fn echo_usize(req: HttpRequest) -> HttpResponse { + let num = req.app_data::().unwrap(); + HttpResponse::Ok().body(format!("{}", num)) + } + + #[allow(dead_code)] + fn echo_u32(req: HttpRequest) -> HttpResponse { + let num = req.app_data::().unwrap(); + HttpResponse::Ok().body(format!("{}", num)) + } + + #[allow(dead_code)] + fn echo_both(req: HttpRequest) -> HttpResponse { + let num = req.app_data::().unwrap(); + let num2 = req.app_data::().unwrap(); + HttpResponse::Ok().body(format!("{}-{}", num, num2)) + } + + let mut srv = init_service( + App::new() + .app_data(88usize) + .service(web::resource("/").route(web::get().to(echo_usize))) + .service( + web::resource("/one") + .app_data(1usize) + .route(web::get().to(echo_usize)), + ) + .service( + web::resource("/two") + .app_data(2usize) + .route(web::get().to(echo_usize)), + ) + .service( + web::resource("/three") + .app_data(3u32) + // this doesnt work because app_data "overrides" the + // entire data field potentially passed down + // .route(web::get().to(echo_both)), + .route(web::get().to(echo_u32)), + ) + .service(web::resource("/eight").route(web::get().to(echo_usize))), + ) + .await; + + let req = TestRequest::get().uri("/").to_request(); + let resp = srv.call(req).await.unwrap(); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"88")); + + let req = TestRequest::get().uri("/one").to_request(); + let resp = srv.call(req).await.unwrap(); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"1")); + + let req = TestRequest::get().uri("/two").to_request(); + let resp = srv.call(req).await.unwrap(); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"2")); + + // let req = TestRequest::get().uri("/three").to_request(); + // let resp = srv.call(req).await.unwrap(); + // let body = read_body(resp).await; + // assert_eq!(body, Bytes::from_static(b"88-3")); + + let req = TestRequest::get().uri("/eight").to_request(); + let resp = srv.call(req).await.unwrap(); + let body = read_body(resp).await; + assert_eq!(body, Bytes::from_static(b"88")); + } + #[actix_rt::test] async fn test_middleware_fn() { let mut srv = init_service( diff --git a/src/scope.rs b/src/scope.rs index 1569e90b0..2016c6f1c 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -151,9 +151,11 @@ where self.app_data(Data::new(data)) } - /// Set or override application data. + /// Add scope data. /// - /// This method overrides data stored with [`App::app_data()`](#method.app_data) + /// If used, this method will create a new data context used for extracting + /// from requests. Data added here is *not* merged with data added on App + /// or containing scopes. pub fn app_data(mut self, data: U) -> Self { if self.data.is_none() { self.data = Some(Extensions::new()); From d5ceae20740837bddef85053c3354452b437877a Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Sat, 2 May 2020 12:14:50 +0300 Subject: [PATCH 5/7] Replace deprecated now with now_utc (#1481) * Replace deprecated now with now_utc * Update doctest --- actix-http/src/config.rs | 2 +- actix-http/src/cookie/builder.rs | 2 +- actix-http/src/cookie/jar.rs | 2 +- actix-http/src/cookie/mod.rs | 2 +- actix-http/src/time_parser.rs | 2 +- src/middleware/logger.rs | 16 ++++++++-------- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/actix-http/src/config.rs b/actix-http/src/config.rs index 3221b9b8a..abf3d8ff9 100644 --- a/actix-http/src/config.rs +++ b/actix-http/src/config.rs @@ -214,7 +214,7 @@ impl Date { write!( self, "{}", - OffsetDateTime::now().format("%a, %d %b %Y %H:%M:%S GMT") + OffsetDateTime::now_utc().format("%a, %d %b %Y %H:%M:%S GMT") ) .unwrap(); } diff --git a/actix-http/src/cookie/builder.rs b/actix-http/src/cookie/builder.rs index 80e7ee71f..b64352e35 100644 --- a/actix-http/src/cookie/builder.rs +++ b/actix-http/src/cookie/builder.rs @@ -63,7 +63,7 @@ impl CookieBuilder { /// use actix_http::cookie::Cookie; /// /// let c = Cookie::build("foo", "bar") - /// .expires(time::OffsetDateTime::now()) + /// .expires(time::OffsetDateTime::now_utc()) /// .finish(); /// /// assert!(c.expires().is_some()); diff --git a/actix-http/src/cookie/jar.rs b/actix-http/src/cookie/jar.rs index 9fa6bdc7d..0c76c1cfe 100644 --- a/actix-http/src/cookie/jar.rs +++ b/actix-http/src/cookie/jar.rs @@ -221,7 +221,7 @@ impl CookieJar { if self.original_cookies.contains(cookie.name()) { cookie.set_value(""); cookie.set_max_age(Duration::zero()); - cookie.set_expires(OffsetDateTime::now() - Duration::days(365)); + cookie.set_expires(OffsetDateTime::now_utc() - Duration::days(365)); self.delta_cookies.replace(DeltaCookie::removed(cookie)); } else { self.delta_cookies.remove(cookie.name()); diff --git a/actix-http/src/cookie/mod.rs b/actix-http/src/cookie/mod.rs index b8ea6f4af..b94e0fe0f 100644 --- a/actix-http/src/cookie/mod.rs +++ b/actix-http/src/cookie/mod.rs @@ -733,7 +733,7 @@ impl<'c> Cookie<'c> { pub fn make_permanent(&mut self) { let twenty_years = Duration::days(365 * 20); self.set_max_age(twenty_years); - self.set_expires(OffsetDateTime::now() + twenty_years); + self.set_expires(OffsetDateTime::now_utc() + twenty_years); } fn fmt_parameters(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { diff --git a/actix-http/src/time_parser.rs b/actix-http/src/time_parser.rs index b5b07ccba..0d06a5867 100644 --- a/actix-http/src/time_parser.rs +++ b/actix-http/src/time_parser.rs @@ -19,7 +19,7 @@ fn try_parse_rfc_850(time: &str) -> Option { // If the `time` string contains a two-digit year, then as per RFC 2616 ยง 19.3, // we consider the year as part of this century if it's within the next 50 years, // otherwise we consider as part of the previous century. - let now = OffsetDateTime::now(); + let now = OffsetDateTime::now_utc(); let century_start_year = (now.year() / 100) * 100; let mut expanded_year = century_start_year + dt.year(); diff --git a/src/middleware/logger.rs b/src/middleware/logger.rs index e40fe648a..7d1577c96 100644 --- a/src/middleware/logger.rs +++ b/src/middleware/logger.rs @@ -163,11 +163,11 @@ where LoggerResponse { fut: self.service.call(req), format: None, - time: OffsetDateTime::now(), + time: OffsetDateTime::now_utc(), _t: PhantomData, } } else { - let now = OffsetDateTime::now(); + let now = OffsetDateTime::now_utc(); let mut format = self.inner.format.clone(); for unit in &mut format.0 { @@ -380,12 +380,12 @@ impl FormatText { FormatText::Percent => "%".fmt(fmt), FormatText::ResponseSize => size.fmt(fmt), FormatText::Time => { - let rt = OffsetDateTime::now() - entry_time; + let rt = OffsetDateTime::now_utc() - entry_time; let rt = rt.as_seconds_f64(); fmt.write_fmt(format_args!("{:.6}", rt)) } FormatText::TimeMillis => { - let rt = OffsetDateTime::now() - entry_time; + let rt = OffsetDateTime::now_utc() - entry_time; let rt = (rt.whole_nanoseconds() as f64) / 1_000_000.0; fmt.write_fmt(format_args!("{:.6}", rt)) } @@ -520,7 +520,7 @@ mod tests { .uri("/test/route/yeah") .to_srv_request(); - let now = OffsetDateTime::now(); + let now = OffsetDateTime::now_utc(); for unit in &mut format.0 { unit.render_request(now, &req); } @@ -551,7 +551,7 @@ mod tests { ) .to_srv_request(); - let now = OffsetDateTime::now(); + let now = OffsetDateTime::now_utc(); for unit in &mut format.0 { unit.render_request(now, &req); } @@ -561,7 +561,7 @@ mod tests { unit.render_response(&resp); } - let entry_time = OffsetDateTime::now(); + let entry_time = OffsetDateTime::now_utc(); let render = |fmt: &mut Formatter<'_>| { for unit in &format.0 { unit.render(fmt, 1024, entry_time)?; @@ -579,7 +579,7 @@ mod tests { let mut format = Format::new("%t"); let req = TestRequest::default().to_srv_request(); - let now = OffsetDateTime::now(); + let now = OffsetDateTime::now_utc(); for unit in &mut format.0 { unit.render_request(now, &req); } From f37cb6dd0b484708bad5ff75cf7c0f15baa19b70 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 3 May 2020 09:37:40 +0100 Subject: [PATCH 6/7] refactor h1 status line helper to remove unsafe usage (#1484) Co-authored-by: Yuki Okushi --- actix-http/Cargo.toml | 4 + actix-http/benches/status-line.rs | 222 ++++++++++++++++++++++++++++++ actix-http/src/helpers.rs | 101 ++++++-------- 3 files changed, 269 insertions(+), 58 deletions(-) create mode 100644 actix-http/benches/status-line.rs diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index fcb05dd37..b2c6b8e0a 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -101,3 +101,7 @@ rust-tls = { version="0.17", package = "rustls" } [[bench]] name = "content-length" harness = false + +[[bench]] +name = "status-line" +harness = false diff --git a/actix-http/benches/status-line.rs b/actix-http/benches/status-line.rs new file mode 100644 index 000000000..51f840f89 --- /dev/null +++ b/actix-http/benches/status-line.rs @@ -0,0 +1,222 @@ +use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion}; + +use bytes::BytesMut; +use http::Version; + +const CODES: &[u16] = &[201, 303, 404, 515]; + +fn bench_write_status_line_11(c: &mut Criterion) { + let mut group = c.benchmark_group("write_status_line v1.1"); + + let version = Version::HTTP_11; + + for i in CODES.iter() { + group.bench_with_input(BenchmarkId::new("Original (unsafe)", i), i, |b, &i| { + b.iter(|| { + let mut b = BytesMut::with_capacity(35); + _original::write_status_line(version, i, &mut b); + }) + }); + + group.bench_with_input(BenchmarkId::new("New (safe)", i), i, |b, &i| { + b.iter(|| { + let mut b = BytesMut::with_capacity(35); + _new::write_status_line(version, i, &mut b); + }) + }); + + group.bench_with_input(BenchmarkId::new("Naive", i), i, |b, &i| { + b.iter(|| { + let mut b = BytesMut::with_capacity(35); + _naive::write_status_line(version, i, &mut b); + }) + }); + } + + group.finish(); +} + +fn bench_write_status_line_10(c: &mut Criterion) { + let mut group = c.benchmark_group("write_status_line v1.0"); + + let version = Version::HTTP_10; + + for i in CODES.iter() { + group.bench_with_input(BenchmarkId::new("Original (unsafe)", i), i, |b, &i| { + b.iter(|| { + let mut b = BytesMut::with_capacity(35); + _original::write_status_line(version, i, &mut b); + }) + }); + + group.bench_with_input(BenchmarkId::new("New (safe)", i), i, |b, &i| { + b.iter(|| { + let mut b = BytesMut::with_capacity(35); + _new::write_status_line(version, i, &mut b); + }) + }); + + group.bench_with_input(BenchmarkId::new("Naive", i), i, |b, &i| { + b.iter(|| { + let mut b = BytesMut::with_capacity(35); + _naive::write_status_line(version, i, &mut b); + }) + }); + } + + group.finish(); +} + +fn bench_write_status_line_09(c: &mut Criterion) { + let mut group = c.benchmark_group("write_status_line v0.9"); + + let version = Version::HTTP_09; + + for i in CODES.iter() { + group.bench_with_input(BenchmarkId::new("Original (unsafe)", i), i, |b, &i| { + b.iter(|| { + let mut b = BytesMut::with_capacity(35); + _original::write_status_line(version, i, &mut b); + }) + }); + + group.bench_with_input(BenchmarkId::new("New (safe)", i), i, |b, &i| { + b.iter(|| { + let mut b = BytesMut::with_capacity(35); + _new::write_status_line(version, i, &mut b); + }) + }); + + group.bench_with_input(BenchmarkId::new("Naive", i), i, |b, &i| { + b.iter(|| { + let mut b = BytesMut::with_capacity(35); + _naive::write_status_line(version, i, &mut b); + }) + }); + } + + group.finish(); +} + +criterion_group!( + benches, + bench_write_status_line_11, + bench_write_status_line_10, + bench_write_status_line_09 +); +criterion_main!(benches); + +mod _naive { + use bytes::{BufMut, BytesMut}; + use http::Version; + + pub(crate) fn write_status_line(version: Version, n: u16, bytes: &mut BytesMut) { + match version { + Version::HTTP_11 => bytes.put_slice(b"HTTP/1.1 "), + Version::HTTP_10 => bytes.put_slice(b"HTTP/1.0 "), + Version::HTTP_09 => bytes.put_slice(b"HTTP/0.9 "), + _ => { + // other HTTP version handlers do not use this method + } + } + + bytes.put_slice(n.to_string().as_bytes()); + } +} + +mod _new { + use bytes::{BufMut, BytesMut}; + use http::Version; + + const DIGITS_START: u8 = b'0'; + + pub(crate) fn write_status_line(version: Version, n: u16, bytes: &mut BytesMut) { + match version { + Version::HTTP_11 => bytes.put_slice(b"HTTP/1.1 "), + Version::HTTP_10 => bytes.put_slice(b"HTTP/1.0 "), + Version::HTTP_09 => bytes.put_slice(b"HTTP/0.9 "), + _ => { + // other HTTP version handlers do not use this method + } + } + + let d100 = (n / 100) as u8; + let d10 = ((n / 10) % 10) as u8; + let d1 = (n % 10) as u8; + + bytes.put_u8(DIGITS_START + d100); + bytes.put_u8(DIGITS_START + d10); + bytes.put_u8(DIGITS_START + d1); + + bytes.put_u8(b' '); + } +} + +mod _original { + use std::ptr; + + use bytes::{BufMut, BytesMut}; + use http::Version; + + const DEC_DIGITS_LUT: &[u8] = b"0001020304050607080910111213141516171819\ + 2021222324252627282930313233343536373839\ + 4041424344454647484950515253545556575859\ + 6061626364656667686970717273747576777879\ + 8081828384858687888990919293949596979899"; + + pub(crate) const STATUS_LINE_BUF_SIZE: usize = 13; + + pub(crate) fn write_status_line(version: Version, mut n: u16, bytes: &mut BytesMut) { + let mut buf: [u8; STATUS_LINE_BUF_SIZE] = *b"HTTP/1.1 "; + + match version { + Version::HTTP_2 => buf[5] = b'2', + Version::HTTP_10 => buf[7] = b'0', + Version::HTTP_09 => { + buf[5] = b'0'; + buf[7] = b'9'; + } + _ => (), + } + + let mut curr: isize = 12; + let buf_ptr = buf.as_mut_ptr(); + let lut_ptr = DEC_DIGITS_LUT.as_ptr(); + let four = n > 999; + + // decode 2 more chars, if > 2 chars + let d1 = (n % 100) << 1; + n /= 100; + curr -= 2; + unsafe { + ptr::copy_nonoverlapping( + lut_ptr.offset(d1 as isize), + buf_ptr.offset(curr), + 2, + ); + } + + // decode last 1 or 2 chars + if n < 10 { + curr -= 1; + unsafe { + *buf_ptr.offset(curr) = (n as u8) + b'0'; + } + } else { + let d1 = n << 1; + curr -= 2; + unsafe { + ptr::copy_nonoverlapping( + lut_ptr.offset(d1 as isize), + buf_ptr.offset(curr), + 2, + ); + } + } + + bytes.put_slice(&buf); + if four { + bytes.put_u8(b' '); + } + } +} diff --git a/actix-http/src/helpers.rs b/actix-http/src/helpers.rs index 86f8250b6..ff647e72b 100644 --- a/actix-http/src/helpers.rs +++ b/actix-http/src/helpers.rs @@ -1,69 +1,34 @@ -use std::{io, ptr}; +use std::io; use bytes::{BufMut, BytesMut}; use http::Version; use crate::extensions::Extensions; -const DEC_DIGITS_LUT: &[u8] = b"0001020304050607080910111213141516171819\ - 2021222324252627282930313233343536373839\ - 4041424344454647484950515253545556575859\ - 6061626364656667686970717273747576777879\ - 8081828384858687888990919293949596979899"; - -pub(crate) const STATUS_LINE_BUF_SIZE: usize = 13; - -pub(crate) fn write_status_line(version: Version, mut n: u16, bytes: &mut BytesMut) { - let mut buf: [u8; STATUS_LINE_BUF_SIZE] = *b"HTTP/1.1 "; - match version { - Version::HTTP_2 => buf[5] = b'2', - Version::HTTP_10 => buf[7] = b'0', - Version::HTTP_09 => { - buf[5] = b'0'; - buf[7] = b'9'; - } - _ => (), - } - - let mut curr: isize = 12; - let buf_ptr = buf.as_mut_ptr(); - let lut_ptr = DEC_DIGITS_LUT.as_ptr(); - let four = n > 999; - - // decode 2 more chars, if > 2 chars - let d1 = (n % 100) << 1; - n /= 100; - curr -= 2; - unsafe { - ptr::copy_nonoverlapping(lut_ptr.offset(d1 as isize), buf_ptr.offset(curr), 2); - } - - // decode last 1 or 2 chars - if n < 10 { - curr -= 1; - unsafe { - *buf_ptr.offset(curr) = (n as u8) + b'0'; - } - } else { - let d1 = n << 1; - curr -= 2; - unsafe { - ptr::copy_nonoverlapping( - lut_ptr.offset(d1 as isize), - buf_ptr.offset(curr), - 2, - ); - } - } - - bytes.put_slice(&buf); - if four { - bytes.put_u8(b' '); - } -} - const DIGITS_START: u8 = b'0'; +pub(crate) fn write_status_line(version: Version, n: u16, bytes: &mut BytesMut) { + match version { + Version::HTTP_11 => bytes.put_slice(b"HTTP/1.1 "), + Version::HTTP_10 => bytes.put_slice(b"HTTP/1.0 "), + Version::HTTP_09 => bytes.put_slice(b"HTTP/0.9 "), + _ => { + // other HTTP version handlers do not use this method + } + } + + let d100 = (n / 100) as u8; + let d10 = ((n / 10) % 10) as u8; + let d1 = (n % 10) as u8; + + bytes.put_u8(DIGITS_START + d100); + bytes.put_u8(DIGITS_START + d10); + bytes.put_u8(DIGITS_START + d1); + + // trailing space before reason + bytes.put_u8(b' '); +} + /// NOTE: bytes object has to contain enough space pub fn write_content_length(n: usize, bytes: &mut BytesMut) { bytes.put_slice(b"\r\ncontent-length: "); @@ -189,8 +154,28 @@ impl DataFactory for Data { #[cfg(test)] mod tests { + use std::str::from_utf8; + use super::*; + #[test] + fn test_status_line() { + let mut bytes = BytesMut::new(); + bytes.reserve(50); + write_status_line(Version::HTTP_11, 200, &mut bytes); + assert_eq!(from_utf8(&bytes.split().freeze()).unwrap(), "HTTP/1.1 200 "); + + let mut bytes = BytesMut::new(); + bytes.reserve(50); + write_status_line(Version::HTTP_09, 404, &mut bytes); + assert_eq!(from_utf8(&bytes.split().freeze()).unwrap(), "HTTP/0.9 404 "); + + let mut bytes = BytesMut::new(); + bytes.reserve(50); + write_status_line(Version::HTTP_09, 515, &mut bytes); + assert_eq!(from_utf8(&bytes.split().freeze()).unwrap(), "HTTP/0.9 515 "); + } + #[test] fn test_write_content_length() { let mut bytes = BytesMut::new(); From b521e9b2218a23b3c2e288611ec123042b287053 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 3 May 2020 14:33:29 +0100 Subject: [PATCH 7/7] conditional test compilation [range, charset] (#1483) * conditionally compile range and charset tests * remove deprecated try macros Co-authored-by: Yuki Okushi --- actix-http/src/extensions.rs | 2 + actix-http/src/header/common/range.rs | 302 +++++++++++------------- actix-http/src/header/shared/charset.rs | 29 ++- src/config.rs | 1 + 4 files changed, 159 insertions(+), 175 deletions(-) diff --git a/actix-http/src/extensions.rs b/actix-http/src/extensions.rs index 6a4a034a4..4e3918537 100644 --- a/actix-http/src/extensions.rs +++ b/actix-http/src/extensions.rs @@ -6,6 +6,8 @@ use fxhash::FxHashMap; #[derive(Default)] /// A type map of request extensions. pub struct Extensions { + /// Use FxHasher with a std HashMap with for faster + /// lookups on the small `TypeId` (u64 equivalent) keys. map: FxHashMap>, } diff --git a/actix-http/src/header/common/range.rs b/actix-http/src/header/common/range.rs index fc1bc8159..f9e203bb2 100644 --- a/actix-http/src/header/common/range.rs +++ b/actix-http/src/header/common/range.rs @@ -183,13 +183,13 @@ impl fmt::Display for Range { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { Range::Bytes(ref ranges) => { - try!(write!(f, "bytes=")); + write!(f, "bytes=")?; for (i, range) in ranges.iter().enumerate() { if i != 0 { - try!(f.write_str(",")); + f.write_str(",")?; } - try!(Display::fmt(range, f)); + Display::fmt(range, f)?; } Ok(()) } @@ -214,9 +214,9 @@ impl FromStr for Range { } Ok(Range::Bytes(ranges)) } - (Some(unit), Some(range_str)) if unit != "" && range_str != "" => Ok( - Range::Unregistered(unit.to_owned(), range_str.to_owned()), - ), + (Some(unit), Some(range_str)) if unit != "" && range_str != "" => { + Ok(Range::Unregistered(unit.to_owned(), range_str.to_owned())) + } _ => Err(::Error::Header), } } @@ -229,7 +229,8 @@ impl FromStr for ByteRangeSpec { let mut parts = s.splitn(2, '-'); match (parts.next(), parts.next()) { - (Some(""), Some(end)) => end.parse() + (Some(""), Some(end)) => end + .parse() .or(Err(::Error::Header)) .map(ByteRangeSpec::Last), (Some(start), Some("")) => start @@ -272,163 +273,138 @@ impl Header for Range { } } -#[test] -fn test_parse_bytes_range_valid() { - let r: Range = Header::parse_header(&"bytes=1-100".into()).unwrap(); - let r2: Range = Header::parse_header(&"bytes=1-100,-".into()).unwrap(); - let r3 = Range::bytes(1, 100); - assert_eq!(r, r2); - assert_eq!(r2, r3); +#[cfg(test)] +mod tests { + use super::*; - let r: Range = Header::parse_header(&"bytes=1-100,200-".into()).unwrap(); - let r2: Range = - Header::parse_header(&"bytes= 1-100 , 101-xxx, 200- ".into()).unwrap(); - let r3 = Range::Bytes(vec![ - ByteRangeSpec::FromTo(1, 100), - ByteRangeSpec::AllFrom(200), - ]); - assert_eq!(r, r2); - assert_eq!(r2, r3); + #[test] + fn test_parse_bytes_range_valid() { + let r: Range = Header::parse_header(&"bytes=1-100".into()).unwrap(); + let r2: Range = Header::parse_header(&"bytes=1-100,-".into()).unwrap(); + let r3 = Range::bytes(1, 100); + assert_eq!(r, r2); + assert_eq!(r2, r3); - let r: Range = Header::parse_header(&"bytes=1-100,-100".into()).unwrap(); - let r2: Range = Header::parse_header(&"bytes=1-100, ,,-100".into()).unwrap(); - let r3 = Range::Bytes(vec![ - ByteRangeSpec::FromTo(1, 100), - ByteRangeSpec::Last(100), - ]); - assert_eq!(r, r2); - assert_eq!(r2, r3); + let r: Range = Header::parse_header(&"bytes=1-100,200-".into()).unwrap(); + let r2: Range = + Header::parse_header(&"bytes= 1-100 , 101-xxx, 200- ".into()).unwrap(); + let r3 = Range::Bytes(vec![ + ByteRangeSpec::FromTo(1, 100), + ByteRangeSpec::AllFrom(200), + ]); + assert_eq!(r, r2); + assert_eq!(r2, r3); - let r: Range = Header::parse_header(&"custom=1-100,-100".into()).unwrap(); - let r2 = Range::Unregistered("custom".to_owned(), "1-100,-100".to_owned()); - assert_eq!(r, r2); -} - -#[test] -fn test_parse_unregistered_range_valid() { - let r: Range = Header::parse_header(&"custom=1-100,-100".into()).unwrap(); - let r2 = Range::Unregistered("custom".to_owned(), "1-100,-100".to_owned()); - assert_eq!(r, r2); - - let r: Range = Header::parse_header(&"custom=abcd".into()).unwrap(); - let r2 = Range::Unregistered("custom".to_owned(), "abcd".to_owned()); - assert_eq!(r, r2); - - let r: Range = Header::parse_header(&"custom=xxx-yyy".into()).unwrap(); - let r2 = Range::Unregistered("custom".to_owned(), "xxx-yyy".to_owned()); - assert_eq!(r, r2); -} - -#[test] -fn test_parse_invalid() { - let r: ::Result = Header::parse_header(&"bytes=1-a,-".into()); - assert_eq!(r.ok(), None); - - let r: ::Result = Header::parse_header(&"bytes=1-2-3".into()); - assert_eq!(r.ok(), None); - - let r: ::Result = Header::parse_header(&"abc".into()); - assert_eq!(r.ok(), None); - - let r: ::Result = Header::parse_header(&"bytes=1-100=".into()); - assert_eq!(r.ok(), None); - - let r: ::Result = Header::parse_header(&"bytes=".into()); - assert_eq!(r.ok(), None); - - let r: ::Result = Header::parse_header(&"custom=".into()); - assert_eq!(r.ok(), None); - - let r: ::Result = Header::parse_header(&"=1-100".into()); - assert_eq!(r.ok(), None); -} - -#[test] -fn test_fmt() { - use header::Headers; - - let mut headers = Headers::new(); - - headers.set(Range::Bytes(vec![ - ByteRangeSpec::FromTo(0, 1000), - ByteRangeSpec::AllFrom(2000), - ])); - assert_eq!(&headers.to_string(), "Range: bytes=0-1000,2000-\r\n"); - - headers.clear(); - headers.set(Range::Bytes(vec![])); - - assert_eq!(&headers.to_string(), "Range: bytes=\r\n"); - - headers.clear(); - headers.set(Range::Unregistered( - "custom".to_owned(), - "1-xxx".to_owned(), - )); - - assert_eq!(&headers.to_string(), "Range: custom=1-xxx\r\n"); -} - -#[test] -fn test_byte_range_spec_to_satisfiable_range() { - assert_eq!( - Some((0, 0)), - ByteRangeSpec::FromTo(0, 0).to_satisfiable_range(3) - ); - assert_eq!( - Some((1, 2)), - ByteRangeSpec::FromTo(1, 2).to_satisfiable_range(3) - ); - assert_eq!( - Some((1, 2)), - ByteRangeSpec::FromTo(1, 5).to_satisfiable_range(3) - ); - assert_eq!( - None, - ByteRangeSpec::FromTo(3, 3).to_satisfiable_range(3) - ); - assert_eq!( - None, - ByteRangeSpec::FromTo(2, 1).to_satisfiable_range(3) - ); - assert_eq!( - None, - ByteRangeSpec::FromTo(0, 0).to_satisfiable_range(0) - ); - - assert_eq!( - Some((0, 2)), - ByteRangeSpec::AllFrom(0).to_satisfiable_range(3) - ); - assert_eq!( - Some((2, 2)), - ByteRangeSpec::AllFrom(2).to_satisfiable_range(3) - ); - assert_eq!( - None, - ByteRangeSpec::AllFrom(3).to_satisfiable_range(3) - ); - assert_eq!( - None, - ByteRangeSpec::AllFrom(5).to_satisfiable_range(3) - ); - assert_eq!( - None, - ByteRangeSpec::AllFrom(0).to_satisfiable_range(0) - ); - - assert_eq!( - Some((1, 2)), - ByteRangeSpec::Last(2).to_satisfiable_range(3) - ); - assert_eq!( - Some((2, 2)), - ByteRangeSpec::Last(1).to_satisfiable_range(3) - ); - assert_eq!( - Some((0, 2)), - ByteRangeSpec::Last(5).to_satisfiable_range(3) - ); - assert_eq!(None, ByteRangeSpec::Last(0).to_satisfiable_range(3)); - assert_eq!(None, ByteRangeSpec::Last(2).to_satisfiable_range(0)); + let r: Range = Header::parse_header(&"bytes=1-100,-100".into()).unwrap(); + let r2: Range = Header::parse_header(&"bytes=1-100, ,,-100".into()).unwrap(); + let r3 = Range::Bytes(vec![ + ByteRangeSpec::FromTo(1, 100), + ByteRangeSpec::Last(100), + ]); + assert_eq!(r, r2); + assert_eq!(r2, r3); + + let r: Range = Header::parse_header(&"custom=1-100,-100".into()).unwrap(); + let r2 = Range::Unregistered("custom".to_owned(), "1-100,-100".to_owned()); + assert_eq!(r, r2); + } + + #[test] + fn test_parse_unregistered_range_valid() { + let r: Range = Header::parse_header(&"custom=1-100,-100".into()).unwrap(); + let r2 = Range::Unregistered("custom".to_owned(), "1-100,-100".to_owned()); + assert_eq!(r, r2); + + let r: Range = Header::parse_header(&"custom=abcd".into()).unwrap(); + let r2 = Range::Unregistered("custom".to_owned(), "abcd".to_owned()); + assert_eq!(r, r2); + + let r: Range = Header::parse_header(&"custom=xxx-yyy".into()).unwrap(); + let r2 = Range::Unregistered("custom".to_owned(), "xxx-yyy".to_owned()); + assert_eq!(r, r2); + } + + #[test] + fn test_parse_invalid() { + let r: ::Result = Header::parse_header(&"bytes=1-a,-".into()); + assert_eq!(r.ok(), None); + + let r: ::Result = Header::parse_header(&"bytes=1-2-3".into()); + assert_eq!(r.ok(), None); + + let r: ::Result = Header::parse_header(&"abc".into()); + assert_eq!(r.ok(), None); + + let r: ::Result = Header::parse_header(&"bytes=1-100=".into()); + assert_eq!(r.ok(), None); + + let r: ::Result = Header::parse_header(&"bytes=".into()); + assert_eq!(r.ok(), None); + + let r: ::Result = Header::parse_header(&"custom=".into()); + assert_eq!(r.ok(), None); + + let r: ::Result = Header::parse_header(&"=1-100".into()); + assert_eq!(r.ok(), None); + } + + #[test] + fn test_fmt() { + use header::Headers; + + let mut headers = Headers::new(); + + headers.set(Range::Bytes(vec![ + ByteRangeSpec::FromTo(0, 1000), + ByteRangeSpec::AllFrom(2000), + ])); + assert_eq!(&headers.to_string(), "Range: bytes=0-1000,2000-\r\n"); + + headers.clear(); + headers.set(Range::Bytes(vec![])); + + assert_eq!(&headers.to_string(), "Range: bytes=\r\n"); + + headers.clear(); + headers.set(Range::Unregistered("custom".to_owned(), "1-xxx".to_owned())); + + assert_eq!(&headers.to_string(), "Range: custom=1-xxx\r\n"); + } + + #[test] + fn test_byte_range_spec_to_satisfiable_range() { + assert_eq!( + Some((0, 0)), + ByteRangeSpec::FromTo(0, 0).to_satisfiable_range(3) + ); + assert_eq!( + Some((1, 2)), + ByteRangeSpec::FromTo(1, 2).to_satisfiable_range(3) + ); + assert_eq!( + Some((1, 2)), + ByteRangeSpec::FromTo(1, 5).to_satisfiable_range(3) + ); + assert_eq!(None, ByteRangeSpec::FromTo(3, 3).to_satisfiable_range(3)); + assert_eq!(None, ByteRangeSpec::FromTo(2, 1).to_satisfiable_range(3)); + assert_eq!(None, ByteRangeSpec::FromTo(0, 0).to_satisfiable_range(0)); + + assert_eq!( + Some((0, 2)), + ByteRangeSpec::AllFrom(0).to_satisfiable_range(3) + ); + assert_eq!( + Some((2, 2)), + ByteRangeSpec::AllFrom(2).to_satisfiable_range(3) + ); + assert_eq!(None, ByteRangeSpec::AllFrom(3).to_satisfiable_range(3)); + assert_eq!(None, ByteRangeSpec::AllFrom(5).to_satisfiable_range(3)); + assert_eq!(None, ByteRangeSpec::AllFrom(0).to_satisfiable_range(0)); + + assert_eq!(Some((1, 2)), ByteRangeSpec::Last(2).to_satisfiable_range(3)); + assert_eq!(Some((2, 2)), ByteRangeSpec::Last(1).to_satisfiable_range(3)); + assert_eq!(Some((0, 2)), ByteRangeSpec::Last(5).to_satisfiable_range(3)); + assert_eq!(None, ByteRangeSpec::Last(0).to_satisfiable_range(3)); + assert_eq!(None, ByteRangeSpec::Last(2).to_satisfiable_range(0)); + } } diff --git a/actix-http/src/header/shared/charset.rs b/actix-http/src/header/shared/charset.rs index 6ddfa03ea..00e7309d4 100644 --- a/actix-http/src/header/shared/charset.rs +++ b/actix-http/src/header/shared/charset.rs @@ -137,17 +137,22 @@ impl FromStr for Charset { } } -#[test] -fn test_parse() { - assert_eq!(Us_Ascii, "us-ascii".parse().unwrap()); - assert_eq!(Us_Ascii, "US-Ascii".parse().unwrap()); - assert_eq!(Us_Ascii, "US-ASCII".parse().unwrap()); - assert_eq!(Shift_Jis, "Shift-JIS".parse().unwrap()); - assert_eq!(Ext("ABCD".to_owned()), "abcd".parse().unwrap()); -} +#[cfg(test)] +mod tests { + use super::*; -#[test] -fn test_display() { - assert_eq!("US-ASCII", format!("{}", Us_Ascii)); - assert_eq!("ABCD", format!("{}", Ext("ABCD".to_owned()))); + #[test] + fn test_parse() { + assert_eq!(Us_Ascii, "us-ascii".parse().unwrap()); + assert_eq!(Us_Ascii, "US-Ascii".parse().unwrap()); + assert_eq!(Us_Ascii, "US-ASCII".parse().unwrap()); + assert_eq!(Shift_Jis, "Shift-JIS".parse().unwrap()); + assert_eq!(Ext("ABCD".to_owned()), "abcd".parse().unwrap()); + } + + #[test] + fn test_display() { + assert_eq!("US-ASCII", format!("{}", Us_Ascii)); + assert_eq!("ABCD", format!("{}", Ext("ABCD".to_owned()))); + } } diff --git a/src/config.rs b/src/config.rs index 6db378c7b..19a5ccc7b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -123,6 +123,7 @@ impl AppService { } } +/// Application connection config #[derive(Clone)] pub struct AppConfig(Rc);