Skip to content

Commit 243bc28

Browse files
wolfvwolfv
authored
fix: complain when filter is used on undefined in strict and semi-strict mode (#877)
1 parent b9afca4 commit 243bc28

File tree

6 files changed

+213
-16
lines changed

6 files changed

+213
-16
lines changed

‎.github/workflows/build-wheels.yml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ jobs:
2929
- name: Build wheels
3030
uses: PyO3/maturin-action@v1
3131
with:
32-
args: --release --target universal2-apple-darwin --out dist -m minijinja-py/Cargo.toml
32+
args: --release --strip --target universal2-apple-darwin --out dist -m minijinja-py/Cargo.toml
3333
- name: Upload wheels
3434
uses: actions/upload-artifact@v4
3535
with:
@@ -56,7 +56,7 @@ jobs:
5656
uses: PyO3/maturin-action@v1
5757
with:
5858
target: ${{ matrix.target }}
59-
args: --release --out dist -m minijinja-py/Cargo.toml --interpreter "3.9"
59+
args: --release --strip --out dist -m minijinja-py/Cargo.toml --interpreter "3.9"
6060
manylinux: auto
6161
- name: Upload wheels
6262
uses: actions/upload-artifact@v4
@@ -84,7 +84,7 @@ jobs:
8484
uses: PyO3/maturin-action@v1
8585
with:
8686
target: ${{ matrix.target }}
87-
args: --release --out dist -m minijinja-py/Cargo.toml --interpreter "3.9"
87+
args: --release --strip --out dist -m minijinja-py/Cargo.toml --interpreter "3.9"
8888
manylinux: musllinux_1_2
8989
- name: Upload wheels
9090
uses: actions/upload-artifact@v4
@@ -110,7 +110,7 @@ jobs:
110110
uses: PyO3/maturin-action@v1
111111
with:
112112
target: ${{ matrix.target }}
113-
args: --release --out dist -m minijinja-py/Cargo.toml
113+
args: --release --strip --out dist -m minijinja-py/Cargo.toml
114114
- name: Upload wheels
115115
uses: actions/upload-artifact@v4
116116
with:

‎minijinja-py/pyproject.toml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ Repository = "https://github.com/mitsuhiko/minijinja"
3434
[tool.maturin]
3535
module-name = "minijinja._lowlevel"
3636
python-source = "python"
37-
strip = true
3837

3938
[tool.pyright]
4039
include = ["python/**/*.pyi"]

‎minijinja/src/filters.rs‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -554,9 +554,12 @@ mod builtins {
554554
/// {{ "42"|int == 42 }} -> true
555555
/// ```
556556
#[cfg_attr(docsrs, doc(cfg(feature = "builtins")))]
557-
pub fn int(value: &Value) -> Result<Value, Error> {
557+
pub fn int(state: &State, value: &Value) -> Result<Value, Error> {
558558
match &value.0 {
559-
ValueRepr::Undefined(_) | ValueRepr::None => Ok(Value::from(0)),
559+
ValueRepr::Undefined(_) | ValueRepr::None => {
560+
ok!(state.undefined_behavior().assert_value_not_undefined(value));
561+
Ok(Value::from(0))
562+
}
560563
ValueRepr::Bool(x) => Ok(Value::from(*x as u64)),
561564
ValueRepr::U64(_) | ValueRepr::I64(_) | ValueRepr::U128(_) | ValueRepr::I128(_) => {
562565
Ok(value.clone())
@@ -587,9 +590,12 @@ mod builtins {
587590
/// {{ "42.5"|float == 42.5 }} -> true
588591
/// ```
589592
#[cfg_attr(docsrs, doc(cfg(feature = "builtins")))]
590-
pub fn float(value: &Value) -> Result<Value, Error> {
593+
pub fn float(state: &State, value: &Value) -> Result<Value, Error> {
591594
match &value.0 {
592-
ValueRepr::Undefined(_) | ValueRepr::None => Ok(Value::from(0.0)),
595+
ValueRepr::Undefined(_) | ValueRepr::None => {
596+
ok!(state.undefined_behavior().assert_value_not_undefined(value));
597+
Ok(Value::from(0.0))
598+
}
593599
ValueRepr::Bool(x) => Ok(Value::from(*x as u64 as f64)),
594600
ValueRepr::String(..) | ValueRepr::SmallStr(_) => value
595601
.as_str()
@@ -843,12 +849,13 @@ mod builtins {
843849
///
844850
/// If the string has been marked as safe, that value is preserved.
845851
#[cfg_attr(docsrs, doc(cfg(feature = "builtins")))]
846-
pub fn string(value: &Value) -> Value {
847-
if value.kind() == ValueKind::String {
852+
pub fn string(state: &State, value: &Value) -> Result<Value, Error> {
853+
ok!(state.undefined_behavior().assert_value_not_undefined(value));
854+
Ok(if value.kind() == ValueKind::String {
848855
value.clone()
849856
} else {
850857
value.to_string().into()
851-
}
858+
})
852859
}
853860

854861
/// Converts the value into a boolean value.
@@ -860,8 +867,8 @@ mod builtins {
860867
/// {{ 42|bool }} -> true
861868
/// ```
862869
#[cfg_attr(docsrs, doc(cfg(feature = "builtins")))]
863-
pub fn bool(value: &Value) -> bool {
864-
value.is_true()
870+
pub fn bool(state: &State, value: &Value) -> Result<bool, Error> {
871+
state.undefined_behavior().is_true(value)
865872
}
866873

867874
/// Slice an iterable and return a list of lists containing

‎minijinja/src/utils.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,15 @@ pub enum UndefinedBehavior {
134134
/// * **printing:** fails
135135
/// * **iteration:** fails
136136
/// * **attribute access of undefined values:** fails
137+
/// * **string coercion in filters/functions:** fails
137138
/// * **if true:** allowed (is considered false)
138139
SemiStrict,
139140
/// Complains very quickly about undefined values.
140141
///
141142
/// * **printing:** fails
142143
/// * **iteration:** fails
143144
/// * **attribute access of undefined values:** fails
145+
/// * **string coercion in filters/functions:** fails
144146
/// * **if true:** fails
145147
Strict,
146148
}
@@ -199,6 +201,24 @@ impl UndefinedBehavior {
199201
_ => Ok(()),
200202
}
201203
}
204+
205+
/// Checks that undefined cannot be coerced into a concrete type (e.g. string).
206+
///
207+
/// This is called when an undefined value is passed to a filter or function
208+
/// argument that expects a concrete type. Filters that explicitly handle
209+
/// undefined values (such as `default`) avoid this by using `&Value` as
210+
/// their first argument instead of a concrete type like `String`.
211+
#[inline]
212+
pub(crate) fn assert_value_not_undefined(self, value: &Value) -> Result<(), Error> {
213+
match (self, &value.0) {
214+
// silent undefined never errors
215+
(
216+
UndefinedBehavior::Strict | UndefinedBehavior::SemiStrict,
217+
&ValueRepr::Undefined(UndefinedType::Default),
218+
) => Err(Error::from(ErrorKind::UndefinedError)),
219+
_ => Ok(()),
220+
}
221+
}
202222
}
203223

204224
/// Helper to HTML escape a string.

‎minijinja/src/value/argtypes.rs‎

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,14 @@ pub trait ArgType<'a> {
154154
))
155155
}
156156

157+
#[doc(hidden)]
158+
fn from_state_and_value_owned(
159+
_state: Option<&'a State>,
160+
value: Value,
161+
) -> Result<Self::Output, Error> {
162+
Self::from_value_owned(value)
163+
}
164+
157165
#[doc(hidden)]
158166
fn from_state_and_value(
159167
_state: Option<&'a State>,
@@ -566,6 +574,16 @@ impl<'a> ArgType<'a> for Cow<'_, str> {
566574
None => Err(Error::from(ErrorKind::MissingArgument)),
567575
}
568576
}
577+
578+
fn from_state_and_value(
579+
state: Option<&'a State>,
580+
value: Option<&'a Value>,
581+
) -> Result<(Self::Output, usize), Error> {
582+
if let (Some(state), Some(value)) = (state, value) {
583+
ok!(state.undefined_behavior().assert_value_not_undefined(value));
584+
}
585+
Ok((ok!(Self::from_value(value)), 1))
586+
}
569587
}
570588

571589
impl<'a> ArgType<'a> for &Value {
@@ -675,15 +693,15 @@ impl<'a, T: ArgType<'a, Output = T>> ArgType<'a> for Rest<T> {
675693
}
676694

677695
fn from_state_and_values(
678-
_state: Option<&'a State>,
696+
state: Option<&'a State>,
679697
values: &'a [Value],
680698
offset: usize,
681699
) -> Result<(Self, usize), Error> {
682700
let args = values.get(offset..).unwrap_or_default();
683701
Ok((
684702
Rest(ok!(args
685703
.iter()
686-
.map(|v| T::from_value(Some(v)))
704+
.map(|v| T::from_state_and_value(state, Some(v)).map(|x| x.0))
687705
.collect::<Result<_, _>>())),
688706
args.len(),
689707
))
@@ -979,6 +997,28 @@ impl<'a> ArgType<'a> for String {
979997
}
980998
}
981999

1000+
fn from_state_and_value(
1001+
state: Option<&'a State>,
1002+
value: Option<&'a Value>,
1003+
) -> Result<(Self::Output, usize), Error> {
1004+
if let (Some(state), Some(value)) = (state, value) {
1005+
ok!(state.undefined_behavior().assert_value_not_undefined(value));
1006+
}
1007+
Ok((ok!(Self::from_value(value)), 1))
1008+
}
1009+
1010+
fn from_state_and_value_owned(
1011+
state: Option<&'a State>,
1012+
value: Value,
1013+
) -> Result<Self::Output, Error> {
1014+
if let Some(state) = state {
1015+
ok!(state
1016+
.undefined_behavior()
1017+
.assert_value_not_undefined(&value));
1018+
}
1019+
Self::from_value_owned(value)
1020+
}
1021+
9821022
fn from_value_owned(value: Value) -> Result<Self, Error> {
9831023
Ok(value.to_string())
9841024
}
@@ -1005,6 +1045,27 @@ impl<'a, T: ArgType<'a, Output = T>> ArgType<'a> for Vec<T> {
10051045
}
10061046
}
10071047

1048+
fn from_state_and_value(
1049+
state: Option<&'a State>,
1050+
value: Option<&'a Value>,
1051+
) -> Result<(Self::Output, usize), Error> {
1052+
match value {
1053+
None => Ok((Vec::new(), 1)),
1054+
Some(value) => {
1055+
let iter = ok!(value
1056+
.as_object()
1057+
.filter(|x| matches!(x.repr(), ObjectRepr::Seq | ObjectRepr::Iterable))
1058+
.and_then(|x| x.try_iter())
1059+
.ok_or_else(|| { Error::new(ErrorKind::InvalidOperation, "not iterable") }));
1060+
let mut rv = Vec::new();
1061+
for value in iter {
1062+
rv.push(ok!(T::from_state_and_value_owned(state, value)));
1063+
}
1064+
Ok((rv, 1))
1065+
}
1066+
}
1067+
}
1068+
10081069
fn from_value_owned(value: Value) -> Result<Self, Error> {
10091070
let iter = ok!(value
10101071
.as_object()

‎minijinja/tests/test_undefined.rs‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,14 @@ fn test_lenient_undefined() {
4343
fn test_semi_strict_undefined() {
4444
let mut env = Environment::new();
4545
env.set_undefined_behavior(UndefinedBehavior::SemiStrict);
46+
env.add_filter(
47+
"test_rest_join",
48+
|_state: &State, values: minijinja::value::Rest<String>| -> String { values.join("|") },
49+
);
50+
env.add_filter(
51+
"test_vec_join",
52+
|_state: &State, values: Vec<String>| -> String { values.join("|") },
53+
);
4654

4755
assert_eq!(
4856
env.render_str("{{ true.missing_attribute }}", ())
@@ -100,12 +108,65 @@ fn test_semi_strict_undefined() {
100108
.kind(),
101109
ErrorKind::UndefinedError
102110
);
111+
assert_eq!(
112+
env.render_str("{{ undefined|upper }}", ())
113+
.unwrap_err()
114+
.kind(),
115+
ErrorKind::UndefinedError
116+
);
117+
assert_eq!(
118+
env.render_str("{{ undefined|int }}", ())
119+
.unwrap_err()
120+
.kind(),
121+
ErrorKind::UndefinedError
122+
);
123+
assert_eq!(
124+
env.render_str("{{ undefined|float }}", ())
125+
.unwrap_err()
126+
.kind(),
127+
ErrorKind::UndefinedError
128+
);
129+
assert_eq!(
130+
env.render_str("{{ undefined|string }}", ())
131+
.unwrap_err()
132+
.kind(),
133+
ErrorKind::UndefinedError
134+
);
135+
assert_eq!(
136+
env.render_str("{{ undefined|test_rest_join }}", ())
137+
.unwrap_err()
138+
.kind(),
139+
ErrorKind::UndefinedError
140+
);
141+
assert_eq!(
142+
env.render_str("{{ [undefined]|test_vec_join }}", ())
143+
.unwrap_err()
144+
.kind(),
145+
ErrorKind::UndefinedError
146+
);
147+
// bool follows is_true semantics: SemiStrict allows it (returns false), Strict errors
148+
assert_eq!(render!(in env, "{{ undefined|bool }}"), "false");
149+
// none|int should still work (undefined check only applies to undefined, not none)
150+
assert_eq!(render!(in env, "{{ none|int }}"), "0");
151+
assert_eq!(
152+
env.render_str("{{ undefined|default('FALLBACK') }}", ())
153+
.unwrap(),
154+
"FALLBACK"
155+
);
103156
}
104157

105158
#[test]
106159
fn test_strict_undefined() {
107160
let mut env = Environment::new();
108161
env.set_undefined_behavior(UndefinedBehavior::Strict);
162+
env.add_filter(
163+
"test_rest_join",
164+
|_state: &State, values: minijinja::value::Rest<String>| -> String { values.join("|") },
165+
);
166+
env.add_filter(
167+
"test_vec_join",
168+
|_state: &State, values: Vec<String>| -> String { values.join("|") },
169+
);
109170

110171
assert_eq!(
111172
env.render_str("{{ true.missing_attribute }}", ())
@@ -174,6 +235,55 @@ fn test_strict_undefined() {
174235
.kind(),
175236
ErrorKind::UndefinedError
176237
);
238+
assert_eq!(
239+
env.render_str("{{ undefined|upper }}", ())
240+
.unwrap_err()
241+
.kind(),
242+
ErrorKind::UndefinedError
243+
);
244+
assert_eq!(
245+
env.render_str("{{ undefined|int }}", ())
246+
.unwrap_err()
247+
.kind(),
248+
ErrorKind::UndefinedError
249+
);
250+
assert_eq!(
251+
env.render_str("{{ undefined|float }}", ())
252+
.unwrap_err()
253+
.kind(),
254+
ErrorKind::UndefinedError
255+
);
256+
assert_eq!(
257+
env.render_str("{{ undefined|bool }}", ())
258+
.unwrap_err()
259+
.kind(),
260+
ErrorKind::UndefinedError
261+
);
262+
assert_eq!(
263+
env.render_str("{{ undefined|string }}", ())
264+
.unwrap_err()
265+
.kind(),
266+
ErrorKind::UndefinedError
267+
);
268+
assert_eq!(
269+
env.render_str("{{ undefined|test_rest_join }}", ())
270+
.unwrap_err()
271+
.kind(),
272+
ErrorKind::UndefinedError
273+
);
274+
assert_eq!(
275+
env.render_str("{{ [undefined]|test_vec_join }}", ())
276+
.unwrap_err()
277+
.kind(),
278+
ErrorKind::UndefinedError
279+
);
280+
// none|int should still work (only undefined is rejected)
281+
assert_eq!(render!(in env, "{{ none|int }}"), "0");
282+
assert_eq!(
283+
env.render_str("{{ undefined|default('FALLBACK') }}", ())
284+
.unwrap(),
285+
"FALLBACK"
286+
);
177287
}
178288

179289
#[test]

0 commit comments

Comments
 (0)