Merge #124

124: Fix app_data access, which I broke in the Actix Web 4 upgrade. r=smarnach a=smarnach

Accessing application state changed in Actix Web 4, and I implemented the changes in #123 only incompletely. With this change, we always use the `Data` wrapper for the endpoint state, which fixes the problem.

Co-authored-by: Sven Marnach <sven@mozilla.com>

src/endpoints/classify.rs

@@ -1,5 +1,5 @@
 use crate::{endpoints::EndpointState, errors::ClassifyError, utils::RequestClientIp};
-use actix_web::{http, HttpRequest, HttpResponse};
+use actix_web::{http, web::Data, HttpRequest, HttpResponse};
 use chrono::{DateTime, Utc};
 use maxminddb::{self, geoip2};
 use serde::Serializer;
@@ -37,9 +37,11 @@ impl<'a> Default for ClientClassification<'a> {
     }
 }
 
-pub async fn classify_client(req: HttpRequest) -> Result<HttpResponse, ClassifyError> {
-    req.app_data::<EndpointState>()
-        .expect("Could not get app state")
+pub async fn classify_client(
+    req: HttpRequest,
+    state: Data<EndpointState>,
+) -> Result<HttpResponse, ClassifyError> {
+    state
         .geoip
         .locate(req.client_ip()?)
         .map(move |country| {
@@ -62,7 +64,8 @@ mod tests {
     use actix_web::{
         http,
         test::{self, TestRequest},
-        web, App,
+        web::{self, Data},
+        App,
     };
     use chrono::DateTime;
     use maxminddb::geoip2;
@@ -110,7 +113,7 @@ mod tests {
         };
         let service = test::init_service(
             App::new()
-                .app_data(state)
+                .app_data(Data::new(state))
                 .route("/", web::get().to(super::classify_client)),
         )
         .await;
@@ -140,7 +143,7 @@ mod tests {
     async fn test_classify_endpoint_has_correct_cache_headers() {
         let service = test::init_service(
             App::new()
-                .app_data(EndpointState {
+                .app_data(Data::new(EndpointState {
                     geoip: Arc::new(
                         GeoIp::builder()
                             .path("./GeoLite2-Country.mmdb")
@@ -148,7 +151,7 @@ mod tests {
                             .unwrap(),
                     ),
                     ..EndpointState::default()
-                })
+                }))
                 .route("/", web::get().to(super::classify_client)),
         )
         .await;

src/endpoints/debug.rs

@@ -1,5 +1,5 @@
 use crate::{endpoints::EndpointState, utils::RequestClientIp};
-use actix_web::{HttpRequest, HttpResponse};
+use actix_web::{web::Data, HttpRequest, HttpResponse};
 
 /// Show debugging information about the server comprising:
 ///
@@ -8,11 +8,11 @@ use actix_web::{HttpRequest, HttpResponse};
 ///  * Calculated client IP for the current request
 ///
 /// This handler should be disabled in production servers.
-pub async fn debug_handler(req: HttpRequest) -> HttpResponse {
+pub async fn debug_handler(req: HttpRequest, state: Data<EndpointState>) -> HttpResponse {
     HttpResponse::Ok().body(format!(
         "received headers: {:?}\n\nrequest state: {:?}\n\nclient ip: {:?}",
         req.headers(),
-        &req.app_data::<EndpointState>(),
+        state,
         req.client_ip()
     ))
 }

src/logging.rs

@@ -1,5 +1,6 @@
 use actix_web::{
     dev::{Service, ServiceRequest, ServiceResponse, Transform},
+    web::Data,
     Error, HttpRequest, HttpResponse,
 };
 use futures::{future, Future, FutureExt};
@@ -109,7 +110,7 @@ where
     actix_web::dev::forward_ready!(service);
 
     fn call(&self, req: ServiceRequest) -> Self::Future {
-        let log = match req.app_data::<EndpointState>() {
+        let log = match req.app_data::<Data<EndpointState>>() {
             Some(state) => state.log.clone(),
             None => return Box::pin(self.service.call(req)),
         };

src/metrics.rs

@@ -1,6 +1,7 @@
 use crate::{endpoints::EndpointState, errors::ClassifyError, APP_NAME};
 use actix_web::{
     dev::{Service, ServiceRequest, ServiceResponse, Transform},
+    web::Data,
     Error,
 };
 use cadence::{prelude::*, BufferedUdpMetricSink, StatsdClient};
@@ -82,7 +83,7 @@ where
     actix_web::dev::forward_ready!(service);
 
     fn call(&self, req: ServiceRequest) -> Self::Future {
-        let metrics = match req.app_data::<EndpointState>() {
+        let metrics = match req.app_data::<Data<EndpointState>>() {
             Some(state) => state.metrics.clone(),
             None => return Box::pin(self.service.call(req)),
         };
@@ -118,7 +119,8 @@ pub mod tests {
     use crate::endpoints::EndpointState;
     use actix_web::{
         test::{self, TestRequest},
-        web, App, HttpResponse,
+        web::{self, Data},
+        App, HttpResponse,
     };
     use cadence::StatsdClient;
     use regex::Regex;
@@ -153,7 +155,7 @@ pub mod tests {
         };
         let service = test::init_service(
             App::new()
-                .app_data(state)
+                .app_data(Data::new(state))
                 .wrap(ResponseTimer)
                 .route("/", web::get().to(HttpResponse::InternalServerError)),
         )
@@ -192,7 +194,7 @@ pub mod tests {
         };
         let service = test::init_service(
             App::new()
-                .app_data(state)
+                .app_data(Data::new(state))
                 .wrap(ResponseTimer)
                 .route("/", web::get().to(HttpResponse::InternalServerError)),
         )

src/utils.rs

@@ -1,5 +1,5 @@
 use crate::{endpoints::EndpointState, errors::ClassifyError};
-use actix_web::HttpRequest;
+use actix_web::{web::Data, HttpRequest};
 use std::net::IpAddr;
 
 pub trait RequestClientIp<S> {
@@ -20,7 +20,7 @@ pub trait RequestTraceIps<'a> {
 impl RequestClientIp<EndpointState> for HttpRequest {
     fn client_ip(&self) -> Result<IpAddr, ClassifyError> {
         let trusted_proxy_list = &self
-            .app_data::<EndpointState>()
+            .app_data::<Data<EndpointState>>()
             .expect("Expected app state")
             .trusted_proxies;
 
@@ -96,7 +96,7 @@ mod tests {
 
         let req = TestRequest::get()
             .insert_header(("x-forwarded-for", "1.2.3.4, 5.6.7.8"))
-            .app_data(state)
+            .app_data(Data::new(state))
             .to_http_request();
 
         assert_eq!(
@@ -118,7 +118,7 @@ mod tests {
 
         let req = TestRequest::get()
             .insert_header(("x-forwarded-for", "1.2.3.4, 5.6.7.8"))
-            .app_data(state)
+            .app_data(Data::new(state))
             .to_http_request();
 
         assert_eq!(
@@ -140,7 +140,7 @@ mod tests {
 
         let req = TestRequest::get()
             .insert_header(("x-forwarded-for", "1.2.3.4, 5.6.7.8"))
-            .app_data(state)
+            .app_data(Data::new(state))
             .to_http_request();
 
         assert!(