Typed Error Catchers, Fairings

[mmrath]

What I want is to inspect every request and if it is anything but POST to /api/auth/register or /api/auth/login then check for for auth header. If header is not present, then reply 401. I looked at Fairing - but Fairing cant respond to requests. I looked at request guard, it looks like for them I need to add a param to every route handler.

Questions

Any questions must include:

1. The version of Rocket this question is based on, if any. master

2. What steps you've taken to answer the question yourself. Looked at Fairing and request guard docs

3. What documentation you believe should include an answer to this question. not sure

[jebrosen]

The pain point you are encountering is that request Fairings cannot alter the request handling flow by responding or by returning some kind of error type. This is a known issue that can and should be available in Rocket's API eventually but I don't see an open issue for it.

Adding a request guard to every route handler is annoying, but does make it more obvious which routes need authentication and which don't. If you do add a request guard to every route indicating its privilege level, you can require instances of those request guards in database or API code as a compile-time assurance that the necessary security properties are enforced, as well.

But if "every" route requires authentication, it can be annoying to do that. You can work around the fairing response situation by altering the request uri in the fairing, thereby changing which route it ends up going to. rocket_cors does this (https://github.com/lawliet89/rocket_cors/blob/master/src/fairing.rs#L61), for example. It is a hack and I'm hesitant to even bring it up, but it might be worth exploring its suitability for your problem.

[mmrath]

From what I hear from you this issue should be a feature request than a question. I pretty much need everything except login screen to be authenticated. I think this is very common scenario.

[SergioBenitez]

@mmrath I'm with you. We have a feature in mind that would resolve this problem, but alas, it is as of now unimplementable due to lacking features in Rust.

[NoUsername]

@SergioBenitez would be interesting to know what features Rust would need so this could be implemented. Keep up the good work 👍

[mmrath]

@SergioBenitez Would you be able to give a bit more information on how the solution might look like?

[SergioBenitez]

@NoUsername We'd need non_static_type_id, blocked in https://github.com/rust-lang/rust/issues/41875.

@mmrath The gist, from my design doc, is:

Design

As the name implies, the guiding principle behind type-based catchers is that catchers should operated type-dependently. Instead of invoking catchers based purely on an error's status code, a catcher will be invoked based on the type of an error, if any. This requires a full revamp of the catch attribute, including its syntax and semantics, a semantic change to the [Fairing] trait, as well as minor changes to guard traits.

Second, and equally important, the default semantics of error catchers will be changed so that if the type of an error value implements Responder, a Response will be created directly from the error value.

Fairings

Fairings gain the ability to respond early to requests. The primary changes to the Fairing trait are: the addition of a new associated type, Response, and a modification of the on_request method:

type Result<R> = Result<Data, (Status, R)>;

pub trait Fairing: Send + Sync + 'static {
+   type Response: Error = !;

    ...
-   fn on_request(&self, request: &mut Request, data: &Data) { .. }
+   fn on_request(&self, request: &mut Request, data: Data) -> Result<Self::Response> {
+       Ok(data)
+   }
    ...
}

Note that this is a departure from the previous (explicitly designed) rule that fairings cannot terminate or respond to an incoming request directly. The justification is two-fold:

• Rocket will properly and explicitly log how, when, and which fairing caused an early termination.
• The user can always catch the fairing's value. As such, the user always maintains full control.

[mmrath]

@SergioBenitez Thanks for the insight. I am not sure I understood fully, but is there no alternate design to implement this with current Rust? non_static_type_id does not appear to be available soon.

[SergioBenitez]

@mmrath No, I don't believe so. I'm not sure what you mean by "does not appear to be available soon". It seems that all relevant parties are interested in having the feature be implemented. What's missing is a bit of elbow grease to fix a compiler bug that's preventing the implementation.

[jebrosen]

The non_static_type_id feature, required for the plan described in https://github.com/SergioBenitez/Rocket/issues/749#issuecomment-432572197, has (unfortunately for Rocket) been rejected. I do have a vague idea of a partial solution, however! - see https://github.com/SergioBenitez/Rocket/issues/1234#issuecomment-647168940.

[Stargateur]

it's annoying cause that mean I can't return a descriptive error using guard, that very unfortunate to just return a "303" when I could be very precise about error for example a revoked jwt.

[Nis5l]

For anyone having this problem, I'm currently solving this by pushing the error data onto the local_cache of the request and reading it at the catcher. Don't know if its a good solution, but it works.

[leokhachatorians]

For anyone having this problem, I'm currently solving this by pushing the error data onto the local_cache of the request and reading it at the catcher. Don't know if its a good solution, but it works.

It's not perfect, but it works and it seems to be easy to understand what's going on. :)

[Guillermogsjc]

hi folks,

is this 422 error catching into json response proposed by https://github.com/SergioBenitez/Rocket/issues/1234#issue-567845110 already integrated in any form into rocket API, or any form to catch this error thrown on html by default into json?

[f3oall]

Hey guys, I'm really confused now about how I should provide details about the error(e.g. 422 validation errors). It seems like an essential part of a web server. So any news about it?

[Stargateur]

it's a core design problem, that not like someone can fix it with a small commit :p

[dtolnay]

If this is still stuck on non_static_type_id (https://github.com/SergioBenitez/Rocket/issues/749#issuecomment-432572197), the following stable workaround may be enough to unstick.

use std::any::TypeId;
use std::marker::PhantomData;
use std::mem;

pub fn non_static_type_id<T: ?Sized>() -> TypeId {
    trait NonStaticAny {
        fn get_type_id(&self) -> TypeId where Self: 'static;
    }

    impl<T: ?Sized> NonStaticAny for PhantomData<T> {
        fn get_type_id(&self) -> TypeId where Self: 'static {
            TypeId::of::<T>()
        }
    }

    let phantom_data = PhantomData::<T>;
    NonStaticAny::get_type_id(unsafe {
        mem::transmute::<&dyn NonStaticAny, &(dyn NonStaticAny + 'static)>(&phantom_data)
    })
}

This produces TypeId values which are 100% compatible with ones obtained in the usual way.

fn assert_t_is_str<T>() {
    assert_eq!(non_static_type_id::<T>(), TypeId::of::<&str>());
}

fn main() {
    assert_t_is_str::<&str>();
}

[ananyo141]

For anyone having this problem, I'm currently solving this by pushing the error data onto the local_cache of the request and reading it at the catcher. Don't know if its a good solution, but it works.

Can you please provide a snippet of what should be done so newbies like me can get a hint at the implementation until a canonical way is established?

[Nis5l]

For anyone having this problem, I'm currently solving this by pushing the error data onto the local_cache of the request and reading it at the catcher. Don't know if its a good solution, but it works.

Can you please provide a snippet of what should be done so newbies like me can get a hint at the implementation until a canonical way is established?

When I encountered this problem, I was seeking a solution for this project. You can search for local_cache within the project.

[SergioBenitez]

@Ian-Bright

5 years this issue has been open and still no fix.... This seems like something that should have been in place from the start.

Please feel free to submit a PR with an implementation! I'd be more than happy to review it.

[SergioBenitez]

I've been working on this recently in a push to get 0.6 out sooner than later. There are ~two difficult problems that we need to solve to make this happen. As far as I can tell, there's no evidence to suggest that a solution to the latter exists. They are:

1. Get the TypeId for any T: ?Sized.

   @dtolnay's approach solves this, as do a few other similar approaches. This is needed so that we can take an error value produced by some part of a Rocket application, such as a Fairing or FromRequest impl, and store it as an opaque pointer that we can later recover as the original value.

2. Safely downcast an opaque pointer into a value of its original type which may contain references.

   This is the "recover as the original value" part. A user requests some value of type Foo<'x> for any concrete or generic lifetime 'x; does the opaque pointer point to a value of type Foo<'x>? If so, downcast to it and provide it to the user.

As a concrete example, consider the Json<T> request guard. When the guard fails, it produces an error value of type json::Error<'r>, where 'r is the lifetime of the request. We'd like to write a handler/catcher pair as follows:

#[post("/", data = "<json>")]
fn submit(json: Json<Foo>) { .. }

#[catch(default)]
fn json<'r>(error: json::Error<'r>) -> MyError<'r> { .. }

As mentioned before, if the Json<Foo> guard fails for some request &'r Request<'_>, it will produce an error value of type json::Error<'r>. The idea is to forward this error type to the matching catcher that request's it, or json in this case. Here, the json error catcher, to avoid cloning unnecessarily, uses the reference in the json::Error::Parse variant in its MyError<'r> response.

So far, this is well typed and sound, and we can accomplish this today. However, consider the following version of json instead:

#[catch(default)]
fn json(error: json::Error<'static>) -> MyError<'static> { .. }

It would be unsound for Rocket to take the json::Error<'r> produced by the failing Json<Foo> guard and convert it into a json::Error<'static>, but the TypeId we produce in (1) above does not discriminate between lifetimes, so a straight-forward downcast would be incorrect. We need some way to downcast only when doing so would produce a value that does not contain references that outlive the request.

As of now, I do not know how to do this.

[the10thWiz]

As of now, I do not know how to do this.

I'm pretty sure this isn't possible in Rust right now. Lifetimes are erased at compile time, before TypeIds are generated. Our problem is somewhat more specialized than a generic non_static_typeid, since we already know what lifetimes we want to allow: 'request and 'static. As it stands, the only thing we really can do, is look at restricting errors to one lifetime, and enforcing that lifetime at both ends. This is relatively easy if we choose 'static, since we can trivially require the catcher types to implement 'static', and use a specialization trick likeSentinelto only extract error types with'static` lifetimes.

On the other hand, I'm not sure how to go about restricting the input lifetime of a type to a specific (non-static) lifetime. The catcher signature would look something like: for<T, 'r: T> fn(T) -> E. (Note that Rust doesn't support this, since I'm asking that a lifetime outlive a type). I think the only real option at this point would be to use the macro to check that the catcher don't use any explicit 'static lifetimes, and instead require them to either be implicit, or a lifetime parameter. However, I think this is still unsafe in the presence of type aliases. Playing with some toy examples, I'm pretty sure that there is nothing we can do to make this safe, unless Rust implements non_static_typeid.

[the10thWiz]

We could potentially enforce a variant of this via and ouroboros-like implementation, where the error type is 'static, because it contains the Request it borrows from. However, I suspect making error types that are compatible with this is significantly more work, and we would need to experiment with some type of macro.