This repository has been archived by the owner on Jun 28, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Cleanup slices, strings, tags, and vecs (#44)
This started with enhancing tags, but when I was done I discovered a sigsev which could be triggered even from Rust code. I knew I had screwed up the lifetimes and unsafe code. So, I started working and I kept unravelling more and more. Slice and vec have been split into their own files as lib.rs was getting large. Many From traits were changed to work with &T instead of T because of lifetime issues. In this PR, some C FFI APIs for tags have been removed. A subsequent PR will add them back and enhance them. I wanted to keep the PR size to be somewhat manageable. Some places using some form of string have changed to use `Cow<'static, str>`. This allows you to borrow static strings, and own all others. When calling from C, the difference is very little because they _should_ have been copied (probably, was unsafe if not). I believe these are the changes which actually fixed the crash.
- Loading branch information
1 parent
e1bd12d
commit 9989036
Showing
8 changed files
with
593 additions
and
614 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| use std::borrow::Cow; | ||
| use std::fmt::{Debug, Display, Formatter}; | ||
|
|
||
| #[derive(Clone, Eq, PartialEq)] | ||
| pub struct Tag { | ||
| key: Cow<'static, str>, | ||
| value: Cow<'static, str>, | ||
| } | ||
|
|
||
| impl Debug for Tag { | ||
| fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { | ||
| f.debug_struct("Tag") | ||
| .field("key", &self.key) | ||
| .field("value", &self.value) | ||
| .finish() | ||
| } | ||
| } | ||
|
|
||
| // Any type which implements Display automatically has to_string. | ||
| impl Display for Tag { | ||
| fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { | ||
| // A tag isn't supposed to end with a colon, so if there isn't a value | ||
| // then don't follow the tag with a colon. | ||
| if self.value.is_empty() { | ||
| write!(f, "{}", self.key) | ||
| } else { | ||
| write!(f, "{}:{}", self.key, self.value) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl Tag { | ||
| pub fn new<IntoCow: Into<Cow<'static, str>>>( | ||
| key: IntoCow, | ||
| value: IntoCow, | ||
| ) -> Result<Self, Cow<'static, str>> { | ||
| let key = key.into(); | ||
| let value = value.into(); | ||
| if key.is_empty() { | ||
| return Err("tag key was empty".into()); | ||
| } | ||
|
|
||
| let first_valid_char = key | ||
| .chars() | ||
| .find(|char| *char != std::char::REPLACEMENT_CHARACTER && !char.is_whitespace()); | ||
|
|
||
| if first_valid_char.is_none() { | ||
| return Err("tag contained only whitespace or invalid unicode characters".into()); | ||
| } | ||
|
|
||
| Ok(Self { key, value }) | ||
| } | ||
|
|
||
| pub fn key(&self) -> &Cow<str> { | ||
| &self.key | ||
| } | ||
| pub fn value(&self) -> &Cow<str> { | ||
| &self.value | ||
| } | ||
|
|
||
| pub fn into_owned(mut self) -> Self { | ||
| self.key = self.key.to_owned(); | ||
| self.value = self.value.to_owned(); | ||
| self | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use crate::Tag; | ||
|
|
||
| #[test] | ||
| fn test_empty_key() { | ||
| let _ = Tag::new("", "woof").expect_err("empty key is not allowed"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_empty_value() { | ||
| let tag = Tag::new("key1", "").expect("empty value is okay"); | ||
| assert_eq!("key1", tag.to_string()); // notice no trailing colon! | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_bad_utf8() { | ||
| // 0b1111_0xxx is the start of a 4-byte sequence, but there aren't any | ||
| // more chars, so it will get converted into the utf8 replacement | ||
| // character. This results in a string with a space (32) and a | ||
| // replacement char, so it should be an error (no valid chars). | ||
| let bytes = &[32, 0b1111_0111]; | ||
| let key = String::from_utf8_lossy(bytes); | ||
| let _ = Tag::new(key, "value".into()).expect_err("invalid tag is rejected"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_value_has_colon() { | ||
| let result = Tag::new("env", "staging:east").expect("values can have colons"); | ||
| assert_eq!("env:staging:east", result.to_string()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_suspicious_tags() { | ||
| // Based on tag rules, these should all fail. However, there is a risk | ||
| // that profile tags will then differ or cause failures compared to | ||
| // trace tags. These require cross-team, cross-language collaboration. | ||
| let cases = [ | ||
| (":key-starts-with-colon".to_string(), "value".to_owned()), | ||
| ("key".to_string(), "value-ends-with-colon:".to_owned()), | ||
| ( | ||
| "the-tag-length-is-over-200-characters".repeat(6), | ||
| "value".to_owned(), | ||
| ), | ||
| ]; | ||
|
|
||
| for case in cases { | ||
| let result = Tag::new(case.0, case.1); | ||
| // Again, these should fail, but it's not implemented yet | ||
| assert!(result.is_ok()) | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.