rwf2 / Rocket

A web framework for Rust.
https://rocket.rs
Other
24.2k stars 1.55k forks source link

Support for FromJsonValue or a common Guard trait (For both JSON and Form) #1045

Closed Muqito closed 4 years ago

Muqito commented 5 years ago

Hello

This is my first post and I hope I am doing this right:

I would like to see a FromJsonValue

Currently if you're using say:

struct User {
 age: UserAge
}

You can have a FromFormValue implemented on UserAge if you are requesting Form<User>

My current workaround to have it in my DataGuard after parsing the JSON:

<UserAge as FromFormValue>::from_form_value(RawStr::from_str(&result.age.0))

I think it would be nice to validate the same data with a common trait with a validate method (for both JSON and Form (x-www-form-urlencoded data))

Maybe something like this: (sorry if this is incorrect)

impl<'v> ValidateValue<'v> for UserAge {
    type Error = &'v RawStr;

    fn validate(value: &'v RawStr) -> Result<UserAge, Self::Error> {
        // Validate data
    }
}

or

impl<'v> ValidateValue<'v> for UserAge {
    type Error = &'v str;

    fn validate_string(value: &'v str) -> Result<UserAge, Self::Error> {
        // Validate data
    }
}

But maybe this is a dumb idea.

Thanks in advance! Sincerely, Christoffer

jebrosen commented 5 years ago

It sounds like what you are asking for is for Json<User> to work (Json from rocket_contrib). This can be accomplished by implementing Serialize from the serde crate for UserAge, and #[derive(Serialize)] for User.

Muqito commented 5 years ago

Hello @jebrosen

I might be doing something wrong. But see here:

https://imgur.com/a/5juRpC9

for the following code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a8b4890f7560d9fd8105f41826aed9e5

EDIT: Note that it's not validating the age. And if I post with age 5 it's still letting me through for the JSON Request.

jebrosen commented 5 years ago

Ah, so there are two different things going on here: supporting form or json at the same endpoint and also validation.

My first (and opinionated) recommendation is to do this kind of validation at a stage after FromDataValue or Serialize - maybe with the validator crate or other guards, or right in the route depending on how many kinds of validation you are dealing with. I see nothing intrinsically 'greater than 21' about an age by itself - only when validated as part of a form.

As for having both Json and Form submission at the same endpoint, you could continue defining two routes as you are now. To validate and process the same way between both routes, they could delegate the real work to a third function fn validate_user(user: User) -> String.

Alternatively, you could create a new FormOrJson type and implement FromData for that, calling either Form::from_data or Json::from_data depending on the Content-Type of the request. I feel like that has been done before, but I can't seem to find the code on the issue tracker.

Muqito commented 5 years ago

This is pretty much what I am currently doing and it's working. However it would be neat if they both had this built in.

Thank you for your replies :)

jebrosen commented 5 years ago

I offered multiple suggestions. Which of them are you currently doing and which were you suggesting could be built in?

Muqito commented 5 years ago

Oh sorry if I closed this issue. Not my intention

My current solution looks like this: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=7e6ebe7df45e9409ac6033c286004357

so I am basically "parsing it myself" inside the FromData, then validating.

Yeah I just took an age as an example. But let's say you want to filter out certain names, sure you could do those type of checks in the route. But as my take on the "request guards" function is to filter out stuff that's not allowed by either denying or forwarding the request.

I am not sure what you mean with your first option: "My first (and opinionated) recommendation is to do this kind of validation at a stage after FromDataValue or Serialize"

If you mean adding a FromDataValue, this would be appreciated. Then we could combine it with what you said in: "real work to a third function fn validate_user(user: User) -> String.". I can't really think of why you would have different validations on input form from a JSON string or Form data; but maybe there is a case.

Regarding this approach (if I am understanding it correctly)

"As for having both Json and Form submission at the same endpoint, you could continue defining two routes as you are now. To validate and process the same way between both routes, they could delegate the real work to a third function fn validate_user(user: User) -> String."

I don't want to have this constant pattern "if form or json" I am already doing it in my catchers in order to provide error messages depending on request type.

So I don't want to have that check as well in the route. Just like the discussion here: https://github.com/SergioBenitez/Rocket/issues/453

Sorry if I am having troubles making myself understood.

Sincerely, Christoffer

jebrosen commented 5 years ago

If you mean adding a FromDataValue, this would be appreciated

That was a typo, I meant FromFormValue. The equivalent for Json is Deserialize. I see you have derived Deserialize for UserAge, but if you implemented it manually you would be able to do the check. You would be duplicating code, but you wouldn't need that FromData impl anymore because you could use Json again.

Maybe some code will help explain what I mean better. I was addressing two different points with my ideas.


My first recommendation was to do something like this:

#[post("/new", format = "form", data = "<user>")]
fn validate_user_form(user: Form<User>) -> Result<String, Error> {
    validate_user(user.into_inner())
}
#[post("/new", format = "json", data = "<user>")]
fn validate_user_json(user: Json<User>) -> Result<String, Error> {
    validate_user(user.into_inner())
}

// split out to avoid duplicating the shared part of the code
fn validate_user(user: User) -> Result<String, Error> {

    // Could use a validation library for this instead
    if user.age < 21 {
        return Err(Error::TooYoung)
    }

    Ok(format!("Hello {}, your age is: {}", user.name, user.age.0))
}

The point here was to not do the data validation in FromFormValue or Deserialize: this way, you can more easily have different routes support different age ranges later, but they all accept the same User data type. It also avoids needing to duplicate the validation code in both FromFormValue and in Deserialize.


My second idea was something like this:

struct FormOrJson<T>(pub T);

impl<T> FromData for FormOrJson<T> where T: Deserialize + FromForm {
    fn from_data(request, data) -> Self {
        // pseudocode, not intended to be valid as is. Transform makes this idea
        // more complex to implement than what I wrote here.
        if request.content_type() == ContentType::JSON {
            Self(Json::from_data(request, data).into_inner())
        } else if request.content_type() == ContentType::Form {
            Self(Form::from_data(request, data).into_inner())
        }
    }
}

This still requires you to do the validation "before" (in FromFormValue and Deserialize) or "after" (in a data guard that wraps around this one, or in the route). But this would let you define one route function, instead of two.

Muqito commented 5 years ago

Thank you for taking the time :) I like the second approach a little more. But I'll see what I'll end up using.

Cheers Jeb

jebrosen commented 4 years ago

Closing due to inactivity. Feel free to reopen or leave further comments, though.

rien commented 4 years ago

@Muqito did the second approach work? I'm in the same situation and I'm interested to see if you got it working with the Transform.

jebrosen commented 4 years ago

I wrote a version of the session example that uses FormOrJson: https://paste.rs/qbm.rs . I didn't yet make any effort to make the code pretty or documented, but it works in the success cases.

ollyde commented 1 year ago

@jebrosen your link doesn't work anymore, it goes to a list of movie files :-P

I'm currently trying to automatically run validation on guard inputs but struggling, any ideas?

The example endpoint

#[derive(ToSchema, Clone, Deserialize, Validate)]
pub struct CreateUserInput {
    #[validate(max_length = 25)]
    #[validate(min_length = 2)]
    pub name: String,
    #[validate(minimum = 16)]
    #[validate(maximum = 200)]
    pub age: u8,
    pub job_title: Option<String>,
}

#[post("/create-user", data = "<data>")]
pub fn create_user(data: Json<CreateUserInput>) -> Result<Json<User>, ApiError> {
    // This works, but needs to be inside a guard ...
    validate_data(&data)?;

    let user = User {
        name: data.name.clone(),
        age: data.age,
        job_title: data.job_title.clone(),
        jwt: String::from("utoipa-rocks"),
    };

    return Ok(user.to_api_response());
}

Validate Function

pub fn validate_data<T: Validate>(json: &Json<T>) -> Result<(), ApiError> {
    json.validate().map_err(|err| validation_error(err))?;
    return Ok(());
}

And finally the broken request guard..

pub struct ValidationGuard<T: Validate> {
    pub data: T,
}

#[rocket::async_trait]
impl<'r, T: Validate> FromData<'r> for ValidationGuard<T> {
    type Error = ApiError;

    async fn from_data(req: &'r Request<'_>, data: Data<'r>) -> Outcome<'r, Self> {
        if req.content_type() == ContentType::JSON {
            Self(Json::from_data(req, data).into_inner())
        } else if req.content_type() == ContentType::Form {
            Self(Form::from_data(req, data).into_inner())
        }
    }
}

// Will not work..
// #[rocket::async_trait]
// impl<'r, T: Validate + Deserialize<'r>> FromRequest<'r> for ValidationGuard<T> {
//     type Error = ApiError;

//     async fn from_request(req: &'r Request<'_>) -> Outcome<Self, Self::Error> {
//         let json = match Json::<T>::deserialize(req) {
//             Ok(json) => json,
//             Err(err) => {
//                 return Outcome::Failure((Status::UnprocessableEntity, validation_error(err)))
//             }
//         };

//         match validate_data(&json) {
//             Ok(_) => Outcome::Success(ValidationGuard { data: json }),
//             Err(err) => Outcome::Failure((Status::UnprocessableEntity, err)),
//         }
//     }
// }