fix(sessions): Escape strings in the sessions __repr__ output

rust-bindings/src/expr/path_mapping.rs

@@ -122,16 +122,22 @@ impl PyPathMappingRule {
         &self.inner.destination_path
     }
 
-    fn __repr__(&self) -> String {
+    fn __repr__(&self, py: Python<'_>) -> PyResult<String> {
         // Render `source_path_format` using its Python name
         // (`PathFormat.POSIX`) rather than the underlying Rust
         // enum's `Debug` name (`Posix`). Matches the Python
         // convention for enum repr.
         let fmt: PyPathFormat = self.inner.source_path_format.into();
-        format!(
-            "PathMappingRule(source_path_format=PathFormat.{}, source_path='{}', destination_path='{}')",
-            fmt.variant_name(), self.inner.source_path, self.inner.destination_path
-        )
+        // The paths go through CPython's repr rather than `'{}'`. Hand-rolled
+        // quoting corrupted a Windows destination silently: `C:\temp` emitted
+        // `'C:\temp'`, which Python reads as `C:` + TAB + `emp`, and an
+        // apostrophe in a path closed the literal early.
+        Ok(format!(
+            "PathMappingRule(source_path_format=PathFormat.{}, source_path={}, destination_path={})",
+            fmt.variant_name(),
+            crate::py_repr::py_str(py, &self.inner.source_path)?,
+            crate::py_repr::py_str(py, &self.inner.destination_path)?,
+        ))
     }
 
     /// Two `PathMappingRule`s compare equal when they have the

rust-bindings/src/lib.rs

@@ -4,6 +4,7 @@
 mod expr;
 mod model;
 mod pickle_helpers;
+mod py_repr;
 mod sessions;
 
 use pyo3::prelude::*;

rust-bindings/src/py_repr.rs

@@ -0,0 +1,48 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+//! Rendering values into `__repr__` output that Python can parse.
+//!
+//! `format!("{:?}", s)` is not a Python literal writer. Rust's `Debug` for
+//! `str` special-cases only the quote, the backslash, and NUL, tab, CR and
+//! LF; every other control character and every non-printable falls through
+//! to Rust's brace form, `\u{1b}` or `\u{a0}`. Python wants exactly four
+//! hex digits after `\u`, so those do not parse. ESC is the one to keep in
+//! mind: ANSI colour sequences in captured process output hit this far more
+//! often than any exotic codepoint does.
+//!
+//! Delegating to CPython's own `repr()` removes the guesswork rather than
+//! reimplementing its escaping table: the output is by construction
+//! whatever the running interpreter produces, including its per-string
+//! choice of quote character.
+//!
+//! Scope: the `sessions` reprs route through here. Reprs under `model/`
+//! and the rest of `expr/` still use `{:?}` or hand-rolled quoting and
+//! carry the same defect — see the tracking note in the pull request that
+//! introduced this module. A new repr should use these helpers.
+//!
+//! Callers must not hold a lock across `py_str`: it re-enters the
+//! interpreter, which can run arbitrary Python (allocation may trigger a
+//! GC pass and with it `__del__` and weakref callbacks). Read what you
+//! need out from under the guard, drop it, then format.
+
+use pyo3::prelude::*;
+use pyo3::types::{PyString, PyStringMethods};
+
+/// CPython's `repr()` of `value`, ready to embed in a `__repr__`.
+pub(crate) fn py_str(py: Python<'_>, value: &str) -> PyResult<String> {
+    // `to_cow` reads the UTF-8 directly and propagates failure. Going via
+    // `to_string()` would resolve to PyO3's `Display`, which calls `str()`
+    // on the object -- a second interpreter round-trip whose error has
+    // nowhere to go but a panic out of `__repr__`.
+    Ok(PyString::new(py, value).repr()?.to_cow()?.into_owned())
+}
+
+/// An optional `int` as Python spells it. `Debug` would emit `Some(0)`,
+/// which evaluates to a `NameError`.
+pub(crate) fn py_opt_int(value: Option<i32>) -> String {
+    match value {
+        Some(v) => v.to_string(),
+        None => "None".to_string(),
+    }
+}

rust-bindings/src/sessions/session.rs

@@ -691,8 +691,19 @@ impl PySession {
         }
     }
 
-    fn __repr__(&self) -> String {
-        let snap = lock_recover(&self.snapshot);
-        format!("Session(id={:?}, state={:?})", snap.session_id, snap.state)
+    fn __repr__(&self, py: Python<'_>) -> PyResult<String> {
+        // Read out from under the guard and drop it before calling into
+        // CPython: `py_str` allocates, an allocation can trigger a GC pass,
+        // and a finalizer run by that pass may re-enter this Session and
+        // re-lock `snapshot`, which is not reentrant.
+        let (session_id, state) = {
+            let snap = lock_recover(&self.snapshot);
+            (snap.session_id.clone(), snap.state)
+        };
+        Ok(format!(
+            "Session(session_id={}, state=SessionState.{})",
+            crate::py_repr::py_str(py, &session_id)?,
+            crate::sessions::types::PySessionState::from(state).name()
+        ))
     }
 }

rust-bindings/src/sessions/session_user.rs

@@ -63,12 +63,12 @@ impl PyPosixSessionUser {
         self.inner.is_process_user()
     }
 
-    fn __repr__(&self) -> String {
-        format!(
-            "PosixSessionUser(user={:?}, group={:?})",
-            self.inner.user(),
-            self.inner.group()
-        )
+    fn __repr__(&self, py: Python<'_>) -> PyResult<String> {
+        Ok(format!(
+            "PosixSessionUser(user={}, group={})",
+            crate::py_repr::py_str(py, self.inner.user())?,
+            crate::py_repr::py_str(py, self.inner.group())?
+        ))
     }
 
     /// Pickle support — round-trips through `__init__(user, *, group=...)`.
@@ -307,8 +307,11 @@ impl PyWindowsSessionUser {
         self.inner.is_process_user()
     }
 
-    fn __repr__(&self) -> String {
-        format!("WindowsSessionUser(user={:?})", self.inner.user())
+    fn __repr__(&self, py: Python<'_>) -> PyResult<String> {
+        Ok(format!(
+            "WindowsSessionUser(user={})",
+            crate::py_repr::py_str(py, self.inner.user())?
+        ))
     }
 
     /// Pickle support — round-trips through `__init__(user, *,

rust-bindings/src/sessions/types.rs

@@ -53,7 +53,7 @@ impl From<SessionState> for PySessionState {
 impl PySessionState {
     /// Variant name as a string (e.g. `"READY"`).
     #[getter]
-    fn name(&self) -> &'static str {
+    pub(crate) fn name(&self) -> &'static str {
         match self {
             Self::READY => "READY",
             Self::RUNNING => "RUNNING",
@@ -323,8 +323,9 @@ impl PyActionStatus {
 
     fn __repr__(&self) -> String {
         format!(
-            "ActionStatus(state={:?}, exit_code={:?})",
-            self.inner.state, self.inner.exit_code
+            "ActionStatus(state=ActionState.{}, exit_code={})",
+            self.state().name(),
+            crate::py_repr::py_opt_int(self.inner.exit_code)
         )
     }
 
@@ -525,13 +526,13 @@ impl PyActionResult {
         }
     }
 
-    fn __repr__(&self) -> String {
-        format!(
-            "ActionResult(state={}, exit_code={:?}, stdout={:?})",
+    fn __repr__(&self, py: Python<'_>) -> PyResult<String> {
+        Ok(format!(
+            "ActionResult(state=ActionState.{}, exit_code={}, stdout={})",
             self.state.name(),
-            self.exit_code,
-            self.stdout,
-        )
+            crate::py_repr::py_opt_int(self.exit_code),
+            crate::py_repr::py_str(py, &self.stdout)?,
+        ))
     }
 
     fn __eq__(&self, other: &Self) -> bool {

test/openjd/sessions/__init__.py

@@ -0,0 +1,2 @@
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# SPDX-License-Identifier: Apache-2.0