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

URL parameters not being decoded. #209

Closed
cedws opened this issue May 4, 2018 · 15 comments
Closed

URL parameters not being decoded. #209

cedws opened this issue May 4, 2018 · 15 comments

Comments

@cedws
Copy link

cedws commented May 4, 2018

I have a basic project using actix-web, with a route, accepting a URL parameter named url.
When I call this route by requesting http://localhost:8080/%2F, the parameter is not decoded, and remains as "%2F" instead of "/".

server::new(|| App::new().resource("/{url}", |r| r.with(process)))
    .bind("0.0.0.0:8080")
    .unwrap()
    .start();

...

// RETURNS "%2F" NOT "/"
let url = req.match_info()
    .get("url")
    .unwrap();
@fafhrd91
Copy link
Member

fafhrd91 commented May 4, 2018

decoding / would change uri path, actix-web does not decode / and this is explicit decision.

@fafhrd91 fafhrd91 closed this as completed May 4, 2018
@cedws
Copy link
Author

cedws commented May 4, 2018

@fafhrd91 Could we discuss this before closing please? I have been using actix-web for a month or so now, and back when I first implemented it into the project, this feature was working.

Parts of a URL element with a "%" followed by two hexadecimal digits is intended to be decoded as text. It should not change the route of the URL itself. This is standard across all web frameworks I've used.

Has actix-web removed this feature because it wants users to do this decoding themselves?

Thanks.

@fafhrd91 fafhrd91 reopened this May 4, 2018
@fafhrd91
Copy link
Member

fafhrd91 commented May 4, 2018

problem with decoding /, it changes semantics. same for +.

@fafhrd91
Copy link
Member

fafhrd91 commented May 4, 2018

I think original behavior was wrong

@cedws
Copy link
Author

cedws commented May 4, 2018

@fafhrd91 I see. I've just switched my project over to use query parameters instead which is decoded correctly. I would prefer to use URL parameters but I respect that it may be difficult.

@fafhrd91
Copy link
Member

fafhrd91 commented May 4, 2018

you can manually do percent decoding.

something like /path/{url}

use percent_encoding::percent_decode;

fn index(data: Path<(String,)>) -> Result<HttpResponse> {
    let url = percent_decode(data.0)?;
    ....
}

#182

@cedws
Copy link
Author

cedws commented May 4, 2018

@fafhrd91 It's definitely a possible workaround. I see that you introduced a fix in #182, but I guess that the issue still persists. Please look into getting it working if you have the time 😄. Thank you.

@cedws cedws closed this as completed May 4, 2018
@fafhrd91
Copy link
Member

fafhrd91 commented May 4, 2018

what issue?

@fafhrd91
Copy link
Member

fafhrd91 commented May 4, 2018

@cedws
Copy link
Author

cedws commented May 4, 2018

@fafhrd91 From what I can see, #182 is referring to the same issue as I am. The %2F expansion into / causing routes to not match has been fixed, but the problem of not being able to use encoded characters in URL parameters still exists.

@fafhrd91
Copy link
Member

fafhrd91 commented May 4, 2018

I need test case. #182 introduced new Url type which decode percent encoding of utf8 symbols except /+

@cedws
Copy link
Author

cedws commented May 4, 2018

@fafhrd91 Ah yes, sorry, my bad. All characters appear to be getting decoded, except / and +, but I need to be able to have / decoded for my use case.

@fafhrd91
Copy link
Member

fafhrd91 commented May 4, 2018

example I posted above should work

@cedws
Copy link
Author

cedws commented May 4, 2018

@fafhrd91 Yes, but it's a workaround. Ideally actix-web should be able to do it standalone. I feel that excluding / and + from decoding is also inconsistent/unexpected behavior. I'm sure it must be possible to decode all characters.

@vimpunk
Copy link

vimpunk commented Sep 4, 2019

Having the same issue, would be good to get / and + decoded as text without changing the route.

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

3 participants