Request: Access to managed state in `from_param`

[rbalicki2]

• I love the pattern shown in https://rocket.rs/guide/state/#usage where the DbConn is created from the managed state pool and then exposed as a Request Guard.
• I'd like to do be able to do the same thing in a FromParam implementation. Namely, I want to enable

#[get("/todos/<todo>")]
pub fn get_todo(
  todo: Todo
) -> Json<Todo> {
  Json(todo)
}

And have Todo's FromParam implementation fetch a todo with a particular ID.

• Right now, I use a separate db::establish_connection function which returns a ConnectionResult<PgConnection>. This is not ideal, as it doesn't share the same pool.
• Alternatively, I could implement FromRequest for Todo, and call request.route().unwrap().uri. However, then Rocket complains that x is declared as an argument but isn't in the function signature. Likewise, I wouldn't know which path parameter to match (e.g. it is ambiguous in the case of /user/<user>/photos/<photo> where we want to do this for both users and photos)
• I think ideally, one should be able to do this in other places, for example when deserializing a struct from data, although I haven't thought very deeply about how that would actually happen.
• One possible solution is to make the request accessible from the RawStr (or just the managed state.) This seems weird, obviously, as why would a raw string know about a request?
• Or, one could pass an object PathParamWrapper which dereferences or Into's to the RawStr (honestly, I'm not sure if what I just said makes sense).

[tcmal]

In my opinion this isn't needed & should be in your routing function, letting you handle errors correctly. Users aren't passing you a Todo; they're passing you a reference to one they'd like you to resolve (if you can).

[inferiorhumanorgans]

In my opinion this isn't needed & should be in your routing function, letting you handle errors correctly. Users aren't passing you a Todo; they're passing you a reference to one they'd like you to resolve (if you can).

Being able to access state (e.g. the database pool) from a parameter guard would be useful here for turning the reference into a todo object.

Currently you can shove db requests into a request guard but then you'll have no real way of figuring out what the reference actually is. In my use case I've got a path with a variety of segments representing objects in the database. Some classes are used multiple times in the path, and some classes will want to do a bit of inspection higher up the hierarchy. My current solution is lots of boilerplate code in each route which I'd love to get rid of.

[Jayshua]

I'll add my use case: Ensuring the user has access to the resource they are requesting. For example, imagine a todo:

#[get("/todo/<id>")]
fn get_todo(id: TodoId, authorization: ApiToken) -> Json<Todo> {
   ...
}

It would be nice if the TodoId FromParam implementation could ensure the user has authorization to access the requested todo. Particularly when dealing with token-based authentication where an account can have multiple authentication tokens generated for different third-party applications, each with their own access permissions.

Right now I have multiple authorization guards for different permission levels:

#[get("/todo/<id>")]
fn get_todo(id: TodoId, authorization: ReadTodo) -> Json<Todo> {
   ...
}

But while this ensures the user is allowed to read a todo, it doesn't ensure they are allowed to read the specific todo requested.

[jebrosen]

I've gone back and forth on this quite a bit.

On the one hand FromParam to me "feels like" it should mainly or only be used for quick validation and type coercion such as ensuring a parameter is indeed a number, UUID, date, in the valid range, and so on.

But it would be really convenient to be able to leverage Forward in the case of missing authorization or an entry not being found, something that you might need access to managed state for. The main workarounds seem to be combining a path parameter with FromRequest (fragile) or doing manual delegation to other routes inside the route in question (awkward).

[jhoobergs]

@jebrosen I'm justing starting to use Rocket and following comment should be viewed as a comment of a newbie :)

Would it be a solution to allow using the path parameters in a request guard? The request guard would then check whether access to the specific todo should be granted or not?

This seems to me something that happens quite a lot and I do believe that FromParam needs to remain as lightweight as possible, so allowing the request guard to access the parameters (like they can access other guards) seems a good solution to me? I am, however, not sure if this is feasible.

[the10thWiz]

This is more complicated that it seems on the surface. I don't think it's appropriate for FromParam to allow access to shared state or any other part of the request. Of the workarounds above, I think there are a few things to point out: manual delegation should be avoided, that's what the Router & Forwarding is for. Using FromRequest seems to be the best solution available, but it's obviously not a very good one.

One potential solution would be something like .guard::<T>() on Request, which would allow running param guards from inside a request guard. However, this is still dependant on the URL format, which Request Guards shouldn't be. It may be desirable to add an additional trait, such as FromParamAndRequest, which would take both the request & the param. Figuring out how to mark this for the codegen is difficult (although perhaps a blanket impl could make this simpler), and it seems unintuitive and complicated.

The solution I like the best is something like a GuardedDatabase, which is like a database connection, but it requires the id, and checks permissions. Encouraging this type of design may be the best solution here, unless we can come up with a more complete solution.

One option for exposing Parameters to the Request Guards would be via a sort of generic type. E.g., that would make the above example:

#[get("/todo/<id>")]
fn get_todo(id: TodoId, auth: ReadTodo<id>) -> _ { }

It's going to take some time to figure out how to write traits for this, let me know if this is worthwhile.

[jebrosen]

so allowing the request guard to access the parameters (like they can access other guards) seems a good solution to me? I am, however, not sure if this is feasible.

@jhoobergs It's more than feasible - request guards can already invoke parameter guards via Request::param. But this is pretty unsatisfactory because it accesses parameters by index instead of in a way that can be verified at compile time; that gap is why I called it a "fragile" solution.

@the10thWiz I strongly dislike the reuse of generics syntax in that example. Hypothetically, I think we could do something similar with attributes. But regardless of syntax I'm worried about the complexity of this feature, both to implement and to explain and use. Allowing FromParam to access &Request seems like a much more straightforward way to achieve it.

#[get("/todo/<id>")]
fn get_todo(id: TodoId, #[args(id)] auth: ReadTodo) -> _ { }
// would call `<ReadTodo as FromRequest<'r, (TodoId)>>::from_request(r, (id))`

[jhoobergs]

@jebrosen Thanks for the info. Is it feasable to make that fragile solution less fragile? Using the index is certainly not robust enough.

[jebrosen]

Is it feasable to make that fragile solution less fragile? Using the index is certainly not robust enough.

In principle it's not only feasible, it should be relatively trivial to add - i.e. the main cost is changing every implementation of FromParam everywhere. I think the question is more about whether we want to encourage this pattern with FromParam vs another approach.

[Tortoaster]

If we were to vote, I would be in favor. I would love to be able to turn this pattern:

#[get("/post/<id>")]
pub async fn view_post(id: i32, db: MyDB) {
    let result: QueryResult<Post> = db.run(move |conn| posts.find(id).first(conn)).await;
    match result {
        Ok(post) => { /* ... */ }
        Err(_) => { /* handle error*/ }
    }
}

Into this:

#[get("/post/<post>")]
pub async fn view_post(post: Post) {
    // ...
}

Which would be possible if FromParam has access to the request (and from_param is async, thank you @the10thWiz):

#[rocket::async_trait]
impl<'a, 'r> FromParam<'a, 'r> for Post {
    type Error = ();

    async fn from_param(param: &'a str, request: &'r Request<'_>) -> request::Outcome<Self, Self::Error> {
        param.parse::<i32>().or_forward(()).and_then(|id|
            request.guard::<MyDB>().await.and_then(|db|
                db.run(move |conn| posts.find(id).first(conn)).await.or_forward(())
            )
        )
    }
}

I think it's quite intuitive, as this makes parameters more similar to request guards. I also have a User request guard that reads the id of the logged-in user from a cookie, and automatically retrieves associated data (i.e. username) from the database. This is currently possible if the id is a cookie, but not if it's a parameter, despite both being part of the request. I'm not sure if this approach can bring problems that I overlooked, but I think it would be great if Rocket can retrieve all necessary information for a page just by looking at the function!

[the10thWiz]

I too am in favor. The from_param method isn't async, so @Tortoaster 's code example actually doesn't work. This could be handled by making from_param async, which I think is an important collorary to this change.

I would be quite happy to create a PR with these changes.