Merge remote-tracking branch 'isis/bug25185'

4 files changed, 161 insertions, 123 deletions

diff --git a/src/rust/protover/ffi.rs b/src/rust/protover/ffi.rs
index 2ee0286ecf..e4cf0a7e88 100644
--- a/src/rust/protover/ffi.rs
+++ b/src/rust/protover/ffi.rs
@@ -12,9 +12,6 @@ use std::ffi::CString;
 use protover::*;
 use smartlist::*;
 use tor_allocate::allocate_and_copy_string;
-use tor_util::strings::byte_slice_is_c_like;
-use tor_util::strings::empty_static_cstr;
-
 
 /// Translate C enums to Rust Proto enums, using the integer value of the C
 /// enum to map to its associated Rust enum
@@ -143,18 +140,7 @@ pub extern "C" fn protocol_list_supports_protocol_or_later(
 pub extern "C" fn protover_get_supported_protocols() -> *const c_char {
     let supported: &'static CStr;
 
-    // If we're going to pass it to C, there cannot be any intermediate NUL
-    // bytes.  An assert is okay here, since changing the const byte slice
-    // in protover.rs to contain a NUL byte somewhere in the middle would be a
-    // programming error.
-    assert!(byte_slice_is_c_like(SUPPORTED_PROTOCOLS));
-
-    // It's okay to unwrap the result of this function because
-    // we can see that the bytes we're passing into it 1) are valid UTF-8,
-    // 2) have no intermediate NUL bytes, and 3) are terminated with a NUL
-    // byte.
-    supported = CStr::from_bytes_with_nul(SUPPORTED_PROTOCOLS).unwrap();
-
+    supported = get_supported_protocols_cstr();
     supported.as_ptr()
 }
 
@@ -202,10 +188,9 @@ pub extern "C" fn protover_is_supported_here(
 #[no_mangle]
 pub extern "C" fn protover_compute_for_old_tor(version: *const c_char) -> *const c_char {
     let supported: &'static CStr;
-    let elder_protocols: &'static [u8];
     let empty: &'static CStr;
 
-    empty = empty_static_cstr();
+    empty = cstr!("");
 
     if version.is_null() {
         return empty.as_ptr();
@@ -220,19 +205,6 @@ pub extern "C" fn protover_compute_for_old_tor(version: *const c_char) -> *const
         Err(_) => return empty.as_ptr(),
     };
 
-    elder_protocols = compute_for_old_tor(&version);
-
-    // If we're going to pass it to C, there cannot be any intermediate NUL
-    // bytes.  An assert is okay here, since changing the const byte slice
-    // in protover.rs to contain a NUL byte somewhere in the middle would be a
-    // programming error.
-    assert!(byte_slice_is_c_like(elder_protocols));
-
-    // It's okay to unwrap the result of this function because
-    // we can see that the bytes we're passing into it 1) are valid UTF-8,
-    // 2) have no intermediate NUL bytes, and 3) are terminated with a NUL
-    // byte.
-    supported = CStr::from_bytes_with_nul(elder_protocols).unwrap_or(empty);
-
+    supported = compute_for_old_tor(&version);
     supported.as_ptr()
 }
diff --git a/src/rust/protover/lib.rs b/src/rust/protover/lib.rs
index fe8c0f9bb2..8e80fcef4c 100644
--- a/src/rust/protover/lib.rs
+++ b/src/rust/protover/lib.rs
@@ -26,6 +26,7 @@ extern crate libc;
 extern crate smartlist;
 extern crate external;
 extern crate tor_allocate;
+#[macro_use]
 extern crate tor_util;
 
 mod protover;
diff --git a/src/rust/protover/protover.rs b/src/rust/protover/protover.rs
index e5dc69b9a0..03776411ca 100644
--- a/src/rust/protover/protover.rs
+++ b/src/rust/protover/protover.rs
@@ -5,14 +5,13 @@ use external::c_tor_version_as_new_as;
 
 use std::str;
 use std::str::FromStr;
+use std::ffi::CStr;
 use std::fmt;
 use std::collections::{HashMap, HashSet};
 use std::ops::Range;
 use std::string::String;
 use std::u32;
 
-use tor_util::strings::NUL_BYTE;
-
 /// The first version of Tor that included "proto" entries in its descriptors.
 /// Authorities should use this to decide whether to guess proto lines.
 ///
@@ -26,30 +25,6 @@ const FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS: &'static str = "0.2.9.3-alpha";
 /// C_RUST_COUPLED: src/or/protover.c `MAX_PROTOCOLS_TO_EXPAND`
 const MAX_PROTOCOLS_TO_EXPAND: usize = (1<<16);
 
-/// Currently supported protocols and their versions, as a byte-slice.
-///
-/// # Warning
-///
-/// This byte-slice ends in a NUL byte.  This is so that we can directly convert
-/// it to an `&'static CStr` in the FFI code, in order to hand the static string
-/// to C in a way that is compatible with C static strings.
-///
-/// Rust code which wishes to accesses this string should use
-/// `protover::get_supported_protocols()` instead.
-///
-/// C_RUST_COUPLED: src/or/protover.c `protover_get_supported_protocols`
-pub(crate) const SUPPORTED_PROTOCOLS: &'static [u8] =
-    b"Cons=1-2 \
-    Desc=1-2 \
-    DirCache=1-2 \
-    HSDir=1-2 \
-    HSIntro=3-4 \
-    HSRend=1-2 \
-    Link=1-5 \
-    LinkAuth=1,3 \
-    Microdesc=1-2 \
-    Relay=1-2\0";
-
 /// Known subprotocols in Tor. Indicates which subprotocol a relay supports.
 ///
 /// C_RUST_COUPLED: src/or/protover.h `protocol_type_t`
@@ -97,21 +72,51 @@ impl FromStr for Proto {
     }
 }
 
-/// Get the string representation of current supported protocols
+/// Get a CStr representation of current supported protocols, for
+/// passing to C, or for converting to a `&str` for Rust.
 ///
 /// # Returns
 ///
-/// A `String` whose value is the existing protocols supported by tor.
+/// An `&'static CStr` whose value is the existing protocols supported by tor.
 /// Returned data is in the format as follows:
 ///
 /// "HSDir=1-1 LinkAuth=1"
 ///
-pub fn get_supported_protocols() -> &'static str {
-    // The `len() - 1` is to remove the NUL byte.
-    // The `unwrap` is safe becauase we SUPPORTED_PROTOCOLS is under
-    // our control.
-    str::from_utf8(&SUPPORTED_PROTOCOLS[..SUPPORTED_PROTOCOLS.len() - 1])
-        .unwrap_or("")
+/// # Note
+///
+/// Rust code can use the `&'static CStr` as a normal `&'a str` by
+/// calling `protover::get_supported_protocols`.
+///
+//  C_RUST_COUPLED: src/or/protover.c `protover_get_supported_protocols`
+pub(crate) fn get_supported_protocols_cstr() -> &'static CStr {
+    cstr!("Cons=1-2 \
+           Desc=1-2 \
+           DirCache=1-2 \
+           HSDir=1-2 \
+           HSIntro=3-4 \
+           HSRend=1-2 \
+           Link=1-5 \
+           LinkAuth=1,3 \
+           Microdesc=1-2 \
+           Relay=1-2")
+}
+
+/// Get a string representation of current supported protocols.
+///
+/// # Returns
+///
+/// An `&'a str` whose value is the existing protocols supported by tor.
+/// Returned data is in the format as follows:
+///
+/// "HSDir=1-1 LinkAuth=1"
+pub fn get_supported_protocols<'a>() -> &'a str {
+    let supported_cstr: &'static CStr = get_supported_protocols_cstr();
+    let supported: &str = match supported_cstr.to_str() {
+        Ok(x)  => x,
+        Err(_) => "",
+    };
+
+    supported
 }
 
 pub struct SupportedProtocols(HashMap<Proto, Versions>);
@@ -754,7 +759,7 @@ pub fn is_supported_here(proto: Proto, vers: Version) -> bool {
 ///
 /// # Returns
 ///
-/// A `&'static [u8]` encoding a list of protocol names and supported
+/// A `&'static CStr` encoding a list of protocol names and supported
 /// versions. The string takes the following format:
 ///
 /// "HSDir=1-1 LinkAuth=1"
@@ -763,27 +768,29 @@ pub fn is_supported_here(proto: Proto, vers: Version) -> bool {
 /// only for tor versions older than FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS.
 ///
 /// C_RUST_COUPLED: src/rust/protover.c `compute_for_old_tor`
-pub fn compute_for_old_tor(version: &str) -> &'static [u8] {
+pub fn compute_for_old_tor(version: &str) -> &'static CStr {
+    let empty: &'static CStr = cstr!("");
+
     if c_tor_version_as_new_as(version, FIRST_TOR_VERSION_TO_ADVERTISE_PROTOCOLS) {
-        return NUL_BYTE;
+        return empty;
     }
 
     if c_tor_version_as_new_as(version, "0.2.9.1-alpha") {
-        return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 \
-                 Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0";
+        return cstr!("Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1-2 \
+                      Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2");
     }
 
     if c_tor_version_as_new_as(version, "0.2.7.5") {
-        return b"Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
-                 Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2\0";
+        return cstr!("Cons=1-2 Desc=1-2 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
+                      Link=1-4 LinkAuth=1 Microdesc=1-2 Relay=1-2");
     }
 
     if c_tor_version_as_new_as(version, "0.2.4.19") {
-        return b"Cons=1 Desc=1 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
-                 Link=1-4 LinkAuth=1 Microdesc=1 Relay=1-2\0";
+        return cstr!("Cons=1 Desc=1 DirCache=1 HSDir=1 HSIntro=3 HSRend=1 \
+                      Link=1-4 LinkAuth=1 Microdesc=1 Relay=1-2");
     }
 
-    NUL_BYTE
+    empty
 }
 
 #[cfg(test)]
diff --git a/src/rust/tor_util/strings.rs b/src/rust/tor_util/strings.rs
index 9321ce4f85..4e26e7369c 100644
--- a/src/rust/tor_util/strings.rs
+++ b/src/rust/tor_util/strings.rs
@@ -3,80 +3,138 @@
 
 //! Utilities for working with static strings.
 
-use std::ffi::CStr;
-
-/// A byte-array containing a single NUL byte (`b"\0"`).
-pub const NUL_BYTE: &'static [u8] = b"\0";
-
-/// Determine if a byte slice is a C-like string.
-///
-/// These checks guarantee that:
-///
-/// 1. there are no intermediate NUL bytes
-/// 2. the last byte *is* a NUL byte
+/// Create a `CStr` from a literal byte slice, appending a NUL byte to it first.
 ///
 /// # Warning
 ///
-/// This function does _not_ guarantee that the bytes represent any valid
-/// encoding such as ASCII or UTF-8.
+/// The literal byte slice which is taken as an argument *MUST NOT* have any NUL
+/// bytes (`b"\0"`) in it, anywhere, or else an empty string will be returned
+/// (`CStr::from_bytes_with_nul_unchecked(b"\0")`) so as to avoid `panic!()`ing.
 ///
 /// # Examples
 ///
 /// ```
-/// # use tor_util::strings::byte_slice_is_c_like;
-/// #
-/// let bytes: &[u8] = b"foo bar baz";
+/// #[macro_use]
+/// extern crate tor_util;
 ///
-/// assert!(byte_slice_is_c_like(&bytes) == false);
+/// use std::ffi::CStr;
 ///
-/// let bytes: &[u8] = b"foo\0bar baz";
+/// # fn do_test() -> Result<&'static CStr, &'static str> {
+/// let message: &'static str = "This is a test of the tsunami warning system.";
+/// let tuesday: &'static CStr;
+/// let original: &str;
 ///
-/// assert!(byte_slice_is_c_like(&bytes) == false);
+/// tuesday = cstr!("This is a test of the tsunami warning system.");
+/// original = tuesday.to_str().or(Err("Couldn't unwrap CStr!"))?;
 ///
-/// let bytes: &[u8] = b"foo bar baz\0";
+/// assert!(original == message);
+/// #
+/// # Ok(tuesday)
+/// # }
+/// # fn main() {
+/// #     do_test();  // so that we can use the ? operator in the test
+/// # }
+/// ```
+/// It is also possible to pass several string literals to this macro.  They
+/// will be concatenated together in the order of the arguments, unmodified,
+/// before finally being suffixed with a NUL byte:
 ///
-/// assert!(byte_slice_is_c_like(&bytes) == true);
 /// ```
-pub fn byte_slice_is_c_like(bytes: &[u8]) -> bool {
-    if !bytes[..bytes.len() - 1].contains(&0x00) && bytes[bytes.len() - 1] == 0x00 {
-        return true;
-    }
-    false
-}
-
-/// Get a static `CStr` containing a single `NUL_BYTE`.
+/// #[macro_use]
+/// extern crate tor_util;
+/// #
+/// # use std::ffi::CStr;
+/// #
+/// # fn do_test() -> Result<&'static CStr, &'static str> {
 ///
-/// # Examples
+/// let quux: &'static CStr = cstr!("foo", "bar", "baz");
+/// let orig: &'static str = quux.to_str().or(Err("Couldn't unwrap CStr!"))?;
 ///
-/// When used as follows in a Rust FFI function, which could be called
-/// from C:
+/// assert!(orig == "foobarbaz");
+/// # Ok(quux)
+/// # }
+/// # fn main() {
+/// #     do_test();  // so that we can use the ? operator in the test
+/// # }
+/// ```
+/// This is useful for passing static strings to C from Rust FFI code.  To do so
+/// so, use the `.as_ptr()` method on the resulting `&'static CStr` to convert
+/// it to the Rust equivalent of a C `const char*`:
 ///
 /// ```
-/// # extern crate libc;
-/// # extern crate tor_util;
-/// #
-/// # use tor_util::strings::empty_static_cstr;
-/// use libc::c_char;
+/// #[macro_use]
+/// extern crate tor_util;
+///
 /// use std::ffi::CStr;
+/// use std::os::raw::c_char;
 ///
-/// pub extern "C" fn give_c_code_an_empty_static_string() -> *const c_char {
-///     let empty: &'static CStr = empty_static_cstr();
+/// pub extern "C" fn give_static_borrowed_string_to_c() -> *const c_char {
+///     let hello: &'static CStr = cstr!("Hello, language my parents wrote.");
 ///
-///     empty.as_ptr()
+///     hello.as_ptr()
 /// }
-///
 /// # fn main() {
-/// #     give_c_code_an_empty_static_string();
+/// #     let greetings = give_static_borrowed_string_to_c();
 /// # }
 /// ```
+/// Note that the C code this static borrowed string is passed to *MUST NOT*
+/// attempt to free the memory for the string.
+///
+/// # Note
+///
+/// An unfortunate limitation of the rustc compiler (as of 1.25.0-nightly), is
+/// that the above code compiles, however if we were to change the assignment of
+/// `tuesday` as follows, it will fail to compile, because Rust macros are
+/// expanded at parse time, and at parse time there is no symbols table
+/// available.
 ///
-/// This equates to an "empty" `const char*` static string in C.
-pub fn empty_static_cstr() -> &'static CStr {
-    let empty: &'static CStr;
+/// ```ignore
+/// tuesday = cstr!(message);
+/// ```
+/// with the error message `error: expected a literal`.
+///
+/// # Returns
+///
+/// If the string literals passed as arguments contain no NUL bytes anywhere,
+/// then an `&'static CStr` containing the (concatenated) bytes of the string
+/// literal(s) passed as arguments, with a NUL byte appended, is returned.
+/// Otherwise, an `&'static CStr` containing a single NUL byte is returned (an
+/// "empty" string in C).
+#[macro_export]
+macro_rules! cstr {
+    ($($bytes:expr),*) => (
+        ::std::ffi::CStr::from_bytes_with_nul(
+            concat!($($bytes),*, "\0").as_bytes()
+        ).unwrap_or(
+            unsafe{
+                ::std::ffi::CStr::from_bytes_with_nul_unchecked(b"\0")
+            }
+        )
+    )
+}
+
+#[cfg(test)]
+mod test {
+    use std::ffi::CStr;
+
+    #[test]
+    fn cstr_macro() {
+        let _: &'static CStr = cstr!("boo");
+    }
+
+    #[test]
+    fn cstr_macro_multi_input() {
+        let quux: &'static CStr = cstr!("foo", "bar", "baz");
 
-    unsafe {
-        empty = CStr::from_bytes_with_nul_unchecked(NUL_BYTE);
+        assert!(quux.to_str().unwrap() == "foobarbaz");
     }
 
-    empty
+    #[test]
+    fn cstr_macro_bad_input() {
+        let waving:   &'static CStr = cstr!("waving not drowning o/");
+        let drowning: &'static CStr = cstr!("\0 drowning not waving");
+
+        assert!(waving.to_str().unwrap()   == "waving not drowning o/");
+        assert!(drowning.to_str().unwrap() == "")
+    }
 }