diff --git a/crates/objc2/src/exception.rs b/crates/objc2/src/exception.rs index c0f23ccd4..d5b414776 100644 --- a/crates/objc2/src/exception.rs +++ b/crates/objc2/src/exception.rs @@ -113,7 +113,15 @@ impl fmt::Debug for Exception { // SAFETY: Just checked that object is an NSException let (name, reason) = unsafe { (self.name(), self.reason()) }; - // SAFETY: `name` and `reason` are guaranteed to be NSString. + // SAFETY: + // - `name` and `reason` are guaranteed to be `NSString`s. + // - We control the scope in which they are alive, so we know + // they are not moved outside the current autorelease pool. + // + // Note that these strings are immutable (`NSException` is + // immutable, and the properties are marked as `readonly` and + // `copy` and are copied upon creation), so we also don't have + // to worry about the string being mutated under our feet. let name = name .as_deref() .map(|name| unsafe { nsstring_to_str(name, pool) }); @@ -145,7 +153,7 @@ impl fmt::Display for Exception { let reason = unsafe { self.reason() }; if let Some(reason) = &reason { - // SAFETY: `reason` is guaranteed to be NSString. + // SAFETY: Same as above in `Debug`. let reason = unsafe { nsstring_to_str(reason, pool) }; return write!(f, "{reason}"); } diff --git a/crates/objc2/src/runtime/__nsstring.rs b/crates/objc2/src/runtime/__nsstring.rs index d7c5adf4b..f99206da0 100644 --- a/crates/objc2/src/runtime/__nsstring.rs +++ b/crates/objc2/src/runtime/__nsstring.rs @@ -29,9 +29,19 @@ pub unsafe fn nsstring_len(obj: &NSObject) -> NSUInteger { /// Extract a [`str`](`prim@str`) representation out of the given NSString. /// +/// Uses [`UTF8String`] under the hood. +/// +/// [`UTF8String`]: https://developer.apple.com/documentation/foundation/nsstring/1411189-utf8string?language=objc +/// +/// /// # Safety /// -/// The object must be an instance of `NSString`. +/// - The object must be an instance of `NSString`. +/// - The returned string must not be moved outside the autorelease pool into +/// which it (may) have been released. +/// +/// Furthermore, the object must not, as is always the case for strings, be +/// mutated in parallel. // // Note: While this is not public, it is still a breaking change to modify, // since `objc2-foundation` relies on it. @@ -42,21 +52,33 @@ pub unsafe fn nsstring_to_str<'r, 's: 'r, 'p: 'r>( // This is necessary until `auto` types stabilizes. pool.__verify_is_inner(); - // The documentation on `UTF8String` is a bit sparse, but with - // educated guesses and testing I've determined that NSString stores - // a pointer to the string data, sometimes with an UTF-8 encoding, - // (usual for ascii data), sometimes in other encodings (UTF-16?). + // The documentation on `UTF8String` is quite sparse, but with educated + // guesses, testing, reading the code for `CFString` and a bit of + // reverse-engineering, we've determined that `NSString` stores a pointer + // to the string data, sometimes with an UTF-8 encoding (usual for ascii + // data), sometimes in other encodings (often UTF-16). // // `UTF8String` then checks the internal encoding: - // - If the data is UTF-8 encoded, it returns the internal pointer. - // - If the data is in another encoding, it creates a new allocation, - // writes the UTF-8 representation of the string into it, - // autoreleases the allocation and returns a pointer to it. + // - If the data is UTF-8 encoded, and (since macOS 10.6) if the string is + // immutable, it returns the internal pointer using + // `CFStringGetCStringPtr`. + // - Otherwise, if the data is in another encoding or is mutable, it + // creates a new allocation, writes the UTF-8 representation of the + // string into it, autoreleases the allocation, and returns a pointer to + // it (similar to `CFStringGetCString`). + // + // If the string is a tagged pointer, or a custom subclass, then another + // code-path is taken that always creates a new allocation and copies the + // string into that using (effectively) `length` and `characterAtIndex:`. // - // So the lifetime of the returned pointer is either the same as the - // NSString OR the lifetime of the innermost @autoreleasepool. + // As a result, the lifetime of the returned pointer is either the same as + // the passed-in `NSString` OR the lifetime of the current / innermost + // `@autoreleasepool`. // - // https://developer.apple.com/documentation/foundation/nsstring/1411189-utf8string?language=objc + // Furthermore, we can allow creating a `&str` from `&obj`, even if the + // string is originally a `NSMutableString` which may be mutated later on, + // since in that case the lifetime will be tied to the pool and not the + // string. let bytes: *const c_char = unsafe { msg_send![obj, UTF8String] }; let bytes: *const u8 = bytes.cast(); diff --git a/crates/objc2/src/runtime/protocol_object.rs b/crates/objc2/src/runtime/protocol_object.rs index cfa7a11cd..1cd99b94d 100644 --- a/crates/objc2/src/runtime/protocol_object.rs +++ b/crates/objc2/src/runtime/protocol_object.rs @@ -160,14 +160,33 @@ impl hash::Hash for ProtocolObject

{ impl fmt::Debug for ProtocolObject

{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let description = self.description(); - // We use a leaking autorelease pool since often the string - // will be UTF-8, and in that case the pool will be - // irrelevant. Also, it allows us to pass the formatter into - // the pool (since it may contain a pool internally that it - // assumes is current when writing). + + // `NSString`s in return types, such as the one in `description`, are + // in general _supposed_ to be immutable: + // + // + // In reality, that isn't actually the case for `NSMutableString`, + // the `description` of that just returns itself instead of copying. + // + // Luckily though, the `UTF8String` returned by mutable objects do not + // return a reference to internal data, and instead always allocate + // a new autoreleased object (see details in `nsstring_to_str`), so + // we do not have to worry about the string being mutated in e.g. a + // malicious `Write` implementation in `fmt::Formatter` while we hold + // the `&str`. + + // We use a leaking autorelease pool since often the string will be + // UTF-8, and in that case the pool will be irrelevant. Also, it + // allows us to pass the formatter into the pool (since it may contain + // a pool internally that it assumes is current when writing). autoreleasepool_leaking(|pool| { - // SAFETY: `description` selector is guaranteed to always - // return an instance of `NSString`. + // SAFETY: + // - The `description` selector is guaranteed to always return an + // instance of `NSString`. + // - We control the scope in which the string is alive, so we know + // it is not moved outside the current autorelease pool + // (`autoreleasepool_leaking` is greatly helping with this, + // though by itself does not fully ensure it). let s = unsafe { nsstring_to_str(&description, pool) }; fmt::Display::fmt(s, f) }) diff --git a/framework-crates/objc2-foundation/src/string.rs b/framework-crates/objc2-foundation/src/string.rs index e2bf236c9..395d18306 100644 --- a/framework-crates/objc2-foundation/src/string.rs +++ b/framework-crates/objc2-foundation/src/string.rs @@ -55,11 +55,21 @@ impl NSString { /// Returns [`None`] if the internal storage does not allow this to be /// done efficiently. Use [`NSString::as_str`] or `NSString::to_string` /// if performance is not an issue. + /// + /// + /// # Safety + /// + /// The `NSString` must not be mutated for the lifetime of the returned + /// string. + /// + /// Warning: This is very difficult to ensure in generic contexts. #[doc(alias = "CFStringGetCStringPtr")] #[allow(unused)] #[cfg(target_vendor = "apple")] // TODO: Finish this - fn as_str_wip(&self) -> Option<&str> { + // TODO: Can this be used on NSStrings that are not internally CFString? + // (i.e. custom subclasses of NSString)? + unsafe fn as_str_wip(&self) -> Option<&str> { use core::ptr::NonNull; use std::os::raw::c_char; @@ -90,7 +100,7 @@ impl NSString { #[allow(unused)] #[cfg(target_vendor = "apple")] // TODO: Finish this - fn as_utf16(&self) -> Option<&[u16]> { + unsafe fn as_utf16(&self) -> Option<&[u16]> { use core::ptr::NonNull; extern "C" { @@ -102,12 +112,78 @@ impl NSString { .map(|ptr| unsafe { slice::from_raw_parts(ptr.as_ptr(), self.len_utf16()) }) } - /// Get the [`str`](`prim@str`) representation of this. + /// Convert the string into a [string slice](`prim@str`). + /// + /// The signature of this method can be a bit confusing, as it contains + /// several lifetimes; the lifetime `'s` of the `NSString`, the lifetime + /// `'p` of the current autorelease pool and the lifetime `'r` of the + /// returned string slice. + /// + /// In general, this method converts the string to a newly allocated UTF-8 + /// string, autoreleases the buffer, and returns a slice pointer to this + /// internal buffer, which will become invalid once the autorelease pool + /// is popped. So the lifetime of the return value is bound to the current + /// autorelease pool. + /// + /// However, as an optimization, this method may choose to instead return + /// an internal reference to the `NSString` when it can, and when the + /// string is immutable, and that is why the lifetime of the returned + /// string slice is also bound to the string itself. + /// + /// You should prefer the [`to_string`] method or the + /// [`Display` implementation][display-impl] over this method when + /// possible. + /// + /// [`to_string`]: alloc::string::ToString::to_string + /// [display-impl]: NSString#impl-Display-for-NSString + /// + /// + /// # Examples + /// + /// Get the string slice of the `NSString`, and compare it with another + /// inside an autorelease pool. + /// + /// ``` + /// use objc2_foundation::NSString; + /// use objc2::rc::autoreleasepool; /// - /// TODO: Further explain this. + /// let string = NSString::from_str("foo"); + /// autoreleasepool(|pool| { + /// assert_eq!(string.as_str(pool), "foo"); + /// }); + /// ``` + /// + /// Fails to compile because the lifetime of the string slice is bound to + /// the autorelease pool: + /// + /// ```compile_fail + /// # use objc2_foundation::NSString; + /// # use objc2::rc::autoreleasepool; + /// # + /// let string = NSString::from_str("foo"); + /// let s = autoreleasepool(|pool| string.as_str(pool)); + /// assert_eq!(s, "foo"); + /// ``` + /// + /// Fails to compile because the lifetime of the string slice is bound to + /// the string itself: + /// + /// ```compile_fail + /// # use objc2_foundation::NSString; + /// # use objc2::rc::autoreleasepool; + /// # + /// autoreleasepool(|pool| { + /// let string = NSString::from_str("foo"); + /// let s = string.as_str(pool); + /// drop(string); + /// assert_eq!(s, "foo"); + /// }); + /// ``` #[doc(alias = "UTF8String")] pub fn as_str<'r, 's: 'r, 'p: 'r>(&'s self, pool: AutoreleasePool<'p>) -> &'r str { // SAFETY: This is an instance of `NSString` + // + // TODO: Caller upholds that the string is not moved outside the pool. unsafe { nsstring_to_str(self, pool) } } @@ -229,20 +305,33 @@ impl AddAssign<&NSString> for NSMutableString { impl fmt::Display for NSString { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - autoreleasepool_leaking(|pool| fmt::Display::fmt(self.as_str(pool), f)) + // SAFETY: + // - The object is an instance of `NSString`. + // - We control the scope in which the string is alive, so we know + // it is not moved outside the current autorelease pool. + // + // TODO: Use more performant APIs, maybe by copying bytes into a + // temporary stack buffer so that we avoid allocating? + // + // Beware though that the string may be mutable internally, and that + // mutation may happen on every call to the formatter `f` (so + // `CFStringGetCharactersPtr` is probably out of the question, unless + // we somehow check that the string is immutable?). + autoreleasepool_leaking(|pool| fmt::Display::fmt(unsafe { nsstring_to_str(self, pool) }, f)) } } -impl fmt::Display for NSMutableString { - #[inline] +impl fmt::Debug for NSString { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&**self, f) + // SAFETY: Same as for `Display` above. + autoreleasepool_leaking(|pool| fmt::Debug::fmt(unsafe { nsstring_to_str(self, pool) }, f)) } } -impl fmt::Debug for NSString { +impl fmt::Display for NSMutableString { + #[inline] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - autoreleasepool_leaking(|pool| fmt::Debug::fmt(self.as_str(pool), f)) + fmt::Display::fmt(&**self, f) } }