header name: add from_static method #195
Conversation
|
Nightly build breaking on |
| /// | ||
| /// // Parsing a header that contains invalid uppercase characters. | ||
| /// let a = HeaderName::from_static("foobar"); | ||
| /// let b = HeaderName::from_static("FOOBAR"); // This line panics! |
There was a problem hiding this comment.
It does? Looking at the implementation, it looks like the block with lower: false will still parse successfully. I might not understand all the details of this file, though...
There was a problem hiding this comment.
It is a little weird, but should be correct with the same checking method in from_lower.
However, I agree it is unclear and can be improved.
src/header/name.rs
Outdated
| let b = HeaderName::from_static("vaary"); | ||
| assert_ne!(a, b); | ||
|
|
||
| let result = panic::catch_unwind(|| HeaderName::from_static("Vary")); |
There was a problem hiding this comment.
What if each of these that should panic were separate #[test] #[should_panic] fn foo() {} test cases, so that catch_unwind wouldn't be needed? I think it might be clearer in reading. For example:
#[test]
#[should_panic]
fn from_static_uppercase() {
HeaderName::from_static("Vary");
}Since this would make a bunch of test functions, it might also be a good idea to push these into an inner #[cfg(test)] mod tests { ... }. Whatcha think?
There was a problem hiding this comment.
Definitely agree, and the other tests could also be moved into the block too.
|
No worries, and thanks for all your work on Hyperium! |
Implements: #168 :
HeaderName::from_staticConverts a static string to a HTTP header name.
Function panics on an invalid headers, and requires lowercase characters, as so the static string will have the same format as the internal format.
Function should also be zero copy, beyond what is done in parse_hdr, with the static slice being used for the
ByteStrin custom headers.