allow OPTIONS requests without request-method header (#226)

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

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 }
 

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<Method> {
+pub(crate) fn header_value_try_into_method(hdr: &HeaderValue) -> Option<Method> {
     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)

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<S> {
 }
 
 impl<S> CorsMiddleware<S> {
+    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<ServiceResponse<EitherBody<B>>, 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()
+            return 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()
         }
+
+        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()
     }
 }
 

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::{