gotham-rs / gotham Goto Github PK

This slipped past 0.1 simply due to the amount of other work that took priority. Is stubbed where the code needs to be added, roughly node.rs#L267.

The implementation here must force segment regex to be anchored to rule out a common class of vulnerability in web applications.

We currently have an in memory session backend which is very useful for proving out basic use cases and initial deployments but isn't suitable for larger deployments i.e. with multiple containers that we're treating like 🐄.

Create a second reference implementation (standalone crate) that utilises Redis (and hence services such as elasticache) as a session Backend .

It would be a good idea to reach out to @benashford to find out the status of https://github.com/benashford/redis-async-rs noting no recent commits. There is another crate in a similar state with no recent commits at https://github.com/tokio-rs/tokio-redis which would also be worth finding out status on.

A non Tokio integrated, but well maintained crate exists at https://github.com/mitsuhiko/redis-rs

Following some usage of the Router builder in "the real world" some tweaks have become clear:

Highlight in documentation:

• scope - Subtree in the same router, applies active PipelineChain
• delegate - Delegates a whole subtree to a secondary Router instance, applies active PipelineChain. Delegated Router may define and apply own PipelineChain instances as normal.

Add:

• delegate_without_pipelines - Delegates a whole subtree to a secondary Router instance, does not apply active PipelineChain. Delegated Router may define and apply own PipelineChain instances as normal.
• associate - Pins to a point in the tree and defines multiple routes, e.g.

route.post("/address").to(checkout::address::create);
route.put("/address").to(checkout::address::update);
route.patch("/address").to(checkout::address::update);
route.delete("/address").to(checkout::address::delete);

becomes something like:

route.associate("/address", |assoc| {
    assoc.post().to(checkout::address::create);
    assoc.put().to(checkout::address::update);
    assoc.patch().to(checkout::address::update); 
    assoc.delete().to(checkout::address::delete);
})

cross-posting this bug here: https://github.com/smangelsdorf/borrow-bag/issues/1

Currently, any crate that depends on borrow-bag must be rebuilt every time. In practice, this means "cargo build" on example-app will trigger a rebuild of borrow-bag, then a rebuild of gotham, then a rebuild of example-app, even if nothing has changed. This makes it annoying to run example-app multiple times in short order, because it takes a solid 11 seconds to rebuild example-app on my high-end laptop, even though nothing has changed.

This was recently announced at https://users.rust-lang.org/t/announcing-the-http-crate/12123.

We'll need support in Hyper before we can migrate to this but once that has occurred we should be able to get there quite quickly given the way we've already made use of similar types from Hyper directly instead of reinventing this particular wheel.

https://github.com/carllerche/http
https://carllerche.github.io/http/http/index.html

It could be clearer that these files are for OS specific socket connections.

Maybe:
clone_sockets.rs => unix_sockets.rs?
socket_queue.rs => windows_sockets.rs?

Should behave the same way as MethodOnlyRequestMatcher but add an additional check against the Accept header provided by the client.

If the Accept header is not present, is */* or has at least one match within a list of allowed content types that is configured when the matcher is created the match should succeed.

If no match is possible the Error returned should be 406 - Not Acceptable.

This is mostly going to be used for API endpoints that want to make sure clients are only making well-formed requests, that are compliant with various specs.

I don't know how feasible this is but something I really liked about NodeJS/express which I don't think can currently be done in Rust/Gotham is generating routes based on the file system tree structure.

What I am thinking is having a AutoTreeBuilder that you point to a folder and then it goes through its children and child folders looking for pub Nodes. These nodes' routes would be overwritten with the tree path.

Middlewear would be able to be available by name for the node to add for specific routes.

For the 0.1 release of Gotham the Router API is reasonably verbose.

This is necessitated by the need to set up all the components the Router expects to see for each Route and is further complicated, to some degree, by the underlying data structure that the Router uses.

The good news is that this can be simplified by adding a layer of code that is specifically targetted at easing construction, the Router itself does not need any major change.

The end goal is to get reasonably close to what Rails and Phoenix offers application developers whilst maintaining type safety and compiler guarantees.

#7 should be taken into consideration as part of this work as any builder/macro implementation will likely need to be adapted again to suit this need.

On your blog, you say that some important features are on the roadmap, including async static file serving. Is there any work on that for the moment ?

With the PR #99 (review) it seems that adding a new example to our set of examples for how to use the various forms of IntoResponse would be beneficial.

PR like this going forward will then be able to clearly demonstrate the use case they're solving.

Originally discussed in the announcement thread, it would be nice if we could support concrete future types using the impl Trait feature which is currently only available on nightly Rust.

What we actually need, though, is for impl Trait to be supported in the associated type position, and for the inferred type to be able to vary with the generic type parameters, which is being discussed at:

• rust-lang/rfcs#1738
• rust-lang/rfcs#2071

We'll need a Middleware trait that looks something like:

trait Middleware<R>
where
    R: Future<Item = (State, Response), Error = (State, Error)> + Send,
{
    type Output: Future<Item = (State, Response), Error = (State, Error)> + Send;

    fn call<Chain>(self, state: State, request: Request, chain: Chain) -> Self::Output
    where
        Chain: FnOnce(State, Request) -> R,
        Self: Sized;
}

Here, R is the concrete type of the future returned by the next Middleware. Though it's only used for the call function, it's likely to be important that this exists at the trait level instead, as SomeUsefulMiddleware::<R>::Output will need to vary with the R type.

Implementing Middleware would then look something like:

struct MyMiddleware;
impl<R> Middleware<R> for MyMiddleware {
    type Output = impl Future<Item = (State, Response), Error = (State, Error)> + Send;

    fn call<Chain>(self, state: State, request: Request, chain: Chain) -> Self::Output
    where
        Chain: FnOnce(State, Request) -> R,
        Self: Sized,
    {
        unimplemented!()
    }
}

Constructing the middleware pipeline will be considerably more difficult with the R parameter here, so this will take some experimentation to prove that the idea works. If this is deemed a suitable way forward, we could investigate making a similar change to Handler.

Ideally, expand this concept to a Cargo template depending on progress with https://internals.rust-lang.org/t/pre-rfc-cargo-templates/5056/51

This ticket represents an idea I had, that I'd like to open up to the Gotham community to consider pros and cons.

I was reviewing https://github.com/gotham-rs and it strikes me that even at this early stage we have a number of repositories with a lot of duplication on things like licenses, contribution guidelines, issue triage etc.

There may be some way we can reasonably combine most, if not all of these into a single repository (this one) which may (or may not!) make for easier long term management of the project.

The structure for a single repository is currently unclear to me but it would need to making working with the following accessible for both people and tools:

• The gotham and gotham_derive crates, via cargo;
• Additional crates, such as middlewares that are being maintained by the Gotham core team, again via cargo;
• Documentation, especially that which is published as the Gotham book;
• Community policies; and
• The Gotham website, which is hosted by GitHub.

All comments welcome.

It seems like certain test cases aren't properly gated. While trying to compile the example app, on Windows, I end up with the following error:

error[E0432]: unresolved import `std::os::unix`
 --> C:\Users\pvl\.cargo\registry\src\github.com-1ecc6299db9ec823\gotham-0.1.0\src\test\mod.rs:9:14
  |
9 | use std::os::unix::net::UnixStream;
  |              ^^^^ Could not find `unix` in `os`

error[E0432]: unresolved import `std::os::unix`
  --> C:\Users\pvl\.cargo\registry\src\github.com-1ecc6299db9ec823\gotham-0.1.0\src\test\mod.rs:10:14
   |
10 | use std::os::unix::io::AsRawFd;
   |              ^^^^ Could not find `unix` in `os`

error[E0433]: failed to resolve. Could not find `unix` in `mio`
   --> C:\Users\pvl\.cargo\registry\src\github.com-1ecc6299db9ec823\gotham-0.1.0\src\test\mod.rs:270:14
    |
270 |         mio::unix::EventedFd(&self.stream.as_raw_fd()).register(poll, token, ready, poll_opt)
    |              ^^^^ Could not find `unix` in `mio`

error[E0433]: failed to resolve. Could not find `unix` in `mio`
   --> C:\Users\pvl\.cargo\registry\src\github.com-1ecc6299db9ec823\gotham-0.1.0\src\test\mod.rs:280:14
    |
280 |         mio::unix::EventedFd(&self.stream.as_raw_fd()).reregister(poll, token, ready, poll_opt)
    |              ^^^^ Could not find `unix` in `mio`

error[E0433]: failed to resolve. Could not find `unix` in `mio`
   --> C:\Users\pvl\.cargo\registry\src\github.com-1ecc6299db9ec823\gotham-0.1.0\src\test\mod.rs:284:14
    |
284 |         mio::unix::EventedFd(&self.stream.as_raw_fd()).deregister(poll)
    |              ^^^^ Could not find `unix` in `mio`

I tried the Hello Router example but seem unable to find some imports.

I seem to have a different version as the example (0.1.2). This might be a bug.

error[E0432]: unresolved import `gotham::router::builder`
  --> src/main.rs:30:21
   |
30 | use gotham::router::builder::{build_simple_router, DefineSingleRoute, DrawRoutes};
   |                     ^^^^^^^ Could not find `builder` in `router`

error[E0425]: cannot find function `start` in module `gotham`
  --> src/main.rs:46:13
   |
46 |     gotham::start(addr, router())
   |             ^^^^^ not found in `gotham`

deps:

gotham = "0.1"
gotham_derive = "0.1"
mime = "0.3"

this is my local lib.rs of Gotham

//! Gotham - A flexible web framework that does not sacrifice safety, security or speed.
//!
//! You can find out more about Gotham, including where to get help,  at https://gotham.rs.
//!
//! We look forward to welcoming you into the Gotham community!
#![doc(html_root_url = "https://docs.rs/gotham/0.1.2")] // Update when changed in Cargo.toml
#![warn(missing_docs, deprecated)]
#![doc(test(no_crate_inject, attr(deny(warnings))))]
// TODO: Remove this when it's a hard error by default (error E0446).
// See Rust issue #34537 <https://github.com/rust-lang/rust/issues/34537>
#![deny(private_in_public)]

#[macro_use]
extern crate hyper;
extern crate futures;
extern crate futures_cpupool;
extern crate tokio_core;
extern crate tokio_io;
extern crate mio;
extern crate borrow_bag;
extern crate url;
extern crate uuid;
#[macro_use]
extern crate log;
extern crate chrono;
extern crate mime;
extern crate serde;
extern crate rand;
extern crate base64;
extern crate rmp_serde;
extern crate linked_hash_map;

#[cfg(test)]
#[macro_use]
extern crate serde_derive;

pub mod handler;
pub mod middleware;
pub mod http;
pub mod router;
pub mod state;
pub mod test;

If this were in Gotham:

impl IntoResponse for Result<Response> {
  fn into_response(self, state: &State) -> Response {
    match self {
      Ok(res) => res.into_response(state),
      Err(e) => e.into_response(state),
    }
  }
}

It would simplify some middleware code. For instance I have this in one of my middleware:

          match result {
            Ok(r) => (state, r).into_handler_future(),
            Err(e) => {
              error!("Require Authn: {}", e);
              let res = Response::new()
                .with_status(StatusCode::InternalServerError)
                .with_body(format!("Require Authn: {}", e))
                .with_header(ContentType::plaintext());
              (state, res).into_handler_future()
            }
          }

If Result<Response,Error> were IntoResponse, the above could be reduced to

(state,result).into_handler_future()

I'm volunteering to address this is with a PR, but wanted to present the case and record the intention.

The 0.1 API for TestServer is a pretty rough, early version. Due to the way we're handling connections internally and allowing everything to be configured, there are a lot of arbitrary variables being set by test cases when they could be defaulted and only overridden when the need arises.

This ticket is mostly a "go back and reduce the boilerplate for integration tests" ticket.

The way a test HTTP client is created and used seems like an obvious target.

Our Router implementation should be able to intelligently provide this detail for controllers, views etc to prevent breakage when restructuring occurs.

Absolute and relative helpers should be supported.

When trying to use gotham in a new project that happens to be using the "slog" crate for logging, I started getting incomprehensible "format argument must be a string literal." errors when trying to derive PathExtractor on a struct.

#[derive(StateData, PathExtractor, StaticResponseExtender)]
struct GetThreadRequestPath {
    no: i64,
}

After some experimentation, I discovered that the trigger for this error is having #[macro_use] extern crate slog; in the main file.

After having looked into it some more, I eventually found the source of the problem. The derive macros in "extractors.rs" in gotham_derive contain several instances of the trace! and error! macros from the "log" crate even though it doesn't even have the "log" crate as a dependency itself. Commenting those out in gotham_derive gets rid of the errors.

It seems like macros inside macros don't get expanded and just leak out into the surrounding code, causing it to basically have an "invisible" dependency on the log crate and causing it to fail with complaints about missing macros if the user does not have the "log" crate as a dependency and #[macro_use] extern crate log; in main. Or, as in my case, if you have another logging crate with incompatible versions of those macros, just give a completely uninformative compile error.

I'm not 100% sure what the best solution here is, but it seems to me like having macros inside macros is a bad idea, as it basically leaks invisible dependencies into the project using them.

Link to reproduction:
https://github.com/forbjok/gotham-repro

hello, thanks for an awesome lib, I was waiting for async HTTP framework since a long time. if possible can you just give benchmarks, so we kinda have some idea?

Thanks, team.

Especially with the idea of providing for modular web apps, it would be incredibly useful to be able to do something like

Response::new()
  .with_header(Location::new(url_for(other_handler))

(this is intended to echo a Rails feature). In other words: Gotham routing already accomplishes routing from a URL to a handler. What this issue proposes is being able to go from a handler and produce URL(s). (Plural because: relative, same base URL as the request, a canonical base...)

There's a number of ways to accomplish this, but one requirement (which isn't addressed by e.g. strings as url paths) is that routing within a module of handlers still work if they're put within a routing tree (or moved around in the tree.)

A further feature requirement would be the ability to "link up" modules - authentication tools often need to know about where to redirect once a user is authenticated, for instance, and reusability relies on being able to configure this into the modules when they're routed.

I would like to run two tokio-based servers in parallel in my application, one of which is a Gotham HTTP server. As far as my understanding of tokio goes, it ought to be possible to create one event loop and register both services onto it.

Is there a way to start a Gotham server using a provided tokio Handle, rather than letting it create its own event loop? If not, and you feel that its a reasonable feature to implement, I'm happy to work on adding this behaviour.

Consider providing cookie prefixes by default, or supporting their use in a low-effort way:

https://www.chromestatus.com/feature/4952188392570880

Integrate with web tooling, such as coveralls.io, codecov.io or codeclimate.com so this information is easily accessible per PR.

Per our previous CI environment this should build with Rust stable, beta and nightly.

If possible the two releases prior to the current stable Rust also.

As part of this ticket Cargo.toml should be updated with AppVeyor badges.

Using the default RouteMatcher implementation, MethodOnlyRouteMatcher does not result in the Allow header being set/populated.

Both the MethodOnlyRouteMatcher and Router implementations need changes to support this, likely one setting Allow values into State and the other populating that into the Response.

There may be a more generic way we can consider to do this, i.e. Router populates all values from a list of headers in State into the Response being careful to not overwrite any existing header set directly by Middleware or Handlers elsewhere in Response generation.

In my freshman app with Gotham I have:

mod app_config {
  use gotham::middleware::{NewMiddleware, Middleware};
  use gotham::state::State;
  use hyper::Request;
  use gotham::handler::HandlerFuture;
  use std::io;
  use std::env;
  use state::AppConfig;
  use futures::{future, Future};

  pub struct New {}

  impl NewMiddleware for New {
    type Instance = Ware;
    fn new_middleware(&self) -> io::Result<Self::Instance> {
      Ok(Ware {})
    }
  }

  pub struct Ware {}

  impl Middleware for Ware {
    fn call<Chain>(self, mut state: State, request: Request, chain: Chain) -> Box<HandlerFuture>
      where Chain: FnOnce(State, Request) -> Box<HandlerFuture> + Send + 'static,
            Self: Sized
    {
      let cfg = AppConfig {
        canonical_url: env::var("CANONICAL_URL").unwrap_or_default(),
        // ...
      };

      debug!("AppConfig: putting config in state");
      state.put(cfg);
      Box::new(future::ok(state).and_then(|state| chain(state, request)))
    }
  }
}

The only thing of interest there is the contents of the call method. It would be nice to only need to write/register that.

Could Middleware/NewMiddleware be implmented for a Fn trait?

This may be a simple as a macro, but I don't have the scope within Gotham to speak to the required generality.

Running the example app, it's easy to cause a panic with this message:

thread '<unnamed>' panicked at 'attempt to add with overflow', src/controllers/challenge.rs:34

simply by visiting this address: http://127.0.0.1:7878/challenge/Rustacean?count=255

If a handler panics, it really shouldn't bring down the entire server. This is where catch_panic before calling each handler would be useful.

It would also be nice if the example-app handled cases where the value was not valid (>255, <0, or not an integer) with a 400 Bad Request extender to present a nice error page. In either case, i think a blank page (which happens now) is worse than causing the browser to show that it couldn't connect to the site or something.

It would be nice to automate some of the core developer activities around Gotham projects. Creating a Middleware, initiating a new web-app, adding a Controller all come to mind, there are likely to be many other things we'd like to automate in the future if other frameworks are anything to go off.

The Diesel approach of shipping a standalone cli tool might be worth considering. This is also how Rails and Phoenix approach the problem.

It would also be prudent to keep the ongoing discussion around Cargo and template support front of mind.

Must support:

• application/x-www-form-urlencoded
• multipart/form-data
• text/plain (?)

Per other extractors for Request path and Query string, we've already shipped this must be type safe and support optional fields.

Something along the lines of

impl<T, U> RouteMatcher for AndMatcher<T, U> {
    fn is_match(&self, state: &State, req: &Request) -> Result<(), StatusCode> {
        self.t.is_match()?;
        self.u.is_match()?;
        Ok(())
    }
}

To support combining individual matchers for a single route.

Generally macros shouldn't have dependencies on the environment in which they're run, but I noticed that deriving PathExtractor and StaticResponseExtender assumes that I have hyper and gotham imported bare in my modules. I think the usual way to avoid this is to use fully-qualified module names by prefixing with ::

In many cases, we've got a rustdoc example being used in lieu of a separate test case. These aren't being included by our current test coverage reporting via Travis / Coveralls.

Per our previous CI environment this should build with Rust stable, beta and nightly.

If possible the two releases prior to the current stable Rust also.

As part of this ticket Cargo.toml should be updated with Travis-CI badges.

One thing which would be useful to add would be an ability to generate some kind of deterministic CSRF cookie.

Proper CSRF is a bit of a PITA, but it would make using gotham for real projects much easier, so it's probably worth having a default solution available.

I think a fairly clean solution would be to add:

trait CSRFProvider {
    fn get_csrf(&self, session) -> CSRFToken;
}
struct DefaultCSRFProvider;

And then pass around a Box<CSRFProvider>. This way a user can customize their CSRFProvider to use a token database or whatever if they don't like the default stateless implementation.

It would also be cool if you could make decoding with CSRF protection a derivable property :)

KitchenSink is a great example of the various pieces of Gotham functioning together but is has grown to the point where maintaining it manually is no longer feasible.

We'll get much better value by splitting this up into integration tests that can be run through CI whilst still maintaining its original purpose as a set of basic examples for working with each component of Gotham.

We should revisit the item visibility we have within Gotham and where practical reduce them to the lowest necessary values, especially where an item should not be used outside of Gotham itself.

This has been possible since Rust 1.18.

Ref: https://doc.rust-lang.org/beta/reference/visibility-and-privacy.html

Right now, gotham is structured with a relatively deep tree. This leads to lots of large use statements in certain files, for example, the routing file in the example app. These statements can be simplified by reexporting types deeper in the tree from higher level modules.

In case I'm not explaining it properly, I mean being able to convert this:

use gotham::router::request::path::NoopPathExtractor;
use gotham::router::request::query_string::NoopQueryStringExtractor;
use gotham::router::tree::TreeBuilder;
use gotham::router::tree::node::{NodeBuilder, SegmentType};

to something like

use gotham::router::request::{NoopPathExtractor, NoopQueryStringExtractor};
use gotham::router::tree::{TreeBuilder, NodeBuilder, SegmentType};

I think this could also be done with other crates like hyper and mime to provide a more streamlined experience, but that might be too much.

Currently, the application State is implemented using a HashMap<TypeId, Box<Any + Send>>. This means that accessing any bit of state data requires a HashMap lookup, and all bits of state must be stored on the heap.

This can be avoided by keeping an HList of the state elements (an HList of RefCell<Option<T>>s). However, this has the downside of requiring each application to declare their own State type, probably using a macro like this:

macro_rules! State {
    $( $repeated: ty ),* => { GothamState(Hlist![$(RefCell<Option<$repeated>>),*]) }
}

// In the application:
type State = State![MyStateBit1, MyStateBit2, MyStateBit3];

Pros: avoids HashMap lookup, Boxing
Cons: forces users to manually declare a State type in their application.

What are your thoughts? Is this a change that the Gotham developers would be interested in?

cc @withoutboats, who has discussed this with me in the past in relation to their web framework, cargonauts.

This would be quite useful as a cargo sub command (this is an area we should explore for Gotham more widely) to easily allow developers to digest the set of routes exposed by an application.

Something along the lines of:

/
| - a*
|   | - b
|   |   | - c*   
|   |   | - d*
|   |
|   | - e* 
|
| - f*

• denotes Node is routable.

Add an example that shows how to use all the various components associated with the initial Request that Gotham providers via State.

New warnings in beta/nightly about unused extern crate are causing warnings on Master which need to be addressed.

Per https://travis-ci.org/gotham-rs/gotham/jobs/269804038

As discussed on Gitter with @bradleybeddoes, I'm missing the possibility to send new HTTP requests inside Gotham handlers because currently it's not possible to get a reference to the underlying Tokio event loop.

@bradleybeddoes pointed me to

An alternative to having Gotham put the handle reference into the state object would be to use a middleware for that: https://github.com/ChristophWurst/gotham-middleware-tokio. You can find an example usage here: https://github.com/ChristophWurst/gotham-middleware-tokio/blob/master/examples/hyper_client.rs#L22

There seem to be a few problems though. I don't know whether we should use a Handle, a Remote or let the developer choose. Handle cannot be shared across threads. However, Hyper expects a Handle rather than a Remote for its client, making it a bit cumbersome to use. The remote object does have method to get a handle, although it's not guaranteed to always get one.

This is somewhat related to hyperium/hyper#1002

@bradleybeddoes @smangelsdorf thoughts on how to integrate this in Gotham?

The session middleware currently hard sets the cookies header without a care in the world.

This means that anything else in the stack which had set a cookie before it gets invoked is sadly out of luck.

Currently this trait is defined as:

pub trait IntoResponse {
    /// Converts this value into a `hyper::Response`
    fn into_response(self) -> Response;
}

Consider extending the signature to allow it to operate with http::response::create_response which requires State, StatusCode and an optional Body.

This may have an undesired impact on the impl IntoResponse for Response case in which case we should not proceed.

Once CI and other tools are configured on GitHub add appropriate badges to Cargo.toml and README.md.

Currently our session cookies are proper session cookies, which (except where defeated by certain modern browser features) are removed at the end of a browser session. Might it be useful to allow the lifetime to be set explicitly?

There are two problems with this from a security point of view:

• If the cookie expires on the client side but not the server side, the cookie value is not invalidated. If somebody has stolen the cookie, they can keep using it to keep refreshing the TTL (i.e. false sense of security for apps that set an expiry and expect it to behave correctly); and
• The cookie survives browser restarts, which is the common way that end users are instructed to ensure logout is completely effective.