1
0
mirror of https://github.com/fafhrd91/actix-net synced 2025-02-20 07:40:33 +01:00

Fixed unsoundness in .and_then()/.then() service combinators

This commit is contained in:
Nikolay Kim 2020-01-16 16:58:11 -08:00
parent e7c2439543
commit dbfa13d6be
8 changed files with 164 additions and 116 deletions

View File

@ -1,5 +1,11 @@
# Changes
## [1.0.5] - 2020-01-16
### Fixed
* Fixed unsoundness in .and_then()/.then() service combinators
## [1.0.4] - 2020-01-15
### Fixed

View File

@ -1,6 +1,6 @@
[package]
name = "actix-service"
version = "1.0.4"
version = "1.0.5"
authors = ["Nikolay Kim <fafhrd91@gmail.com>"]
description = "Actix service"
keywords = ["network", "framework", "async", "futures"]

View File

@ -1,5 +1,6 @@
use std::future::Future;
use std::pin::Pin;
use std::rc::Rc;
use std::task::{Context, Poll};
use super::{Service, ServiceFactory};
@ -9,7 +10,7 @@ use crate::cell::Cell;
/// of another service which completes successfully.
///
/// This is created by the `ServiceExt::and_then` method.
pub struct AndThenService<A, B>(Cell<A>, Cell<B>);
pub(crate) struct AndThenService<A, B>(Cell<(A, B)>);
impl<A, B> AndThenService<A, B> {
/// Create new `AndThen` combinator
@ -18,13 +19,13 @@ impl<A, B> AndThenService<A, B> {
A: Service,
B: Service<Request = A::Response, Error = A::Error>,
{
Self(Cell::new(a), Cell::new(b))
Self(Cell::new((a, b)))
}
}
impl<A, B> Clone for AndThenService<A, B> {
fn clone(&self) -> Self {
AndThenService(self.0.clone(), self.1.clone())
AndThenService(self.0.clone())
}
}
@ -39,8 +40,9 @@ where
type Future = AndThenServiceResponse<A, B>;
fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
let not_ready = { !self.0.get_mut().poll_ready(cx)?.is_ready() };
if !self.1.poll_ready(cx)?.is_ready() || not_ready {
let srv = self.0.get_mut();
let not_ready = !srv.0.poll_ready(cx)?.is_ready();
if !srv.1.poll_ready(cx)?.is_ready() || not_ready {
Poll::Pending
} else {
Poll::Ready(Ok(()))
@ -49,13 +51,13 @@ where
fn call(&mut self, req: A::Request) -> Self::Future {
AndThenServiceResponse {
state: State::A(self.0.get_mut().call(req), Some(self.1.clone())),
state: State::A(self.0.get_mut().0.call(req), Some(self.0.clone())),
}
}
}
#[pin_project::pin_project]
pub struct AndThenServiceResponse<A, B>
pub(crate) struct AndThenServiceResponse<A, B>
where
A: Service,
B: Service<Request = A::Response, Error = A::Error>,
@ -70,7 +72,7 @@ where
A: Service,
B: Service<Request = A::Response, Error = A::Error>,
{
A(#[pin] A::Future, Option<Cell<B>>),
A(#[pin] A::Future, Option<Cell<(A, B)>>),
B(#[pin] B::Future),
Empty,
}
@ -92,7 +94,7 @@ where
Poll::Ready(res) => {
let mut b = b.take().unwrap();
this.state.set(State::Empty); // drop fut A
let fut = b.get_mut().call(res);
let fut = b.get_mut().1.call(res);
this.state.set(State::B(fut));
self.poll(cx)
}
@ -108,7 +110,7 @@ where
}
/// `.and_then()` service factory combinator
pub struct AndThenServiceFactory<A, B>
pub(crate) struct AndThenServiceFactory<A, B>
where
A: ServiceFactory,
A::Config: Clone,
@ -119,8 +121,7 @@ where
InitError = A::InitError,
>,
{
a: A,
b: B,
inner: Rc<(A, B)>,
}
impl<A, B> AndThenServiceFactory<A, B>
@ -136,7 +137,9 @@ where
{
/// Create new `AndThenFactory` combinator
pub(crate) fn new(a: A, b: B) -> Self {
Self { a, b }
Self {
inner: Rc::new((a, b)),
}
}
}
@ -161,34 +164,34 @@ where
type Future = AndThenServiceFactoryResponse<A, B>;
fn new_service(&self, cfg: A::Config) -> Self::Future {
let inner = &*self.inner;
AndThenServiceFactoryResponse::new(
self.a.new_service(cfg.clone()),
self.b.new_service(cfg),
inner.0.new_service(cfg.clone()),
inner.1.new_service(cfg),
)
}
}
impl<A, B> Clone for AndThenServiceFactory<A, B>
where
A: ServiceFactory + Clone,
A: ServiceFactory,
A::Config: Clone,
B: ServiceFactory<
Config = A::Config,
Request = A::Response,
Error = A::Error,
InitError = A::InitError,
> + Clone,
Config = A::Config,
Request = A::Response,
Error = A::Error,
InitError = A::InitError,
>,
{
fn clone(&self) -> Self {
Self {
a: self.a.clone(),
b: self.b.clone(),
inner: self.inner.clone(),
}
}
}
#[pin_project::pin_project]
pub struct AndThenServiceFactoryResponse<A, B>
pub(crate) struct AndThenServiceFactoryResponse<A, B>
where
A: ServiceFactory,
B: ServiceFactory<Request = A::Response>,

View File

@ -1,13 +1,14 @@
use std::future::Future;
use std::marker::PhantomData;
use std::pin::Pin;
use std::rc::Rc;
use std::task::{Context, Poll};
use crate::cell::Cell;
use crate::{Service, ServiceFactory};
/// `Apply` service combinator
pub struct AndThenApplyFn<A, B, F, Fut, Res, Err>
pub(crate) struct AndThenApplyFn<A, B, F, Fut, Res, Err>
where
A: Service,
B: Service,
@ -15,8 +16,7 @@ where
Fut: Future<Output = Result<Res, Err>>,
Err: From<A::Error> + From<B::Error>,
{
a: A,
b: Cell<(B, F)>,
srv: Cell<(A, B, F)>,
r: PhantomData<(Fut, Res, Err)>,
}
@ -31,8 +31,7 @@ where
/// Create new `Apply` combinator
pub(crate) fn new(a: A, b: B, f: F) -> Self {
Self {
a,
b: Cell::new((b, f)),
srv: Cell::new((a, b, f)),
r: PhantomData,
}
}
@ -40,7 +39,7 @@ where
impl<A, B, F, Fut, Res, Err> Clone for AndThenApplyFn<A, B, F, Fut, Res, Err>
where
A: Service + Clone,
A: Service,
B: Service,
F: FnMut(A::Response, &mut B) -> Fut,
Fut: Future<Output = Result<Res, Err>>,
@ -48,8 +47,7 @@ where
{
fn clone(&self) -> Self {
AndThenApplyFn {
a: self.a.clone(),
b: self.b.clone(),
srv: self.srv.clone(),
r: PhantomData,
}
}
@ -69,8 +67,9 @@ where
type Future = AndThenApplyFnFuture<A, B, F, Fut, Res, Err>;
fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
let not_ready = self.a.poll_ready(cx)?.is_pending();
if self.b.get_mut().0.poll_ready(cx)?.is_pending() || not_ready {
let inner = self.srv.get_mut();
let not_ready = inner.0.poll_ready(cx)?.is_pending();
if inner.1.poll_ready(cx)?.is_pending() || not_ready {
Poll::Pending
} else {
Poll::Ready(Ok(()))
@ -78,14 +77,15 @@ where
}
fn call(&mut self, req: A::Request) -> Self::Future {
let fut = self.srv.get_mut().0.call(req);
AndThenApplyFnFuture {
state: State::A(self.a.call(req), Some(self.b.clone())),
state: State::A(fut, Some(self.srv.clone())),
}
}
}
#[pin_project::pin_project]
pub struct AndThenApplyFnFuture<A, B, F, Fut, Res, Err>
pub(crate) struct AndThenApplyFnFuture<A, B, F, Fut, Res, Err>
where
A: Service,
B: Service,
@ -108,7 +108,7 @@ where
Err: From<A::Error>,
Err: From<B::Error>,
{
A(#[pin] A::Future, Option<Cell<(B, F)>>),
A(#[pin] A::Future, Option<Cell<(A, B, F)>>),
B(#[pin] Fut),
Empty,
}
@ -134,7 +134,7 @@ where
let mut b = b.take().unwrap();
this.state.set(State::Empty);
let b = b.get_mut();
let fut = (&mut b.1)(res, &mut b.0);
let fut = (&mut b.2)(res, &mut b.1);
this.state.set(State::B(fut));
self.poll(cx)
}
@ -150,10 +150,8 @@ where
}
/// `AndThenApplyFn` service factory
pub struct AndThenApplyFnFactory<A, B, F, Fut, Res, Err> {
a: A,
b: B,
f: F,
pub(crate) struct AndThenApplyFnFactory<A, B, F, Fut, Res, Err> {
srv: Rc<(A, B, F)>,
r: PhantomData<(Fut, Res, Err)>,
}
@ -168,25 +166,16 @@ where
/// Create new `ApplyNewService` new service instance
pub(crate) fn new(a: A, b: B, f: F) -> Self {
Self {
a: a,
b: b,
f: f,
srv: Rc::new((a, b, f)),
r: PhantomData,
}
}
}
impl<A, B, F, Fut, Res, Err> Clone for AndThenApplyFnFactory<A, B, F, Fut, Res, Err>
where
A: Clone,
B: Clone,
F: Clone,
{
impl<A, B, F, Fut, Res, Err> Clone for AndThenApplyFnFactory<A, B, F, Fut, Res, Err> {
fn clone(&self) -> Self {
Self {
a: self.a.clone(),
b: self.b.clone(),
f: self.f.clone(),
srv: self.srv.clone(),
r: PhantomData,
}
}
@ -210,18 +199,19 @@ where
type Future = AndThenApplyFnFactoryResponse<A, B, F, Fut, Res, Err>;
fn new_service(&self, cfg: A::Config) -> Self::Future {
let srv = &*self.srv;
AndThenApplyFnFactoryResponse {
a: None,
b: None,
f: self.f.clone(),
fut_a: self.a.new_service(cfg.clone()),
fut_b: self.b.new_service(cfg),
f: srv.2.clone(),
fut_a: srv.0.new_service(cfg.clone()),
fut_b: srv.1.new_service(cfg),
}
}
}
#[pin_project::pin_project]
pub struct AndThenApplyFnFactoryResponse<A, B, F, Fut, Res, Err>
pub(crate) struct AndThenApplyFnFactoryResponse<A, B, F, Fut, Res, Err>
where
A: ServiceFactory,
B: ServiceFactory<Config = A::Config, InitError = A::InitError>,
@ -267,8 +257,11 @@ where
if this.a.is_some() && this.b.is_some() {
Poll::Ready(Ok(AndThenApplyFn {
a: this.a.take().unwrap(),
b: Cell::new((this.b.take().unwrap(), this.f.clone())),
srv: Cell::new((
this.a.take().unwrap(),
this.b.take().unwrap(),
this.f.clone(),
)),
r: PhantomData,
}))
} else {

View File

@ -7,7 +7,18 @@ use crate::cell::Cell;
use crate::{Service, ServiceFactory};
/// Convert `Fn(Config, &mut Service1) -> Future<Service2>` fn to a service factory
pub fn apply_cfg<F, C, T, R, S, E>(srv: T, f: F) -> ApplyConfigService<F, C, T, R, S, E>
pub fn apply_cfg<F, C, T, R, S, E>(
srv: T,
f: F,
) -> impl ServiceFactory<
Config = C,
Request = S::Request,
Response = S::Response,
Error = S::Error,
Service = S,
InitError = E,
Future = R,
> + Clone
where
F: FnMut(C, &mut T) -> R,
T: Service,
@ -26,7 +37,14 @@ where
pub fn apply_cfg_factory<F, C, T, R, S>(
factory: T,
f: F,
) -> ApplyConfigServiceFactory<F, C, T, R, S>
) -> impl ServiceFactory<
Config = C,
Request = S::Request,
Response = S::Response,
Error = S::Error,
Service = S,
InitError = T::InitError,
> + Clone
where
F: FnMut(C, &mut T::Service) -> R,
T: ServiceFactory<Config = ()>,
@ -41,7 +59,7 @@ where
}
/// Convert `Fn(Config, &mut Server) -> Future<Service>` fn to NewService\
pub struct ApplyConfigService<F, C, T, R, S, E>
struct ApplyConfigService<F, C, T, R, S, E>
where
F: FnMut(C, &mut T) -> R,
T: Service,
@ -92,7 +110,7 @@ where
}
/// Convert `Fn(&Config) -> Future<Service>` fn to NewService
pub struct ApplyConfigServiceFactory<F, C, T, R, S>
struct ApplyConfigServiceFactory<F, C, T, R, S>
where
F: FnMut(C, &mut T::Service) -> R,
T: ServiceFactory<Config = ()>,
@ -145,7 +163,7 @@ where
}
#[pin_project::pin_project]
pub struct ApplyConfigServiceFactoryResponse<F, C, T, R, S>
struct ApplyConfigServiceFactoryResponse<F, C, T, R, S>
where
F: FnMut(C, &mut T::Service) -> R,
T: ServiceFactory<Config = ()>,

View File

@ -361,10 +361,7 @@ where
}
pub mod dev {
pub use crate::and_then::{AndThenService, AndThenServiceFactory};
pub use crate::and_then_apply_fn::{AndThenApplyFn, AndThenApplyFnFactory};
pub use crate::apply::{Apply, ApplyServiceFactory};
pub use crate::apply_cfg::{ApplyConfigService, ApplyConfigServiceFactory};
pub use crate::fn_service::{
FnService, FnServiceConfig, FnServiceFactory, FnServiceNoConfig,
};
@ -372,7 +369,6 @@ pub mod dev {
pub use crate::map_config::{MapConfig, UnitConfig};
pub use crate::map_err::{MapErr, MapErrServiceFactory};
pub use crate::map_init_err::MapInitErr;
pub use crate::then::{ThenService, ThenServiceFactory};
pub use crate::transform::ApplyTransform;
pub use crate::transform_err::TransformMapInitErr;
}

View File

@ -46,7 +46,12 @@ impl<T: Service> Pipeline<T> {
///
/// Note that this function consumes the receiving service and returns a
/// wrapped version of it.
pub fn and_then<F, U>(self, service: F) -> Pipeline<AndThenService<T, U>>
pub fn and_then<F, U>(
self,
service: F,
) -> Pipeline<
impl Service<Request = T::Request, Response = U::Response, Error = T::Error> + Clone,
>
where
Self: Sized,
F: IntoService<U>,
@ -65,7 +70,7 @@ impl<T: Service> Pipeline<T> {
self,
service: I,
f: F,
) -> Pipeline<AndThenApplyFn<T, U, F, Fut, Res, Err>>
) -> Pipeline<impl Service<Request = T::Request, Response = Res, Error = Err> + Clone>
where
Self: Sized,
I: IntoService<U>,
@ -84,7 +89,12 @@ impl<T: Service> Pipeline<T> {
///
/// Note that this function consumes the receiving pipeline and returns a
/// wrapped version of it.
pub fn then<F, U>(self, service: F) -> Pipeline<ThenService<T, U>>
pub fn then<F, U>(
self,
service: F,
) -> Pipeline<
impl Service<Request = T::Request, Response = U::Response, Error = T::Error> + Clone,
>
where
Self: Sized,
F: IntoService<U>,
@ -168,7 +178,23 @@ pub struct PipelineFactory<T> {
impl<T: ServiceFactory> PipelineFactory<T> {
/// Call another service after call to this one has resolved successfully.
pub fn and_then<F, U>(self, factory: F) -> PipelineFactory<AndThenServiceFactory<T, U>>
pub fn and_then<F, U>(
self,
factory: F,
) -> PipelineFactory<
impl ServiceFactory<
Request = T::Request,
Response = U::Response,
Error = T::Error,
Config = T::Config,
InitError = T::InitError,
Service = impl Service<
Request = T::Request,
Response = U::Response,
Error = T::Error,
> + Clone,
> + Clone,
>
where
Self: Sized,
T::Config: Clone,
@ -193,7 +219,16 @@ impl<T: ServiceFactory> PipelineFactory<T> {
self,
factory: I,
f: F,
) -> PipelineFactory<AndThenApplyFnFactory<T, U, F, Fut, Res, Err>>
) -> PipelineFactory<
impl ServiceFactory<
Request = T::Request,
Response = Res,
Error = Err,
Config = T::Config,
InitError = T::InitError,
Service = impl Service<Request = T::Request, Response = Res, Error = Err> + Clone,
> + Clone,
>
where
Self: Sized,
T::Config: Clone,
@ -214,7 +249,23 @@ impl<T: ServiceFactory> PipelineFactory<T> {
///
/// Note that this function consumes the receiving pipeline and returns a
/// wrapped version of it.
pub fn then<F, U>(self, factory: F) -> PipelineFactory<ThenServiceFactory<T, U>>
pub fn then<F, U>(
self,
factory: F,
) -> PipelineFactory<
impl ServiceFactory<
Request = T::Request,
Response = U::Response,
Error = T::Error,
Config = T::Config,
InitError = T::InitError,
Service = impl Service<
Request = T::Request,
Response = U::Response,
Error = T::Error,
> + Clone,
> + Clone,
>
where
Self: Sized,
T::Config: Clone,

View File

@ -1,5 +1,6 @@
use std::future::Future;
use std::pin::Pin;
use std::rc::Rc;
use std::task::{Context, Poll};
use super::{Service, ServiceFactory};
@ -8,11 +9,8 @@ use crate::cell::Cell;
/// Service for the `then` combinator, chaining a computation onto the end of
/// another service.
///
/// This is created by the `ServiceExt::then` method.
pub struct ThenService<A, B> {
a: A,
b: Cell<B>,
}
/// This is created by the `Pipeline::then` method.
pub(crate) struct ThenService<A, B>(Cell<(A, B)>);
impl<A, B> ThenService<A, B> {
/// Create new `.then()` combinator
@ -21,19 +19,13 @@ impl<A, B> ThenService<A, B> {
A: Service,
B: Service<Request = Result<A::Response, A::Error>, Error = A::Error>,
{
Self { a, b: Cell::new(b) }
Self(Cell::new((a, b)))
}
}
impl<A, B> Clone for ThenService<A, B>
where
A: Clone,
{
impl<A, B> Clone for ThenService<A, B> {
fn clone(&self) -> Self {
ThenService {
a: self.a.clone(),
b: self.b.clone(),
}
ThenService(self.0.clone())
}
}
@ -47,9 +39,10 @@ where
type Error = B::Error;
type Future = ThenServiceResponse<A, B>;
fn poll_ready(&mut self, ctx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
let not_ready = !self.a.poll_ready(ctx)?.is_ready();
if !self.b.get_mut().poll_ready(ctx)?.is_ready() || not_ready {
fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
let srv = self.0.get_mut();
let not_ready = !srv.0.poll_ready(cx)?.is_ready();
if !srv.1.poll_ready(cx)?.is_ready() || not_ready {
Poll::Pending
} else {
Poll::Ready(Ok(()))
@ -58,13 +51,13 @@ where
fn call(&mut self, req: A::Request) -> Self::Future {
ThenServiceResponse {
state: State::A(self.a.call(req), Some(self.b.clone())),
state: State::A(self.0.get_mut().0.call(req), Some(self.0.clone())),
}
}
}
#[pin_project::pin_project]
pub struct ThenServiceResponse<A, B>
pub(crate) struct ThenServiceResponse<A, B>
where
A: Service,
B: Service<Request = Result<A::Response, A::Error>>,
@ -79,7 +72,7 @@ where
A: Service,
B: Service<Request = Result<A::Response, A::Error>>,
{
A(#[pin] A::Future, Option<Cell<B>>),
A(#[pin] A::Future, Option<Cell<(A, B)>>),
B(#[pin] B::Future),
Empty,
}
@ -101,7 +94,7 @@ where
Poll::Ready(res) => {
let mut b = b.take().unwrap();
this.state.set(State::Empty); // drop fut A
let fut = b.get_mut().call(res);
let fut = b.get_mut().1.call(res);
this.state.set(State::B(fut));
self.poll(cx)
}
@ -117,10 +110,7 @@ where
}
/// `.then()` service factory combinator
pub struct ThenServiceFactory<A, B> {
a: A,
b: B,
}
pub(crate) struct ThenServiceFactory<A, B>(Rc<(A, B)>);
impl<A, B> ThenServiceFactory<A, B>
where
@ -135,7 +125,7 @@ where
{
/// Create new `AndThen` combinator
pub(crate) fn new(a: A, b: B) -> Self {
Self { a, b }
Self(Rc::new((a, b)))
}
}
@ -160,28 +150,19 @@ where
type Future = ThenServiceFactoryResponse<A, B>;
fn new_service(&self, cfg: A::Config) -> Self::Future {
ThenServiceFactoryResponse::new(
self.a.new_service(cfg.clone()),
self.b.new_service(cfg),
)
let srv = &*self.0;
ThenServiceFactoryResponse::new(srv.0.new_service(cfg.clone()), srv.1.new_service(cfg))
}
}
impl<A, B> Clone for ThenServiceFactory<A, B>
where
A: Clone,
B: Clone,
{
impl<A, B> Clone for ThenServiceFactory<A, B> {
fn clone(&self) -> Self {
Self {
a: self.a.clone(),
b: self.b.clone(),
}
Self(self.0.clone())
}
}
#[pin_project::pin_project]
pub struct ThenServiceFactoryResponse<A, B>
pub(crate) struct ThenServiceFactoryResponse<A, B>
where
A: ServiceFactory,
B: ServiceFactory<