Rocket: Add support for non-strict FromForm decoding (ignore extra fields)

[naufraghi]

I was transitioning from a json api to a form encoded one.

The first thing I did was to replace JSON with FromForm in the route signature, but I discovered FromForm is strict and fails if the request has more field than the struct.

I found this a "surprise" element. In a real life example, developing a Slack Outgoing Webhook endpoint, Slack documents a set of fields, but in fact emits the service_id too.

[flosse]

How does your struct look like?

[naufraghi]

@flosse here it is:

#[derive(FromForm)]
struct SlackOutgoing {
    token: String,
    service_id: String,
    team_id: String,
    team_domain: String,
    channel_id: String,
    channel_name: String,
    timestamp: String,
    user_id: String,
    user_name: String,
    text: String,
    trigger_word: Option<String>,
}

I currently only need token, user_id and text. If I could I preferred parsing only those fields and be resistant to any API change not effecting the subset I'm using.

[flosse]

you could all optional fields ...well wrap into Option ;-)

#[derive(FromForm)]
struct SlackOutgoing {
    token: String,
    service_id:  Option<String>,
    team_id:  Option<String>,
    team_domain:  Option<String>,
    channel_id:  Option<String>,
    channel_name:  Option<String>,
    timestamp:  Option<String>,
    user_id: String,
    user_name:  Option<String>,
    text: String,
    trigger_word: Option<String>,
}

[naufraghi]

Yes, it can be an _option_ :)

There are a few drawbacks I can see:

• the route will fail if Slack will add a new form field I have no way to Option,
• I lose some 1-1 mapping with JSON, where an absent field and a missing field are different (syntactically at least, semantically it depends),
• I need to write a lot :)

[SergioBenitez]

Copying my comment from #247 to get some wider feedback:

I see three possibilities here.

1. The standard behavior could be to allow extra fields. This was actually the initial implementation.

2. We can allow an attribute to specify the behavior. This would look like:

   [derive(FromForm)]

   [form(lenient)]

   struct MyStruct { ... }

3. We can open an RFC with the Rust team to allow parameters on derives. Something like:

   [derive(FromForm(lenient))]

   struct MyStruct { ... }

[naufraghi]

I think 1. is better, it is similar to JSON behavior, and I think it cover the common use case to get only what you need from a request.

A strict behaviour can be useful too in some cases:

#[derive(FromForm)]
#[form(strict)]
struct MyStruct { ... }

[uniphil]

Option 1 is what I expected when I started, but any way to get lenient forms would help me sleep after setting up third-party APIs.

As another motivating example, stripe's Checkout posts stuff to your backend. Here is what they say they'll send:

https://stripe.com/docs/checkout#integration-options-shipping-address

And here's what my struct has to look like for Rocket to accept it:

#[derive(Debug, FromForm)]
#[allow(non_snake_case)]
struct StripeSubscribe {
    stripeToken: String,
    stripeTokenType: String,
    stripeEmail: String,
    stripeBillingName: String,
    stripeBillingAddressLine1: String,
    stripeBillingAddressZip: String,
    stripeBillingAddressState: String,
    stripeBillingAddressCity: String,
    stripeBillingAddressCountry: String,
    stripeBillingAddressCountryCode: String,
    stripeShippingName: String,
    stripeShippingAddressLine1: String,
    stripeShippingAddressZip: String,
    stripeShippingAddressState: String,
    stripeShippingAddressCity: String,
    stripeShippingAddressCountry: String,
    stripeShippingAddressCountryCode: String,
}

There are at least three extra fields stripe is sending me that they haven't documented. I won't be shocked to get more unexpected fields in the future. Eek!

Also I don't care about most of it and it's a lot of typing :)

[SergioBenitez]

I'm happy to announce that #[form(strict)] and/or #[form(lenient)] are making it into 0.3! 🎉

In 0.3, you'll be able to specify whether form parsing should be strict or lenient via a #[form(strict)] or #[form(lenient)] attribute, respectively. I haven't decided yet which should be the default, and I'd love to hear any input on this matter!

[naufraghi]

I vote for lenient by default and to opt in with #[form(strict)] to restore the current mode.

This is not a breaking change for my use case, the parsing error was unwanted so I had no test on it.

I can imagine some use cases one was expecting an error, but I think a better default is something:

• uniform with JSON
• doesn't explode if let alone (supports third party backward compatible changes)

[stephanbuys]

My internal alarm bells are going off and I am leaning the other way. Perhaps consider strict as the default to ensure no unexpected behaviour, web security is a big deal (could we open ourselves up for DOS attacks?) and who knows what unbounded parsing may result in? Having lenient by default doesn't feel like the rust way to do things either (if there is such a thing 😄), it seems unsafe, but that is only an opinion.

As an aside, parsing to serde_json::Value is quite convenient for those times you cannot be sure of the structure of the JSON object - but I'm sure you know that.

Just my 2c.

[naufraghi]

Fair enough, now I'm not so sure of the best default :)

strict default:

• pro safer (no time and bandwidth spent parsing a huge form to get 2 fields)
• pro backward compatible (not expected at 0.3, but yet)
• cons breaks if a field is added in a backward compatible way
• cons does not respect the principle of least astonishment, given JSON is lenient by default

Yet, to be protected against DoS, we can put a limit on the POST size and on the number of fields, so that if one need the lenient form, is somewhat protected the same (defaults and per form overrides?).

TL;DR, I'm still preferring the lenient default, but only if some form of limiting can be scheduled in a future version.

PS: in fact, a lenient option without limits, even if opt-in, is dangerous, but I think this is a problem related to all inbound streams, the hard thing is to find a good balance between safety and usability.

[SergioBenitez]

Forms are already imposed a strict limit which defaults to 32KiB but can be configured. As such, DOS attacks are not a concern in this decision.

[Roba1993]

I would vote for lenient as default. It's the same way json is handled and IMHO it's what the most developers are expecting. The amount of parameters can always change. This circumstance shouldn't end in an error, when I use the default...

[SergioBenitez]

I've put some thought into this. One thing I wonder is whether leniency should be a property of the structure at all. Instead, leniency can be a property of a structure's _use_. Let me explain the difference.

The proposal thus far is that we can take a structure which we write today as:

#[derive(FromForm)]
struct Input {
    a: String,
    b: usize
}

...and add an attribute to make the derived FromForm implementation strict or lenient, like so:

#[derive(FromForm)]
#[form(lenient)]
struct Input {
    a: String,
    b: usize
}

Once the structure has been declared strict or lenient, it can never change. Any parsing of a form into that structure will be strict or lenient, whatever was declared.

An alternative approach is to say that a structure can be parsed in a strict _or_ lenient fashion, and which is used depends on a runtime value. This would mean that the FromForm trait is changed to take an extra parameter that defines whether the parse should be strict or lenient:

pub trait FromForm<'f>: Sized {
    fn from_form(items: &mut FormItems<'f>, strict: bool) -> Result<Self, Self::Error>;
}

With this approach, no #[form(lenient)] attribute is needed. Instead, the decision is made at a _use_ site. In most cases, this will be as part of a route. To declare that a form should be parsed in a lenient fashion, we would use a new LenientForm type as the data target in a route, as follows:

#[post("/stripe/endpoint", data = "<api_form>")]
fn endpoint(api_form: LenientForm<Input>) { ... }

This will result in Input being parsed leniently. If instead Form had been used, parsing would be strict.

I find that this _may_ be a better approach. First, it's backwards compatible (at the use site). Second, it enables features like #313 and #298 without having to ensure that structures are declared as lenient. Third, it means structures won't need a Rocket-specific annotation like #[form(strict)]. And fourth, it means that the same structure can be used in both a lenient and strict fashion in different routes and use-cases. The drawback, of course, is that it makes custom FromForm implementations a little harder. In practice, this should be a rare occurrence, and we should work to make it even rarer.

[stephanbuys]

I like the approach, especially in the light of #313 and #298.

[drusellers]

Love this new approach. Having it at the call site will be perfect!