shio-rs icon indicating copy to clipboard operation
shio-rs copied to clipboard

Middleware

Open mehcode opened this issue 8 years ago • 7 comments
trafficstars

See https://github.com/mehcode/shio-rs/blob/v0.1.0/examples/middleware/src/main.rs

Don't have time to expand on exactly what's going on there. I feel it's a much better interface than what we had. I'll be working more on this later tonight.

If anyone has an idea of how to make a builder that makes it easier to construct, I'd love the help there.


  • [ ] Design a builder interface for the new Stack and put inside of lib

  • [ ] middleware::Cors
  • [x] middleware::Recover - catch_unwind and return 500 from panics instead of crashing the server. This should probably be included in Shio::default. ( from @Meralis40 )
  • [ ] middleware::Secure - Looking at https://echo.labstack.com/middleware/secure
  • [ ] middleware::Static - Static files

mehcode avatar Aug 29 '17 20:08 mehcode

Link doesn't work

nihiluis avatar Sep 01 '17 11:09 nihiluis

@nihiluis Thanks for pointing that out. This is what I get for not using a permanent link.

See: https://github.com/mehcode/shio-rs/blob/v0.1.0/examples/middleware/src/main.rs

It doesn't compile anymore as IntoFutureExt no longer exists but it should be simple to fix it. I just haven't had the time yet. I removed the example because of that so tests would pass for now.

mehcode avatar Sep 01 '17 17:09 mehcode

thats pretty complex huh

in gin and express you can just place the middleware before or after a route. like this:

let mut router = Router::new();
router.add((Method::Get, "/", index));
router.use(check_auth);
router.add((Method::Get, "/protected", protected_route));

with check_auth being something like this:

    fn check_auth(mut context: Context) {
        context.put::<Number>(0);

        context.next();
}

I'd prefer that way, this seems unnecessary complex to me. maybe its just the rust way, idk.

nihiluis avatar Sep 01 '17 21:09 nihiluis

I admit the usage of that example is garbage, that example is just a proof-of-concept, "hey this works and doesn't use boxes".


This is about as simple as we can make middleware functions:

fn check_auth<H: Handler>(mut context: Context, next: H) -> H::Result {
  context.put::<Number>(0);
  
  // Arbitrary types can't be directly callback until Fn traits are stabilized  
  next.call(context)
}

The next argument must be generic as it could be chaining to anything and we want to avoid boxing (allocations).

If the middleware is complicated at all it'll need boxing until impl Future is stabilized.

fn check_auth<H: Handler>(mut context: Context, next: H) -> BoxFuture<Response, Error> {
  context.put::<Number>(0);

  next.call(context).map(|response| {
    // Do something interesting
    response
  }).into_box()
}

Here are some syntax ideas that I'd like to see be possible before Middleware will be considered done in Shio:

let mut router = Router::new();

// Normal route
router.add((Method::Get, "/", index));

// [1] "Protected" route
router.add((Method::Get, "/protected", protected_route.with(check_auth)));

// [2] "Protected" group of routes
let mut protected_routes = router.group("/").with(check_auth);
protected_routes.add((Method::Get, "/protected", protected_route));

// [3] Whole server is "protected"
Shio::new(router).with(check_auth).run(":7878")

Here is another idea I'd like to see exist behind a nightly flag (maybe with slightly different syntax):

#[get("/protected")]
#[shio(with = "check_auth")]
fn protected_route(c: Context) -> Response { /* ... */ }

// [...]

router.add(protected_route);

mehcode avatar Sep 01 '17 22:09 mehcode

I've done a basic version of a Static middleware as a Handler here, using rayon for the thread pool + std io. Also with an basic example (static_files). Notes:

  • it's actually very difficult with Router to add the same handler for 2 methods on same route, we need a fallback from HEAD to GET handler, that discard the body.
  • or we need a way to have global state (PR #20 is good enough).
  • it's not optimized (many allocations for sending data), and need better error handling.

Meralis40 avatar Sep 14 '17 06:09 Meralis40

using rayon for the thread pool + std io

Why rayon and not [future-cpupool](using rayon for the thread pool + std io)?

it's actually very difficult with Router to add the same handler for 2 methods on same route, we need a fallback from HEAD to GET handler, that discard the body.

Agreed that we should have a default HEAD fallback but we should also make this easier. For now a HEAD fallback will work fine.


A couple more issues I see from a brief look over.

  • Both std::fs::metadata and File::open are blocking calls and must be in the thread pool.
  • I'd rather return the 500 if File can't open a file that just had metadata read successfully from it. That sounds like an exceptional case that shouldn't be swallowed by a 404.

Some more thoughts:

I'd like this to be as simple to configure as possible:

let mut s = Shio::default();

// Attach the Static Middleware to `/`
s.attach(Static::new("static"));

// Attach the Static Middleware under a different prefix
s.attach(Prefix::new("static", Static::new("static")));

// Use the threadpool File manually
s.route((Method::GET, "/static/{filename:.*}", |context| {
  // Not sure on best name, Rocket uses NamedFile
  // Note that a Responder can return a future
  Response::with(shio::File::open(context.get::<Parameters>()["filename"]))
})

Ignore the middleware stuff for now and let's focus on making that last example a thing.

mehcode avatar Sep 15 '17 11:09 mehcode

@mehcode Sure, it's just a draft, can surely be improved. I've use rayon for ease of use only. The last example should be the way to do.

EDIT: I've updated my branch for static files, the example is a little more complex.

Meralis40 avatar Sep 15 '17 12:09 Meralis40