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 Method::from_static #595

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 118 additions & 0 deletions src/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
//! assert_eq!(Method::POST.as_str(), "POST");
//! ```

use extension::StaticExtension;

use self::extension::{AllocatedExtension, InlineExtension};
use self::Inner::*;

Expand Down Expand Up @@ -64,6 +66,8 @@ enum Inner {
ExtensionInline(InlineExtension),
// Otherwise, allocate it
ExtensionAllocated(AllocatedExtension),
// Statically allocated data
ExtensionStatic(StaticExtension),
}

impl Method {
Expand Down Expand Up @@ -134,6 +138,30 @@ impl Method {
}
}

/// Convert static bytes into a `Method`.
pub const fn from_static(src: &'static [u8]) -> Method {
match src {
b"OPTIONS" => Method::OPTIONS,
b"GET" => Method::GET,
b"POST" => Method::POST,
b"PUT" => Method::PUT,
b"DELETE" => Method::DELETE,
b"HEAD" => Method::HEAD,
b"TRACE" => Method::TRACE,
b"CONNECT" => Method::CONNECT,
b"PATCH" => Method::PATCH,
src => {
if src.len() <= 15 {
let inline = InlineExtension::from_static(src);
Copy link
Member

Choose a reason for hiding this comment

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

With an ExtensionStatic available, is there a benefit to keeping this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

InlineExtension has no heap allocation and has the data available in the same allocation. Most HTTP methods fit in this case, and it has no overhead.

ExtensionStatic is a niche for unusually long methods, but has a heap allocation and an extra indirection.

We have three options here:

  • Keep both. No real downsides aside from another internal type, and does not increase the size of the Method type.
  • Keep only InlineExtension, restricting from_static to only methods of len <= 15.
  • Keep only ExtensionStatic, forcing any from_static method to use a heap allocation and an extra indirection.

Personally, I prefer the first choice the most, and the third one the least (since it introduces pointless overhead for most WebDAV methods, albeit minor).

Method(ExtensionInline(inline))
} else {
let allocated = StaticExtension::from_static(src);
Method(ExtensionStatic(allocated))
}
}
}
}

fn extension_inline(src: &[u8]) -> Result<Method, InvalidMethod> {
let inline = InlineExtension::new(src)?;

Expand Down Expand Up @@ -176,6 +204,7 @@ impl Method {
Patch => "PATCH",
ExtensionInline(ref inline) => inline.as_str(),
ExtensionAllocated(ref allocated) => allocated.as_str(),
ExtensionStatic(ref s) => s.as_str(),
}
}
}
Expand Down Expand Up @@ -316,6 +345,9 @@ mod extension {
// Invariant: self.0 contains valid UTF-8.
pub struct AllocatedExtension(Box<[u8]>);

#[derive(Clone, PartialEq, Eq, Hash)]
pub struct StaticExtension(&'static [u8]);

impl InlineExtension {
// Method::from_bytes() assumes this is at least 7
pub const MAX: usize = 15;
Expand All @@ -330,6 +362,34 @@ mod extension {
Ok(InlineExtension(data, src.len() as u8))
}

/// Convert static bytes into an `InlineExtension`.
///
/// # Panics
///
/// If the input bytes are not a valid method name or if the method name is over 15 bytes.
pub const fn from_static(src: &'static [u8]) -> InlineExtension {
let mut i = 0;
let mut dst = [0u8; 15];
if src.len() > 15 {
// panicking in const requires Rust 1.57.0
#[allow(unconditional_panic)]
([] as [u8; 0])[0];
}
while i < src.len() {
let byte = src[i];
let v = METHOD_CHARS[byte as usize];
if v == 0 {
// panicking in const requires Rust 1.57.0
#[allow(unconditional_panic)]
([] as [u8; 0])[0];
}
dst[i] = byte;
i += 1;
}

InlineExtension(dst, i as u8)
}

pub fn as_str(&self) -> &str {
let InlineExtension(ref data, len) = self;
// Safety: the invariant of InlineExtension ensures that the first
Expand All @@ -356,6 +416,32 @@ mod extension {
}
}

impl StaticExtension {
pub const fn from_static(src: &'static [u8]) -> StaticExtension {
let mut i = 0;
while i < src.len() {
let byte = src[i];
let v = METHOD_CHARS[byte as usize];
if v == 0 {
// panicking in const requires Rust 1.57.0
#[allow(unconditional_panic)]
([] as [u8; 0])[0];
}
i += 1;
}

// Invariant: data is exactly src.len() long and write_checked
// ensures that the first src.len() bytes of data are valid UTF-8.
StaticExtension(src)
}

pub fn as_str(&self) -> &str {
// Safety: the invariant of StaticExtension ensures that self.0
// contains valid UTF-8.
unsafe { str::from_utf8_unchecked(&self.0) }
}
}

// From the RFC 9110 HTTP Semantics, section 9.1, the HTTP method is case-sensitive and can
// contain the following characters:
//
Expand Down Expand Up @@ -436,6 +522,38 @@ mod test {
assert_eq!(Method::GET, &Method::GET);
}

#[test]
fn test_from_static() {
seanmonstar marked this conversation as resolved.
Show resolved Hide resolved
// First class variant
assert_eq!(
Method::from_static(b"GET"),
Method::from_bytes(b"GET").unwrap()
);
// Inline, len < 15
assert_eq!(
Method::from_static(b"PROPFIND"),
Method::from_bytes(b"PROPFIND").unwrap()
);
// Inline, len == 15
assert_eq!(Method::from_static(b"GET"), Method::GET);
assert_eq!(
Method::from_static(b"123456789012345").to_string(),
"123456789012345".to_string()
);
// Ref, len > 15
Method::from_static(b"1234567890123456");
assert_eq!(
Method::from_static(b"1234567890123456").to_string(),
"1234567890123456".to_string()
);
}

#[test]
#[should_panic]
fn test_from_static_bad() {
Method::from_static(b"\0");
}

#[test]
fn test_invalid_method() {
assert!(Method::from_str("").is_err());
Expand Down
Loading