Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to parse body? #52

Open
rapodaca opened this issue Sep 8, 2021 · 7 comments
Open

How to parse body? #52

rapodaca opened this issue Sep 8, 2021 · 7 comments

Comments

@rapodaca
Copy link

rapodaca commented Sep 8, 2021

I can get serde_qs to parse query parameters with Actix-web using QsQuery. However, I can't seem to get form data encoded as x-www-form-urlencoded data to parse.

What's the recommended way to parse a body form, which may contain nested arrays using square bracket notation, using serde_qs?

@samscott89
Copy link
Owner

Hey @rapodaca!

We don't have any official support for it. I took a look through the code for actix's built in Form support: https://docs.rs/actix-web/3.3.2/actix_web/web/struct.Form.html and it looks pretty non-trivial to replicate + maintain. I'm not sure if there are specific reasons it was implemented that way. In comparison, the query one was pretty easy.

I'd say the best way to handle it would be to extract the payload as bytes: https://docs.rs/actix-web/3.3.2/actix_web/web/struct.Payload.html and then use serde_qs::from_bytes: https://docs.rs/serde_qs/0.8.4/serde_qs/fn.from_bytes.html

If you'd be open to it, I would accept a PR that implements that as a new helper as part of the library?

@rapodaca
Copy link
Author

rapodaca commented Sep 9, 2021

I took a look through the code for actix's built in Form support: https://docs.rs/actix-web/3.3.2/actix_web/web/struct.Form.html and it looks pretty non-trivial to replicate + maintain.

You're not kidding. I think I've got a handle on it, though.

Here's something that gets to the point of at least reading the body bytes from the Payload in an extractor (needs some cleanup):

use actix_web::{get, web, App, HttpServer, Responder};
use std::pin::Pin;
use futures::Future;
use futures_core::{future::LocalBoxFuture, ready};
use actix_web::{dev, Error, HttpRequest, FromRequest};
use serde::{ de::DeserializeOwned, Deserialize };
use futures_util::{ StreamExt, future::FutureExt};
use std::task::{ Context, Poll };



#[derive(Debug)]
pub struct Reader {

}

pub struct FormExtractFut {
    fut: UrlEncoded
}

impl Future for FormExtractFut {
    type Output = Result<Reader, ()>;

    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        println!("polling FormExtractFut");

        let this = self.get_mut();
        let res = ready!(Pin::new(&mut this.fut).poll(cx));
        let res = match res {
            Err(err) => (),
            Ok(item) => ()//Ok(Reader { })
        };
        // Poll::Ready(Ok(Reader { }))
        Poll::Ready(Ok(Reader { }))
    }
}

pub struct UrlEncoded {
    stream: Option<dev::Payload>,
    fut: Option<LocalBoxFuture<'static, Result<Reader, ()>>>
}

impl UrlEncoded {
    pub fn new(req: &HttpRequest, payload: &mut dev::Payload) -> Self {
        let payload = payload.take();

        UrlEncoded {
            stream: Some(payload),
            fut: None
        }
    }
}

impl Future for UrlEncoded {
    type Output = Result<Reader, ()>;

    fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        if let Some(ref mut fut) = self.fut {
            return Pin::new(fut).poll(cx);
        }

        let mut stream = self.stream.take().unwrap();
        
        self.fut = Some(
            async move {
                let mut bytes = web::BytesMut::new();

                while let Some(item) = stream.next().await {
                    bytes.extend_from_slice(&item.unwrap());
                }

                println!("** FOUND BODY ** {:?}", bytes);

                Ok(Reader { })
            }.boxed_local(),
        );

        self.poll(cx)
    }
}

impl FromRequest for Reader {
    type Error = ();
    type Config = ();
    type Future = FormExtractFut;

    fn from_request(req: &HttpRequest, payload: &mut dev::Payload) -> Self::Future {
        FormExtractFut {
            fut: UrlEncoded::new(req, payload)
        }
    }
}

#[get("/test/")]
async fn index(reader: Reader) -> impl Responder {
    format!("{} {:?}", "hello", reader)
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    HttpServer::new(|| {
        App::new()
            .service(index)
    })
    .bind("127.0.0.1:8080")?
    .run()
    .await
}

The business end is the poll method on UrlEncoded. What remains is to return an Actual Form<T> instead of the dummy Reader type.

@rapodaca
Copy link
Author

I've got it all more or less working:

use actix_web::{dev::Payload, Error as ActixError, FromRequest, HttpRequest};
use actix_web::{get, web, App, HttpServer, Responder};
use futures_core::future::LocalBoxFuture;
use futures_util::{future::FutureExt, StreamExt};
use serde::de::DeserializeOwned;
use serde_qs::{ Config as QsConfig, Error as QsError };
use std::fmt::Debug;
use std::ops::{Deref, DerefMut};
use std::sync::Arc;

#[derive(Debug)]
pub struct QsForm<T>(T);

impl<T> Deref for QsForm<T> {
    type Target = T;

    fn deref(&self) -> &T {
        &self.0
    }
}

impl<T> DerefMut for QsForm<T> {
    fn deref_mut(&mut self) -> &mut T {
        &mut self.0
    }
}

pub struct QsQueryConfig {
    ehandler: Option<Arc<dyn Fn(QsError, &HttpRequest) -> ActixError + Send + Sync>>,
    qs_config: QsConfig,
}

impl QsQueryConfig {
    /// Set custom error handler
    pub fn error_handler<F>(mut self, f: F) -> Self
    where
        F: Fn(QsError, &HttpRequest) -> ActixError + Send + Sync + 'static,
    {
        self.ehandler = Some(Arc::new(f));
        self
    }

    /// Set custom serialization parameters
    pub fn qs_config(mut self, config: QsConfig) -> Self {
        self.qs_config = config;
        self
    }
}

impl Default for QsQueryConfig {
    fn default() -> Self {
        QsQueryConfig {
            ehandler: None,
            qs_config: QsConfig::default(),
        }
    }
}

// See:
// - https://github.com/actix/actix-web/blob/master/src/types/form.rs
// - https://github.com/samscott89/serde_qs/blob/main/src/actix.rs
impl<T> FromRequest for QsForm<T>
where
    T: DeserializeOwned,
{
    type Error = ActixError;
    type Config = ();
    type Future = LocalBoxFuture<'static, Result<Self, ActixError>>;

    fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future {
        let mut stream = payload.take();
        let req_clone = req.clone();
    
        async move {
            let mut bytes = web::BytesMut::new();

            while let Some(item) = stream.next().await {
                bytes.extend_from_slice(&item.unwrap());
            }

            let query_config = req_clone.app_data::<QsQueryConfig>();
            let default_qsconfig = QsConfig::default();
            let qsconfig = query_config
                .map(|c| &c.qs_config)
                .unwrap_or(&default_qsconfig);

            qsconfig
                .deserialize_bytes::<T>(&bytes)
                .map(|val| Ok(QsForm(val)))?
        }
        .boxed_local()
    }
}

// server

#[derive(serde::Deserialize, Debug)]
struct Thing {
    field: String,
}

#[get("/test/")]
async fn index(form: QsForm<Thing>) -> impl Responder {
    format!("Form: {:?}", form)
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    HttpServer::new(|| App::new().service(index))
        .bind("127.0.0.1:8080")?
        .run()
        .await
}

It does away with the convoluted Future handling in Actix's Form while combining the basic pattern used in Serde_qs's query parser. I ran into an issue with the 'static lifetime when trying to implement custom error handling. On the other hand, I'm not sure that's needed. Can't Actix use a middleware to catch SqForm errors?

@rapodaca
Copy link
Author

After some fiddling, I have something that mimics the error handling in serde_qs::actix::QsQuery:

use actix_web::{dev::Payload, Error as ActixError, FromRequest, HttpRequest};
use actix_web::{get, web, App, HttpServer, Responder};
use futures_core::future::LocalBoxFuture;
use futures_util::{future::FutureExt, StreamExt};
use serde::de::DeserializeOwned;
use serde_qs::{ Config as QsConfig, Error as QsError };
use std::fmt::Debug;
use std::ops::{Deref, DerefMut};
use std::sync::Arc;

#[derive(Debug)]
pub struct QsForm<T>(T);

impl<T> Deref for QsForm<T> {
    type Target = T;

    fn deref(&self) -> &T {
        &self.0
    }
}

impl<T> DerefMut for QsForm<T> {
    fn deref_mut(&mut self) -> &mut T {
        &mut self.0
    }
}

// private fields on QsQueryConfig prevent its reuse here, so a new struct
// is defined

pub struct QsFormConfig {
    ehandler: Option<Arc<dyn Fn(QsError, &HttpRequest) -> ActixError + Send + Sync>>,
    qs_config: QsConfig,
}

impl QsFormConfig {
    /// Set custom error handler
    pub fn error_handler<F>(mut self, f: F) -> Self
    where
        F: Fn(QsError, &HttpRequest) -> ActixError + Send + Sync + 'static,
    {
        self.ehandler = Some(Arc::new(f));
        self
    }

    /// Set custom serialization parameters
    pub fn qs_config(mut self, config: QsConfig) -> Self {
        self.qs_config = config;
        self
    }
}

impl Default for QsFormConfig {
    fn default() -> Self {
        QsFormConfig {
            qs_config: QsConfig::default(),
            ehandler: None,
        }
    }
}

// See:
// - https://github.com/actix/actix-web/blob/master/src/types/form.rs
// - https://github.com/samscott89/serde_qs/blob/main/src/actix.rs
impl<T> FromRequest for QsForm<T>
where
    T: DeserializeOwned + Debug,
{
    type Error = ActixError;
    type Config = ();
    type Future = LocalBoxFuture<'static, Result<Self, ActixError>>;

    fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future {
        let mut stream = payload.take();
        let req_clone = req.clone();

        async move {
            let mut bytes = web::BytesMut::new();
            
            while let Some(item) = stream.next().await {
                bytes.extend_from_slice(&item.unwrap());
            }
            
            let query_config = req_clone.app_data::<QsFormConfig>().clone();
            let error_handler = query_config.map(|c| c.ehandler.clone()).unwrap_or(None);

            let default_qsconfig = QsConfig::default();
            let qsconfig = query_config
                .map(|c| &c.qs_config)
                .unwrap_or(&default_qsconfig);

            qsconfig
                .deserialize_bytes::<T>(&bytes)
                .map(|val| Ok(QsForm(val)))
                .unwrap_or_else(|e| {
                    let e = if let Some(error_handler) = error_handler {
                        // e.into()
                        (error_handler)(e, &req_clone)
                    } else {
                        e.into()
                    };

                    Err(e)
                })
        }
        .boxed_local()
    }
}

// server

#[derive(serde::Deserialize, Debug)]
struct Account {
    name: String,
    users: Vec<User>
}

#[derive(serde::Deserialize, Debug)]
struct User {
    login: String
}

#[get("/test/")]
async fn index(req: HttpRequest, form: QsForm<Account>) -> impl Responder {
    format!("Form: {:?}", form)
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    HttpServer::new(|| App::new().service(index))
        .bind("127.0.0.1:8080")?
        .run()
        .await
}

It seems like QSQueryConfig should be reused here, but it can't be due to its private fields. Other than that this appears to work this form parser appears to work the same as the form parser.

I'm not sure how to package it to fit cleanly into serde_qs. I'm also not sure yet if this does everything you'd expect. For example, validation errors should be accessible so that a form could display them back to the user.

@samscott89
Copy link
Owner

Amazing! Thanks for doing that @rapodaca! Do you want to open a PR with this code and we can discuss it there?

@nMessage
Copy link
Contributor

nMessage commented Dec 5, 2023

Hey Is it okay if I make this PR request? I didn't write the code but i think it'd be useful to be incorporated into this library

@rapodaca
Copy link
Author

rapodaca commented Dec 5, 2023

Please feel free to create PRs and merge the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants