From 6fbe2eab9426fc5bcca5a17f8bcaa41d27cab1d5 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 7 Mar 2022 15:32:07 +0000 Subject: [PATCH] allow OPTIONS requests without request-method header (#226) --- actix-cors/CHANGES.md | 3 ++ actix-cors/Cargo.toml | 1 - actix-cors/src/inner.rs | 6 +-- actix-cors/src/middleware.rs | 73 ++++++++++++++++++++++++------------ actix-cors/tests/tests.rs | 2 +- 5 files changed, 55 insertions(+), 30 deletions(-) diff --git a/actix-cors/CHANGES.md b/actix-cors/CHANGES.md index 0940959e5..e9c267050 100644 --- a/actix-cors/CHANGES.md +++ b/actix-cors/CHANGES.md @@ -1,6 +1,9 @@ # Changes ## Unreleased - 2021-xx-xx +- Do not consider requests without a `Access-Control-Request-Method` as preflight. [#226] + +[#226]: https://github.com/actix/actix-extras/pull/226 ## 0.6.0 - 2022-02-25 diff --git a/actix-cors/Cargo.toml b/actix-cors/Cargo.toml index f455e82a6..823821ffa 100644 --- a/actix-cors/Cargo.toml +++ b/actix-cors/Cargo.toml @@ -17,7 +17,6 @@ name = "actix_cors" path = "src/lib.rs" [dependencies] -actix-service = "2" actix-utils = "3" actix-web = { version = "4", default-features = false } diff --git a/actix-cors/src/inner.rs b/actix-cors/src/inner.rs index 92fca0e80..1aa0a1736 100644 --- a/actix-cors/src/inner.rs +++ b/actix-cors/src/inner.rs @@ -33,7 +33,7 @@ impl fmt::Debug for OriginFn { } /// Try to parse header value as HTTP method. -fn header_value_try_into_method(hdr: &HeaderValue) -> Option { +pub(crate) fn header_value_try_into_method(hdr: &HeaderValue) -> Option { hdr.to_str() .ok() .and_then(|meth| Method::try_from(meth).ok()) @@ -141,7 +141,7 @@ impl Inner { // method invalid Some(_) => Err(CorsError::BadRequestMethod), - // method missing + // method missing so this is not a preflight request None => Err(CorsError::MissingRequestMethod), } } @@ -277,7 +277,7 @@ mod test { assert!(cors.inner.validate_allowed_method(req.head()).is_err()); assert!(cors.inner.validate_allowed_headers(req.head()).is_err()); let resp = test::call_service(&cors, req).await; - assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + assert_eq!(resp.status(), StatusCode::OK); let req = TestRequest::default() .method(Method::OPTIONS) diff --git a/actix-cors/src/middleware.rs b/actix-cors/src/middleware.rs index f6030db9e..832e579c9 100644 --- a/actix-cors/src/middleware.rs +++ b/actix-cors/src/middleware.rs @@ -3,7 +3,7 @@ use std::{collections::HashSet, rc::Rc}; use actix_utils::future::ok; use actix_web::{ body::{EitherBody, MessageBody}, - dev::{Service, ServiceRequest, ServiceResponse}, + dev::{forward_ready, Service, ServiceRequest, ServiceResponse}, http::{ header::{self, HeaderValue}, Method, @@ -13,7 +13,11 @@ use actix_web::{ use futures_util::future::{FutureExt as _, LocalBoxFuture}; use log::debug; -use crate::{builder::intersperse_header_values, inner::add_vary_header, AllOrSome, Inner}; +use crate::{ + builder::intersperse_header_values, + inner::{add_vary_header, header_value_try_into_method}, + AllOrSome, Inner, +}; /// Service wrapper for Cross-Origin Resource Sharing support. /// @@ -27,6 +31,25 @@ pub struct CorsMiddleware { } impl CorsMiddleware { + fn is_request_preflight(req: &ServiceRequest) -> bool { + // check request method is OPTIONS + if req.method() != Method::OPTIONS { + return false; + } + + // check follow-up request method is present and valid + if req + .headers() + .get(header::ACCESS_CONTROL_REQUEST_METHOD) + .and_then(header_value_try_into_method) + .is_none() + { + return false; + } + + true + } + fn handle_preflight(inner: &Inner, req: ServiceRequest) -> ServiceResponse { if let Err(err) = inner .validate_origin(req.head()) @@ -136,34 +159,34 @@ where type Error = Error; type Future = LocalBoxFuture<'static, Result>, Error>>; - actix_service::forward_ready!(service); + forward_ready!(service); fn call(&self, req: ServiceRequest) -> Self::Future { - if self.inner.preflight && req.method() == Method::OPTIONS { + if self.inner.preflight && Self::is_request_preflight(&req) { let inner = Rc::clone(&self.inner); let res = Self::handle_preflight(&inner, req); - ok(res.map_into_right_body()).boxed_local() - } else { - let origin = req.headers().get(header::ORIGIN).cloned(); - - if origin.is_some() { - // Only check requests with a origin header. - if let Err(err) = self.inner.validate_origin(req.head()) { - debug!("origin validation failed; inner service is not called"); - return ok(req.error_response(err).map_into_right_body()).boxed_local(); - } - } - - let inner = Rc::clone(&self.inner); - let fut = self.service.call(req); - - async move { - let res = fut.await; - - Ok(Self::augment_response(&inner, res?).map_into_left_body()) - } - .boxed_local() + return ok(res.map_into_right_body()).boxed_local(); } + + let origin = req.headers().get(header::ORIGIN).cloned(); + + if origin.is_some() { + // Only check requests with a origin header. + if let Err(err) = self.inner.validate_origin(req.head()) { + debug!("origin validation failed; inner service is not called"); + return ok(req.error_response(err).map_into_right_body()).boxed_local(); + } + } + + let inner = Rc::clone(&self.inner); + let fut = self.service.call(req); + + async move { + let res = fut.await; + + Ok(Self::augment_response(&inner, res?).map_into_left_body()) + } + .boxed_local() } } diff --git a/actix-cors/tests/tests.rs b/actix-cors/tests/tests.rs index 8ff8b098e..9165e82e1 100644 --- a/actix-cors/tests/tests.rs +++ b/actix-cors/tests/tests.rs @@ -1,5 +1,5 @@ -use actix_service::fn_service; use actix_utils::future::ok; +use actix_web::dev::fn_service; use actix_web::{ dev::{ServiceRequest, Transform}, http::{