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

Add and use a Port struct for Uri::port() #252

Merged
merged 2 commits into from Sep 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 19 additions & 12 deletions src/uri/authority.rs
Expand Up @@ -8,7 +8,7 @@ use std::str::FromStr;
use bytes::Bytes;

use byte_str::ByteStr;
use super::{ErrorKind, InvalidUri, InvalidUriBytes, URI_CHARS};
use super::{ErrorKind, InvalidUri, InvalidUriBytes, URI_CHARS, Port};

/// Represents the authority component of a URI.
#[derive(Clone)]
Expand Down Expand Up @@ -180,12 +180,18 @@ impl Authority {
host(self.as_str())
}

/// Get the port of this `Authority`.
#[deprecated(since="0.2.0", note="please use `port_part` instead")]
#[doc(hidden)]
pub fn port(&self) -> Option<u16> {
self.port_part().and_then(|p| Some(p.as_u16()))
}

/// Get the port part of this `Authority`.
///
/// The port subcomponent of authority is designated by an optional port
/// number in decimal following the host and delimited from it by a single
/// colon (":") character. A value is only returned if one is specified in
/// the URI string, i.e., default port values are **not** returned.
/// number following the host and delimited from it by a single colon (":")
/// character. It can be turned into a decimal port number with the `as_u16`
/// method or as a `str` with the `as_str` method.
///
/// ```notrust
/// abc://username:password@example.com:123/path/data?key=value&key2=value2#fragid1
Expand All @@ -202,7 +208,8 @@ impl Authority {
/// # use http::uri::Authority;
/// let authority: Authority = "example.org:80".parse().unwrap();
///
/// assert_eq!(authority.port(), Some(80));
/// let port = authority.port_part().unwrap();
/// assert_eq!(port.as_u16(), 80);
/// ```
///
/// Authority without port
Expand All @@ -211,13 +218,13 @@ impl Authority {
/// # use http::uri::Authority;
/// let authority: Authority = "example.org".parse().unwrap();
///
/// assert!(authority.port().is_none());
/// assert!(authority.port_part().is_none());
/// ```
pub fn port(&self) -> Option<u16> {
let s = self.as_str();
s.rfind(":").and_then(|i| {
u16::from_str(&s[i+1..]).ok()
})
pub fn port_part(&self) -> Option<Port> {
let bytes = self.as_str();
bytes
.rfind(":")
.and_then(|i| Port::from_str(&bytes[i + 1..]).ok())
}

/// Return a str representation of the authority
Expand Down
29 changes: 19 additions & 10 deletions src/uri/mod.rs
Expand Up @@ -40,9 +40,11 @@ use self::scheme::Scheme2;
pub use self::authority::Authority;
pub use self::path::PathAndQuery;
pub use self::scheme::Scheme;
pub use self::port::Port;

mod authority;
mod path;
mod port;
mod scheme;
#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -543,12 +545,18 @@ impl Uri {
self.authority_part().map(|a| a.host())
}

/// Get the port of this `Uri`.
#[deprecated(since="0.2.0", note="please use `port_part` instead")]
#[doc(hidden)]
pub fn port(&self) -> Option<u16> {
self.port_part().and_then(|p| Some(p.as_u16()))
}

/// Get the port part of this `Uri`.
///
/// The port subcomponent of authority is designated by an optional port
/// number in decimal following the host and delimited from it by a single
/// colon (":") character. A value is only returned if one is specified in
/// the URI string, i.e., default port values are **not** returned.
/// number following the host and delimited from it by a single colon (":")
/// character. It can be turned into a decimal port number with the `as_u16`
/// method or as a `str` with the `as_str` method.
///
/// ```notrust
/// abc://username:password@example.com:123/path/data?key=value&key2=value2#fragid1
Expand All @@ -562,10 +570,11 @@ impl Uri {
/// Absolute URI with port
///
/// ```
/// # use http::Uri;
/// # use http::{Uri, uri::Port};
/// let uri: Uri = "http://example.org:80/hello/world".parse().unwrap();
///
/// assert_eq!(uri.port(), Some(80));
/// let port = uri.port_part().unwrap();
/// assert_eq!(port.as_u16(), 80);
/// ```
///
/// Absolute URI without port
Expand All @@ -574,7 +583,7 @@ impl Uri {
/// # use http::Uri;
/// let uri: Uri = "http://example.org/hello/world".parse().unwrap();
///
/// assert!(uri.port().is_none());
/// assert!(uri.port_part().is_none());
/// ```
///
/// Relative URI
Expand All @@ -583,11 +592,11 @@ impl Uri {
/// # use http::Uri;
/// let uri: Uri = "/hello/world".parse().unwrap();
///
/// assert!(uri.port().is_none());
/// assert!(uri.port_part().is_none());
/// ```
pub fn port(&self) -> Option<u16> {
pub fn port_part(&self) -> Option<Port> {
self.authority_part()
.and_then(|a| a.port())
.and_then(|a| a.port_part())
}

/// Get the query string of this `Uri`, starting after the `?`.
Expand Down
64 changes: 64 additions & 0 deletions src/uri/port.rs
@@ -0,0 +1,64 @@
use std::{fmt, num};
use std::str::FromStr;

/// The port component of a URI.
#[derive(Debug)]
pub struct Port<'a> {
bytes: &'a str,
port: u16,
}

impl<'a> Port<'a> {
/// Converts a `str` to a port number.
///
/// The supplied `str` must be a valid u16.
pub(crate) fn from_str(bytes: &'a str) -> Result<Self, num::ParseIntError> {
u16::from_str(bytes).and_then(|port| Ok(Port { port, bytes }))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BNF of a URI port is *DIGIT, see https://tools.ietf.org/html/rfc3986#section-3.2.3. However from_str allows an optional + sign, see https://doc.rust-lang.org/nightly/std/primitive.u16.html#method.from_str_radix.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory, this should already have been validated when constructing the Uri, so checking again here is not necessary.

}

/// Returns the port number as a `u16`.
///
/// # Examples
///
/// Port as `u16`
///
/// ```
/// # use http::uri::Authority;
/// let authority: Authority = "example.org:80".parse().unwrap();
///
/// let port = authority.port_part().unwrap();
/// assert_eq!(port.as_u16(), 80);
/// ```
pub fn as_u16(&self) -> u16 {
self.port
}

/// Returns the port number as a `str`.
///
/// # Examples
///
/// Port as `str`.
///
/// ```
/// # use http::uri::Authority;
/// let authority: Authority = "example.org:80".parse().unwrap();
///
/// let port = authority.port_part().unwrap();
/// assert_eq!(port.as_str(), "80");
/// ```
pub fn as_str(&self) -> &'a str {
self.bytes
}
}

impl<'a> PartialEq for Port<'a> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required for this PR, but it would be nice to add PartialEq implementations for numeric types (u16, u32, ...).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll put up a subsequent PR adding a generic implementation for PartialEq 😄

fn eq(&self, other: &Self) -> bool {
self.port == other.port
}
}

impl<'a> fmt::Display for Port<'a> {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
write!(fmt, "{}", self.bytes)
}
}