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

Panic in NormalizePath when presented Uri is in authority form #2240

Closed
Eijebong opened this issue May 31, 2021 · 3 comments · Fixed by #2246
Closed

Panic in NormalizePath when presented Uri is in authority form #2240

Eijebong opened this issue May 31, 2021 · 3 comments · Fixed by #2246
Assignees
Labels
A-web project: actix-web C-bug Category: bug good-first-issue easy to pick up for newcomers

Comments

@Eijebong
Copy link

When running the following code:

use actix_web::{
    middleware::{self, normalize::TrailingSlash},
    rt, App, HttpServer,
};

fn main() {
    let mut runtime = rt::System::new("test");
    let server = HttpServer::new(move || {
        App::new()
            .wrap(middleware::NormalizePath::new(TrailingSlash::Trim))
        }).bind("127.0.0.1:8888").unwrap().run();

    runtime.block_on(server).unwrap();
}

I get panics like this:

thread 'actix-rt:worker:0' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidUriParts(InvalidUri(SchemeMissing))',
/usr/local/cargo/registry/src/github.com-1ecc6299db9ec823/actix-web-3.3.2/src/middleware/normalize.rs:149:46

with requests like this: GET eh HTTP/1.1 (notice the missing /)

@robjtede robjtede added C-bug Category: bug good-first-issue easy to pick up for newcomers A-web project: actix-web labels May 31, 2021
@thalesfragoso
Copy link
Member

Should we just skip normalizing if the path is empty ? Or TrailingSlash::Always makes sense for an empty path ?

@robjtede
Copy link
Member

robjtede commented May 31, 2021

path cannot be empty according to http spec

edit: though potentially doable still, an empty path should be converted to / according to client spec

@robjtede
Copy link
Member

robjtede commented Jun 3, 2021

Following discussion on Discord...

In the presented test case (GET eh HTTP/1.1), eh is not a path since all paths must start with a forward slash. It is being interpreted as an authority form (see https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.3) and as such the NormalizePath middleware should do nothing in such cases.

@robjtede robjtede changed the title Panic in NormalizePath when getting a path that doesn't start with a / Panic in NormalizePath when presented Uri is in authority form Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web project: actix-web C-bug Category: bug good-first-issue easy to pick up for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants