Rocket: The usage of the `&str` type is potentially misleading.

[sgrif]

Steps to reproduce:

• Have a route with the following code:

  ```rust
  #[derive(FromForm)]
  pub struct FormData<'a> {
  form_data: &'a str,
  }

  [post("/reproduce_bug", data = "

  ")]

  fn reproduce_bug<'a>(form: Form<'a, FormData<'a>>) -> &'static str {
  assert_eq!("A value with spaces", form.get().form_data);
  "Everything is fine"
  }
  ````

• Create a template with the following HTML, and submit it using a browser

  ```html

  ````

Expected behavior

The response "Everything is fine" is rendered

Actual behavior

The value of the field is "A+value+with+spaces", causing the assertion to fail and a 500 to be returned.

Additional details

This only applies to values of &str and not String, so I'm assuming that &str is unconditionally pointing at the raw request data to avoid any allocation. Even if this behavior is documented (I couldn't find it mentioned), I think this is a surprising behavior and likely to be a common source of confusion. At absolute minimum this should produce some kind of error, but even that is a sub-optimal solution as it would lead to unexpected gotchas for users who do not test their inputs with spaces.

I'd expect the framework to be allocating internally if needed to properly handle decoding entities, and pointing at the raw request data if possible. I would never expect the framework to force me to care about encoding or content-type specific details unless I've specifically asked for some form of "raw" data.

I have not checked, but I suspect there are likely similar issues if the request is submitted with a charset other than UTF-8, and for JSON requests that would require additional decoding (such as an escaped backslash character).

[SergioBenitez]

This is by design. You can use an &str if you're okay with the raw data for whatever reason, or a String and pay the allocation + verification costs. The issue here is really one of documentation. It seems that I missed documenting the provided implementations and providing a cautionary tale, as there is for FromParam.

[sgrif]

As mentioned, I think that considering a documentation bug would be a major mistake. It sets up the expectation that everything could have hidden gotchas in the docs. Most people will just try what they expect to work, which may or not make it obvious to them what is happening. This is extremely subtle behavior that is not made at all explicit by the API

I think a much better approach would be to provide a Raw type which the user can use to request undecoded request data.

[sgrif]

I also didn't realize that the same behavior occurred in FromParam. I would hope this API decision will be reconsidered. In general I think most people will assume that str and String are roughly interchangeable, as that's the expectation that is set up by the entire Rust ecosystem. A note somewhere in the docs is not a great way to communicate that behavior, especially with a strong type system that can be used for communication.

[Bobo1239]

I fully support sgrif's arguments. I'm going to use the framework for a toy project and this is definitely something I wouldn't have expected. A Raw type as suggested seems like a much cleaner solution.

[liigo]

Keep in mind 'Principle of Least Surprise'.
I was surprised by this issue(feature).

[SergioBenitez]

I've reconsidered my position.

Here's the design I'd like to move to: Add a new type &RawStr, that's used anywhere an &str is used now. The type should act like an &str in all possible regards. Furthermore, it should expose convenient methods like .url_decode() and .html_escape().

[mike820324]

@SergioBenitez
While I'm playing with this issue, I'm wondering, is it possible to just add a new public trait UrlDecoder and implement the trait for the &str.

Now the user only need to do is import the UrlDecoder trait and call the url_decode function of type str.

[mike820324]

Moreover, we can also implement the UrlDecoder trait for String type, and hence reduce some of the common logic in

• https://github.com/SergioBenitez/Rocket/blob/master/lib/src/request/param.rs#L213
• https://github.com/SergioBenitez/Rocket/blob/master/lib/src/request/form/from_form_value.rs#L170

[SergioBenitez]

This is now implemented! It was implemented across a few commits, but primarily in 709acf18a4fb67b98e02295381e4c3a969acb061, 0c44e4464151d736ea2806a1ff8321919e6b9aa1, and f5ec470a7d8b24a031fc9201679329ca2a524fde.