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

Parse AbsolutePath further #298

Closed
mikedilger opened this issue Feb 7, 2015 · 5 comments
Closed

Parse AbsolutePath further #298

mikedilger opened this issue Feb 7, 2015 · 5 comments

Comments

@mikedilger
Copy link
Contributor

See https://github.com/mikedilger/hyper/tree/parse_absolute_path for a possible way forward.

That branch offers several benefits, but also has a few problems:

Benefits:

  1. Path and Query are separated. BTW, example/server.rs doesn't show how a consumer might deal with this separation of Path and Query, consumers of hyper are left to fend for themselves.
  2. Fragment, if it exists, is parsed separately and thus not stuck onto the end of Query. The HTTP standard assumes no fragment, but word is that servers do this in practice.
  3. Inherits the encoding related checks of the URL library

Problems:

  1. I don't like that url::parse_path() parses the path component into a Vec. I actually find that less useful than the original String.
  2. One test fails, due to (I think) an issue with url::parse_path() which parses "/" into a vec![""] rather than a vec![](which seems inconsistent)
@pyfisch
Copy link
Contributor

pyfisch commented Feb 7, 2015

Would it not be better if the PathQueryFragment would be a url::Url? (I know that this not work well currently because you have to provide a host, but could not we provide a default host?

@mikedilger
Copy link
Contributor Author

Could do that. But a Url struct would add 6 fields that wouldn't be used: scheme, username, password, host, port, and default port. And I'm not sure if hyper would always know its hostname in every circumstance. Also, the Url::parse_path() output seems to be geared for this usage, only there is no struct, it just returns a 3-tuple. Could take that approach too (i.e. drop the struct).

@seanmonstar
Copy link
Member

Maybe having accessors on the Request would be for the best:

  • fn path(&self) -> Option<&str>
  • fn query(&self) -> Option<&str>

@mikedilger
Copy link
Contributor Author

Sounds good.

@seanmonstar
Copy link
Member

Filed issues to do this in #896 and #897

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