gpui: Handle null string pointer in `window::insert_text` (#12446)

Sean Billig created 2 years ago

`[NSString UTF8String]` sometimes returns null (it's documented as
such), and when it does, zed crashes in `window::insert_text`. I'm
running into this sometimes when using alt-d to delete forward. It
usually only happens with multiple cursors, but sometimes with a single
cursor. It *might* only happen when using the "Unicode Hex Input"
keyboard 'Input Source' (which I started using to avoid entering weird
characters in zed when using emacs meta keybindings that I haven't
defined in zed).

When using the US English input source, alt-d always results in a call
to `insert_text`. When using the Unicode Hex Input source it usually
doesn't, but when it does `text.UTF8String()` returns null. `text` isn't
null. `[text length]` returns 1. `[text characterAtIndex: 0]` seems to
always return `56797` (an undefined utf-16 codepoint).

Release Notes:

- Fixed crash on mac when deleting with alt-d

Change summary

crates/gpui/src/platform/mac.rs        | 20 +++++++++++++++++++-
crates/gpui/src/platform/mac/events.rs | 24 ++++++++++--------------
crates/gpui/src/platform/mac/window.rs | 12 ++++--------
3 files changed, 33 insertions(+), 23 deletions(-)

Detailed changes

crates/gpui/src/platform/mac.rs

@@ -29,7 +29,10 @@ use cocoa::{
 };
 
 use objc::runtime::{BOOL, NO, YES};
-use std::ops::Range;
+use std::{
+    ffi::{c_char, CStr},
+    ops::Range,
+};
 
 pub(crate) use dispatcher::*;
 pub(crate) use display::*;
@@ -52,6 +55,21 @@ impl BoolExt for bool {
     }
 }
 
+trait NSStringExt {
+    unsafe fn to_str(&self) -> &str;
+}
+
+impl NSStringExt for id {
+    unsafe fn to_str(&self) -> &str {
+        let cstr = self.UTF8String();
+        if cstr.is_null() {
+            ""
+        } else {
+            CStr::from_ptr(cstr as *mut c_char).to_str().unwrap()
+        }
+    }
+}
+
 #[repr(C)]
 #[derive(Copy, Clone, Debug)]
 struct NSRange {

crates/gpui/src/platform/mac/events.rs

@@ -1,12 +1,12 @@
 use crate::{
-    point, px, KeyDownEvent, KeyUpEvent, Keystroke, Modifiers, ModifiersChangedEvent, MouseButton,
-    MouseDownEvent, MouseExitEvent, MouseMoveEvent, MouseUpEvent, NavigationDirection, Pixels,
-    PlatformInput, ScrollDelta, ScrollWheelEvent, TouchPhase,
+    platform::mac::NSStringExt, point, px, KeyDownEvent, KeyUpEvent, Keystroke, Modifiers,
+    ModifiersChangedEvent, MouseButton, MouseDownEvent, MouseExitEvent, MouseMoveEvent,
+    MouseUpEvent, NavigationDirection, Pixels, PlatformInput, ScrollDelta, ScrollWheelEvent,
+    TouchPhase,
 };
 use cocoa::{
     appkit::{NSEvent, NSEventModifierFlags, NSEventPhase, NSEventType},
     base::{id, YES},
-    foundation::NSString as _,
 };
 use core_graphics::{
     event::{CGEvent, CGEventFlags, CGKeyCode},
@@ -15,7 +15,7 @@ use core_graphics::{
 use ctor::ctor;
 use metal::foreign_types::ForeignType as _;
 use objc::{class, msg_send, sel, sel_impl};
-use std::{borrow::Cow, ffi::CStr, mem, os::raw::c_char, ptr};
+use std::{borrow::Cow, mem, ptr};
 
 const BACKSPACE_KEY: u16 = 0x7f;
 const SPACE_KEY: u16 = b' ' as u16;
@@ -243,11 +243,10 @@ impl PlatformInput {
 unsafe fn parse_keystroke(native_event: id) -> Keystroke {
     use cocoa::appkit::*;
 
-    let mut chars_ignoring_modifiers =
-        CStr::from_ptr(native_event.charactersIgnoringModifiers().UTF8String() as *mut c_char)
-            .to_str()
-            .unwrap()
-            .to_string();
+    let mut chars_ignoring_modifiers = native_event
+        .charactersIgnoringModifiers()
+        .to_str()
+        .to_string();
     let first_char = chars_ignoring_modifiers.chars().next().map(|ch| ch as u16);
     let modifiers = native_event.modifierFlags();
 
@@ -351,9 +350,6 @@ fn chars_for_modified_key(code: CGKeyCode, cmd: bool, shift: bool) -> String {
 
     unsafe {
         let event: id = msg_send![class!(NSEvent), eventWithCGEvent: &*event];
-        CStr::from_ptr(event.characters().UTF8String())
-            .to_str()
-            .unwrap()
-            .to_string()
+        event.characters().to_str().to_string()
     }
 }

crates/gpui/src/platform/mac/window.rs

@@ -1,4 +1,4 @@
-use super::{ns_string, renderer, MacDisplay, NSRange};
+use super::{ns_string, renderer, MacDisplay, NSRange, NSStringExt};
 use crate::{
     platform::PlatformInputHandler, point, px, size, AnyWindowHandle, Bounds, DevicePixels,
     DisplayLink, ExternalPaths, FileDropEvent, ForegroundExecutor, KeyDownEvent, Keystroke,
@@ -39,7 +39,6 @@ use std::{
     ffi::{c_void, CStr},
     mem,
     ops::Range,
-    os::raw::c_char,
     path::PathBuf,
     ptr::{self, NonNull},
     rc::Rc,
@@ -1711,9 +1710,8 @@ extern "C" fn insert_text(this: &Object, _: Sel, text: id, replacement_range: NS
         } else {
             text
         };
-        let text = CStr::from_ptr(text.UTF8String() as *mut c_char)
-            .to_str()
-            .unwrap();
+
+        let text = text.to_str();
         let replacement_range = replacement_range.to_range();
         send_to_input_handler(
             this,
@@ -1739,9 +1737,7 @@ extern "C" fn set_marked_text(
         };
         let selected_range = selected_range.to_range();
         let replacement_range = replacement_range.to_range();
-        let text = CStr::from_ptr(text.UTF8String() as *mut c_char)
-            .to_str()
-            .unwrap();
+        let text = text.to_str();
 
         send_to_input_handler(
             this,