feat(native): Support SafeString for Python & Jinja (#7242)

Author: ovr

packages/cubejs-backend-native/Cargo.lock

@@ -226,7 +226,14 @@ def context_func(func):
     func.cube_context_func = True
     return func
 
+class SafeString(str):
+    is_safe: bool
+
+    def __init__(self, v: str):
+        self.is_safe = True
+
 __all__ = [
     'context_func',
-    'TemplateContext'
+    'TemplateContext',
+    'SafeString',
 ]

packages/cubejs-backend-native/python/cube/src/__init__.py

@@ -56,7 +56,6 @@ impl std::fmt::Display for CLReprObject {
     }
 }
 
-#[allow(unused)]
 #[derive(Debug)]
 pub enum CLReprKind {
     String,
@@ -68,17 +67,25 @@ pub enum CLReprKind {
     Object,
     JsFunction,
     #[cfg(feature = "python")]
+    #[allow(unused)]
     PythonRef,
     Null,
 }
 
+#[derive(Debug, Clone)]
+pub enum StringType {
+    Normal,
+    #[allow(unused)]
+    Safe,
+}
+
 /// Cross language representation is abstraction to transfer values between
 /// JavaScript and Python across Rust. Converting between two different languages requires
 /// to use Context which is available on the call (one for python and one for js), which result as
 /// blocking.
 #[derive(Debug, Clone)]
 pub enum CLRepr {
-    String(String),
+    String(String, StringType),
     Bool(bool),
     Float(f64),
     Int(i64),
@@ -118,7 +125,7 @@ impl CLRepr {
     #[allow(unused)]
     pub fn kind(&self) -> CLReprKind {
         match self {
-            CLRepr::String(_) => CLReprKind::String,
+            CLRepr::String(_, _) => CLReprKind::String,
             CLRepr::Bool(_) => CLReprKind::Bool,
             CLRepr::Float(_) => CLReprKind::Float,
             CLRepr::Int(_) => CLReprKind::Int,
@@ -139,7 +146,7 @@ impl CLRepr {
     ) -> Result<Self, Throw> {
         if from.is_a::<JsString, _>(cx) {
             let v = from.downcast_or_throw::<JsString, _>(cx)?;
-            Ok(CLRepr::String(v.value(cx)))
+            Ok(CLRepr::String(v.value(cx), StringType::Normal))
         } else if from.is_a::<JsArray, _>(cx) {
             let v = from.downcast_or_throw::<JsArray, _>(cx)?;
             let el = v.to_vec(cx)?;
@@ -205,7 +212,7 @@ impl CLRepr {
         #[cfg(feature = "python")] tcx: IntoJsContext,
     ) -> JsResult<'a, JsValue> {
         Ok(match from {
-            CLRepr::String(v) => cx.string(v).upcast(),
+            CLRepr::String(v, _) => cx.string(v).upcast(),
             CLRepr::Bool(v) => cx.boolean(v).upcast(),
             CLRepr::Float(v) => cx.number(v).upcast(),
             CLRepr::Int(v) => cx.number(v as f64).upcast(),

packages/cubejs-backend-native/src/cross/clrepr.rs

@@ -1,9 +1,9 @@
 use crate::cross::clrepr::CLReprObject;
-use crate::cross::CLRepr;
+use crate::cross::{CLRepr, StringType};
 use pyo3::exceptions::{PyNotImplementedError, PyTypeError};
 use pyo3::types::{
-    PyBool, PyComplex, PyDate, PyDict, PyFloat, PyFrame, PyFunction, PyInt, PyList, PySet,
-    PyString, PyTraceback, PyTuple,
+    PyBool, PyComplex, PyDate, PyDict, PyFloat, PyFrame, PyFunction, PyInt, PyList, PySequence,
+    PySet, PyString, PyTraceback, PyTuple,
 };
 use pyo3::{AsPyPointer, Py, PyAny, PyErr, PyObject, Python, ToPyObject};
 
@@ -30,7 +30,13 @@ impl CLReprPython for CLRepr {
         }
 
         return Ok(if v.get_type().is_subclass_of::<PyString>()? {
-            Self::String(v.to_string())
+            let string_type = if v.hasattr("is_safe")? {
+                StringType::Safe
+            } else {
+                StringType::Normal
+            };
+
+            Self::String(v.to_string(), string_type)
         } else if v.get_type().is_subclass_of::<PyBool>()? {
             Self::Bool(v.downcast::<PyBool>()?.is_true())
         } else if v.get_type().is_subclass_of::<PyFloat>()? {
@@ -92,19 +98,28 @@ impl CLReprPython for CLRepr {
                 "Unable to represent PyDate type as CLR from Python".to_string(),
             ));
         } else if v.get_type().is_subclass_of::<PyFrame>()? {
-            return Err(PyErr::new::<PyTypeError, _>(
-                "Unable to represent PyFrame type as CLR from Python".to_string(),
-            ));
+            let frame = v.downcast::<PyFrame>()?;
+
+            return Err(PyErr::new::<PyTypeError, _>(format!(
+                "Unable to represent PyFrame type as CLR from Python, value: {:?}",
+                frame
+            )));
         } else if v.get_type().is_subclass_of::<PyTraceback>()? {
-            return Err(PyErr::new::<PyTypeError, _>(
-                "Unable to represent PyTraceback type as CLR from Python".to_string(),
-            ));
+            let trb = v.downcast::<PyTraceback>()?;
+
+            return Err(PyErr::new::<PyTypeError, _>(format!(
+                "Unable to represent PyTraceback type as CLR from Python, value: {:?}",
+                trb
+            )));
         } else {
             let is_sequence = unsafe { pyo3::ffi::PySequence_Check(v.as_ptr()) == 1 };
             if is_sequence {
-                return Err(PyErr::new::<PyTypeError, _>(
-                    "Unable to represent PyTraceback type as CLR from Python".to_string(),
-                ));
+                let seq = v.downcast::<PySequence>()?;
+
+                return Err(PyErr::new::<PyTypeError, _>(format!(
+                    "Unable to represent PySequence type as CLR from Python, value: {:?}",
+                    seq
+                )));
             }
 
             Self::PythonRef(PythonRef::PyObject(v.into()))
@@ -113,7 +128,7 @@ impl CLReprPython for CLRepr {
 
     fn into_py_impl(from: CLRepr, py: Python) -> Result<PyObject, PyErr> {
         Ok(match from {
-            CLRepr::String(v) => PyString::new(py, &v).to_object(py),
+            CLRepr::String(v, _) => PyString::new(py, &v).to_object(py),
             CLRepr::Bool(v) => PyBool::new(py, v).to_object(py),
             CLRepr::Float(v) => PyFloat::new(py, v).to_object(py),
             CLRepr::Int(v) => {

packages/cubejs-backend-native/src/cross/clrepr_python.rs

@@ -4,7 +4,7 @@ mod clrepr_python;
 #[cfg(feature = "python")]
 mod py_in_js;
 
-pub use clrepr::{CLRepr, CLReprKind, CLReprObject};
+pub use clrepr::{CLRepr, CLReprKind, CLReprObject, StringType};
 
 #[cfg(feature = "python")]
 pub use clrepr_python::{CLReprPython, PythonRef};

packages/cubejs-backend-native/src/cross/mod.rs

@@ -10,7 +10,10 @@ pub fn to_minijinja_value(from: CLRepr) -> mjv::Value {
         CLRepr::Tuple(inner) | CLRepr::Array(inner) => {
             mjv::Value::from_seq_object(value::JinjaSequenceObject { inner })
         }
-        CLRepr::String(v) => mjv::Value::from(v),
+        CLRepr::String(v, mode) => match mode {
+            StringType::Normal => mjv::Value::from(v),
+            StringType::Safe => mjv::Value::from_safe_string(v),
+        },
         CLRepr::Float(v) => mjv::Value::from(v),
         CLRepr::Int(v) => mjv::Value::from(v),
         CLRepr::Bool(v) => mjv::Value::from(v),

packages/cubejs-backend-native/src/template/mj_value/mod.rs

@@ -28,14 +28,16 @@ pub fn from_minijinja_value(from: &mjv::Value) -> Result<CLRepr, mj::Error> {
                 ))
             }
         }
-        mjv::ValueKind::String => {
-            // TODO: Danger strings? Should we check from.is_safe()?
-            Ok(CLRepr::String(
-                from.as_str()
-                    .expect("ValueKind::String must return string from as_str()")
-                    .to_string(),
-            ))
-        }
+        mjv::ValueKind::String => Ok(CLRepr::String(
+            from.as_str()
+                .expect("ValueKind::String must return string from as_str()")
+                .to_string(),
+            if from.is_safe() {
+                StringType::Safe
+            } else {
+                StringType::Normal
+            },
+        )),
         mjv::ValueKind::Seq => {
             let seq = if let Some(seq) = from.as_seq() {
                 seq