Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Router doesn't match any routes after a POST, thinks request is an Extension instead of a POST #326

Open
shepmaster opened this issue Apr 5, 2016 · 7 comments

Comments

@shepmaster
Copy link

Originally reported on Stack Overflow.

Reproduction steps

Create a new Cargo project

Cargo.toml

mustache = "0.7.0"
nickel = "0.8.0"
rustc-serialize = "0.3.19"

main.rs

#[macro_use]
extern crate nickel;
extern crate mustache;
extern crate rustc_serialize;

use std::collections::HashMap;
use nickel::{Nickel, MediaType, HttpRouter};
use nickel::status::StatusCode;

fn main() {
    let mut server = Nickel::new();
    let mut router = Nickel::router();

    router.get("/", middleware!(|request, mut response| {
        response.set(StatusCode::Ok);
        response.set(MediaType::Html);
        return response.send_file("assets/login.tpl");
    }));

    router.post("/login", middleware!(|request, mut response| {
        response.set(StatusCode::Ok);
        response.set(MediaType::Html);

        let mut data: HashMap<&str, &str> = HashMap::new();
        data.insert("error", "hello");
        return response.render("assets/login.tpl", &data);
    }));

    server.utilize(router);
    server.listen("127.0.0.1:6767");
}

assets/login.tpl

<html lang="en">
    <head>
        <meta charset="utf8"/>
    </head>
    <body>
        <h1>Login</h1>
        <form method="post" action="login">
            <label for="email">Email</label>
            <input type="email" name="email"/>
            <br/>
            <label for="password">Password</label>
            <input type="password" name="password"/>
            <br/>
            <button type="submit">Login</button><br/>
            <a href="/register">Register</a>
        </form>
        {{error}}
    </body>
</html>
  1. Visit http://127.0.0.1:6767/login
  2. Fill in the fields and submit
  3. Fill in the fields a second time

What is expected: The form continues to be shown
What happens: The server returns an HTTP 404

Initial investigation

I added some debug prints to router.rs:

.inspect(|item| println!("{:p}, {:?}, {:?}, {:?}", self, item.method, method, item.matcher.is_match(path)))

On the first POST, it iterates through the middleware with the following:

0x107e29160, Get, Post, false
0x107e29160, Post, Post, true

On the second POST, the incoming request's method is all screwy:

0x107e29160, Get, Extension("email=a%40b.com&password=aPOST"), false
0x107e29160, Post, Extension("email=a%40b.com&password=aPOST"), true
@Ryman
Copy link
Member

Ryman commented Apr 5, 2016

Thanks @shepmaster for taking the time to report it here.
This is a known issue in hyper hyperium/hyper#309.

@cburgdorf @simonpersson We should probably deal with this even if hyper doesn't (yet), perhaps by auto closing keep-alive requests if their body hasn't been fully read, or by reading up to a limit (as suggested in the hyper issue, but can be dangerous without a limit).

@cburgdorf
Copy link
Member

perhaps by auto closing keep-alive requests if their body hasn't been fully read

This is the same as what @seanmonstar means with this right?

If a request claims a body, and you have not read it, then the socket will not be marked keep-alive

So, basically we are talking about requests that claim a content-length but then their body doesn't match that content-length?

If that's the case that I think that would be my preferred approach of handling it but I have to admit that I don't think I get the full picture here ;)

Ryman added a commit to Ryman/nickel.rs that referenced this issue Apr 10, 2016
This puts a bandaid on nickel-org#326. A more complete fix will require
some shuffling of the api exposed by Request so that we can
monitor for when the body has been fully read (or read up to
a defined limit).
@Ryman
Copy link
Member

Ryman commented Apr 10, 2016

This is the same as what seanmonstar means with this right?

Yes, that's the solution they've gone with on the async branch. I took a look at trying to attempt the same on the current sync api (in hyper itself) but what I came up with was pretty awful and would have required passing around RefCells to synchronise between the Request and Response.

So, basically we are talking about requests that claim a content-length but then their body doesn't match that content-length?

Not exactly, it would probably manifest itself as the same issue though. The problem is when a handler doesn't read the body, which leaves a load of junk in the buffer, which hyper then tries to interpret as the next request.

I've pushed a weak fix up as a PR #327. I'd prefer if we had a better solution here but it's tricky to know when the body has been fully read with the current Request api (something which has been a pain for a long time now).

anvie pushed a commit to anvie/nickel.rs that referenced this issue Jan 30, 2017
@gretchenfrage
Copy link

Has this issue been fixed? I would like to use nickel for a project, but I'm encountering this same problem, which seems to render it pretty unusable.

@gretchenfrage
Copy link

Ok, I got my code to work, but this still seems like something that ought to be fixed.

@shepmaster
Copy link
Author

shepmaster commented Jan 2, 2019

@gretchenfrage I don't think that Nickel is actively maintained. You may wish to check Are we web yet? for current recommendations.

I see it was last updated a month ago; perhaps I haven't been paying attention 😉

@jolhoeft
Copy link
Member

jolhoeft commented Jan 2, 2019

Development is going slow, although I hope to merge the serde migration soon.

For this issue, I think it will need to wait until we migrate to a newer version of hyper. I made a pass at that for the hyper-0.11 train, but had to abandon it due to time pressure (and tokio/futures being a bit unstable). I hope to revisit it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants