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

Redesign the Uri type #1000

Closed
seanmonstar opened this issue Jan 12, 2017 · 0 comments
Closed

Redesign the Uri type #1000

seanmonstar opened this issue Jan 12, 2017 · 0 comments
Milestone

Comments

@seanmonstar
Copy link
Member

There are a couple of things that a redesign could help with:

  • Making it an opaque struct instead of an enum means fiddling with internals is no longer a breaking change
  • It'd be best if we could create less copies when parsing out a URI
  • To future-proof for when hyper supports HTTP2

We want to optimize for the most common case, which is the origin-form, (currently RequestUri::AbsolutePath). We can start with keeping a single String, and then keeping indices into it to know where the path and query are.

In HTTP2, the URI would gain additional fields, scheme and authority (what is in HTTP1 the Host header), and the asterisk-form request target is defined as path = *.

We could eventually change the internal source representation to be a MemSlice or whatever it is at that point, to reduce a further copy.

Some quick code describing what it could look like:

pub struct Uri {
    source: String,
    scheme_end: Option<usize>,
    authority_end: Option<usize>,
    query: Option<usize>,
}

impl Uri {
    pub fn path(&self) -> &str {
        let index = self.scheme_end.unwrap_or(0) + self.authority_end.unwrap_or(0);
        let end = self.query.unwrap_or(self.source.len());
        &self.source[index..end]
    }
    // .. and other accessors
}

let star = Uri::from_str("*").unwrap();
assert_eq!(Uri {
    source: String::from("*"),
    scheme_end: None,
    authority_end: None,
    query: None,
}, star);
assert_eq!(star.path(), "*");

let common = Uri::from_str("/p?foo=bar").unwrap();
assert_eq!(Uri {
    source: String::from("/p?foo=bar"),
    scheme_end: None,
    authority_end: None,
    query: Some(2),
}, common);
assert_eq!(common.path(), "/p");
assert_eq!(common.query(), "?foo=bar");

cc @GuillaumeGomez

@seanmonstar seanmonstar added this to the 0.11 milestone Jan 12, 2017
seanmonstar pushed a commit that referenced this issue Jan 18, 2017
Closes #1000

BREAKING CHANGE: The name of `RequestUri` has changed to `Uri`. It is no
  longer an `enum`, but an opaque struct with getter methods.
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

1 participant