diff src/libpam/question.rs @ 143:ebb71a412b58

Turn everything into OsString and Just Walk Out! for strings with nul. To reduce the hazard surface of the API, this replaces most uses of &str with &OsStr (and likewise with String/OsString). Also, I've decided that instead of dealing with callers putting `\0` in their parameters, I'm going to follow the example of std::env and Just Walk Out! (i.e., panic!()). This makes things a lot less annoying for both me and (hopefully) users.
author Paul Fisher <paul@pfish.zone>
date Sat, 05 Jul 2025 22:12:46 -0400
parents a508a69c068a
children 4b3a5095f68c
line wrap: on
line diff
--- a/src/libpam/question.rs	Sat Jul 05 21:49:27 2025 -0400
+++ b/src/libpam/question.rs	Sat Jul 05 22:12:46 2025 -0400
@@ -5,9 +5,10 @@
 use crate::libpam::memory;
 use crate::ErrorCode;
 use crate::Result;
-use libpam_sys_helpers::memory as pammem;
+use libpam_sys_helpers::memory as pam_mem;
 use num_enum::{IntoPrimitive, TryFromPrimitive};
-use std::ffi::{c_int, c_void, CStr};
+use std::ffi::{c_int, c_void, CStr, OsStr};
+use std::os::unix::ffi::OsStrExt;
 use std::ptr::NonNull;
 
 mod style_const {
@@ -69,12 +70,10 @@
     /// # Safety
     ///
     /// It's up to you to pass this only on types with a string value.
-    unsafe fn string_data(&self) -> Result<&str> {
+    unsafe fn string_data(&self) -> &OsStr {
         match self.data.as_ref() {
-            None => Ok(""),
-            Some(data) => CStr::from_ptr(data.as_ptr().cast())
-                .to_str()
-                .map_err(|_| ErrorCode::ConversationError),
+            None => "".as_ref(),
+            Some(data) => OsStr::from_bytes(CStr::from_ptr(data.as_ptr().cast()).to_bytes()),
         }
     }
 
@@ -82,7 +81,7 @@
     unsafe fn binary_data(&self) -> (&[u8], u8) {
         self.data
             .as_ref()
-            .map(|data| pammem::BinaryPayload::contents(data.as_ptr().cast()))
+            .map(|data| pam_mem::BinaryPayload::contents(data.as_ptr().cast()))
             .unwrap_or_default()
     }
 }
@@ -90,9 +89,9 @@
 impl TryFrom<&Exchange<'_>> for Question {
     type Error = ErrorCode;
     fn try_from(msg: &Exchange) -> Result<Self> {
-        let alloc = |style, text| -> Result<_> {
+        let alloc = |style, text: &OsStr| -> Result<_> {
             Ok((style, unsafe {
-                memory::CHeapBox::cast(memory::CHeapString::new(text)?.into_box())
+                memory::CHeapBox::cast(memory::CHeapString::new(text.as_bytes()).into_box())
             }))
         };
         // We will only allocate heap data if we have a valid input.
@@ -136,7 +135,7 @@
                     Style::BinaryPrompt => self
                         .data
                         .as_mut()
-                        .map(|p| pammem::BinaryPayload::zero(p.as_ptr().cast())),
+                        .map(|p| pam_mem::BinaryPayload::zero(p.as_ptr().cast())),
                     #[cfg(feature = "linux-pam-ext")]
                     Style::RadioType => self
                         .data
@@ -168,14 +167,14 @@
         let prompt = unsafe {
             match style {
                 Style::PromptEchoOff => {
-                    Self::MaskedPrompt(MaskedQAndA::new(question.string_data()?))
+                    Self::MaskedPrompt(MaskedQAndA::new(question.string_data()))
                 }
-                Style::PromptEchoOn => Self::Prompt(QAndA::new(question.string_data()?)),
-                Style::ErrorMsg => Self::Error(ErrorMsg::new(question.string_data()?)),
-                Style::TextInfo => Self::Info(InfoMsg::new(question.string_data()?)),
+                Style::PromptEchoOn => Self::Prompt(QAndA::new(question.string_data())),
+                Style::ErrorMsg => Self::Error(ErrorMsg::new(question.string_data())),
+                Style::TextInfo => Self::Info(InfoMsg::new(question.string_data())),
                 #[cfg(feature = "linux-pam-ext")]
                 Style::RadioType => {
-                    Self::RadioPrompt(crate::conv::RadioQAndA::new(question.string_data()?))
+                    Self::RadioPrompt(crate::conv::RadioQAndA::new(question.string_data()))
                 }
                 #[cfg(feature = "linux-pam-ext")]
                 Style::BinaryPrompt => {
@@ -188,8 +187,8 @@
 }
 
 #[cfg(feature = "linux-pam-ext")]
-impl From<pammem::TooBigError> for ErrorCode {
-    fn from(_: pammem::TooBigError) -> Self {
+impl From<pam_mem::TooBigError> for ErrorCode {
+    fn from(_: pam_mem::TooBigError) -> Self {
         ErrorCode::BufferError
     }
 }
@@ -219,12 +218,12 @@
     fn standard() {
         assert_matches!(
             (Exchange::MaskedPrompt, "hocus pocus"),
-            MaskedQAndA::new("hocus pocus")
+            MaskedQAndA::new("hocus pocus".as_ref())
         );
-        assert_matches!((Exchange::Prompt, "what"), QAndA::new("what"));
-        assert_matches!((Exchange::Prompt, "who"), QAndA::new("who"));
-        assert_matches!((Exchange::Info, "hey"), InfoMsg::new("hey"));
-        assert_matches!((Exchange::Error, "gasp"), ErrorMsg::new("gasp"));
+        assert_matches!((Exchange::Prompt, "what"), QAndA::new("what".as_ref()));
+        assert_matches!((Exchange::Prompt, "who"), QAndA::new("who".as_ref()));
+        assert_matches!((Exchange::Info, "hey"), InfoMsg::new("hey".as_ref()));
+        assert_matches!((Exchange::Error, "gasp"), ErrorMsg::new("gasp".as_ref()));
     }
 
     #[test]
@@ -237,7 +236,7 @@
         );
         assert_matches!(
             (Exchange::RadioPrompt, "you must choose"),
-            RadioQAndA::new("you must choose")
+            RadioQAndA::new("you must choose".as_ref())
         );
     }
 }