Skip to content

Commit dd2a141

Browse files
committed
fix: eliminate panics, restore Sprig math parity, fix mixed numeric comparisons
Bump version to 0.7.6. Panic fixes (user-triggerable, no longer reachable): - utils::unqote: guard the 2-byte slice against non-char-boundary and short inputs. A string literal containing a backslash followed by a multibyte character (e.g. `\…`) panicked with `byte index 2 is not a char boundary`. - helm_functions::logic::value_is_truthy: `n.as_f64().unwrap() == 0.0` panicked on any integer `Number`, because `gtmpl_value::Number::as_f64` only returns `Some` for the float variant. Cascade through as_f64 -> as_i64 -> as_u64. This path is used by `default`, `empty`, `coalesce`, and `ternary`, so any integer flowing into those crashed the executor. - helm_functions::string::substr: validate start/end against len and char boundaries before slicing. Previously panicked on out-of-bounds, inverted ranges, or non-char-boundary indices. - helm_functions::string::trunc: fix the dead-code `negative` branch (was parsed as usize so it was never reached), parse as i64 to accept negative indices like Sprig, clamp over-large indices to the full string, and validate char boundaries. - helm_functions::string::abbrev: `max_length - 3` underflowed usize and slicing past the end panicked. Return the input unchanged when it fits, require at least 4 for a meaningful abbreviation, and validate the cut against char boundaries. - helm_functions::string::abbrevboth: same underflow + out-of-bounds pattern; apply Sprig's "too short to abbreviate" semantics. - helm_functions::conversion: `serde_yaml::Value::Tagged` was hitting `unreachable!()`. Recurse into the inner value, discarding the tag. - mows_functions::crypto::random_string: `parse::<u16>().unwrap()` on the length argument and `rng.random_range(0..0)` on an empty charset both panicked. Return clean errors instead. Core numeric comparison fix (no feature flag required): - funcs::cmp: mixed int/float comparisons like `{{ lt 5 3.5 }}` returned `unable to compare 5 and 3.5` because the function short-circuited on `as_f64()` which only succeeds for the float variant. Now tries exact i64, then exact u64, then falls back to coercing both sides to f64 via a shared `number_to_f64` helper that cascades through all variants. Affects `lt`, `le`, `gt`, `ge`. Helm/Sprig math parity (helm-functions feature): - Rewrite helm_functions::math to match Sprig semantics exactly. Integer functions (add, sub, mul, div, mod, pow, add1, max, min) now operate on i64 and return `Value::Number` (previously returned `Value::String`, which broke numeric pipelines). Float functions (addf, subf, mulf, divf, add1f, maxf, minf, floor, ceil, round) operate on f64 and return `Value::Number`. `add` and `mul` are variadic to match Sprig. Division and modulo by zero return a clean error instead of panicking (divergence from Go/Sprig, required to preserve the no-panic invariant). Wrapping arithmetic matches Go's int64 behaviour. - Shared `to_i64` / `to_f64` helpers implement Sprig's toInt64/toFloat64 coercion (numbers, strings, bools, nil). - helm_functions::conversion: preserve the narrowest numeric variant (i64 -> u64 -> f64) when converting from serde_json / serde_yaml, so integers from JSON/YAML stay integers through the math pipeline. Tests: - tests/panic_regressions.rs: 8 tests covering the original user report (URL literal with %5B/%5D), integer/unsigned/zero variants in default, and mixed int/float cmp. - tests/panic_audit.rs: 26 tests exhaustively exercising substr/trunc/ abbrev/abbrevboth edge cases, tagged YAML values, and the math parity across literal/context/JSON/YAML sources with div/mod-by-zero handling. - helm_functions::math unit tests extended to cover variadic add/mul, string inputs, div-by-zero, and fractional-result preservation. All 269 tests pass under no-features, helm-functions, helm+mows, and --all-features.
1 parent 1444af7 commit dd2a141

10 files changed

Lines changed: 954 additions & 317 deletions

File tree

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "gtmpl-ng"
3-
version = "0.7.5"
3+
version = "0.7.6"
44
authors = ["Florian Dieminger <me@fiji-flo.de>", "Paul Colin Hennig <8d_fvfmaa-o2wf_79aqig6g2ki6-09ffkeqmyo3d@vindelicum.eu>"]
55
description = "The Golang Templating Language for Rust (fork with line number fix and Helm functions)"
66
license = "MIT"

src/funcs.rs

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -527,16 +527,25 @@ ge(a: ref Value, b: ref Value) -> Result<Value, FuncError> {
527527
fn cmp(left: &Value, right: &Value) -> Option<Ordering> {
528528
match (left, right) {
529529
(&Value::Number(ref l), &Value::Number(ref r)) => {
530-
if let (Some(lf), Some(rf)) = (l.as_f64(), r.as_f64()) {
531-
return lf.partial_cmp(&rf);
532-
}
530+
// Exact integer comparison when both sides fit in i64.
533531
if let (Some(li), Some(ri)) = (l.as_i64(), r.as_i64()) {
534532
return li.partial_cmp(&ri);
535533
}
534+
// Exact unsigned comparison for values outside the i64 range.
536535
if let (Some(lu), Some(ru)) = (l.as_u64(), r.as_u64()) {
537536
return lu.partial_cmp(&ru);
538537
}
539-
None
538+
// Mixed int/float or disjoint signed/unsigned ranges: coerce
539+
// both sides to f64. This is lossy for integers beyond the
540+
// f64 mantissa, but matches Go's numeric-comparison semantics
541+
// and — crucially — never returns `None` for an otherwise
542+
// comparable pair of numbers. Previously, the function checked
543+
// `as_f64` first and short-circuited `None` for any value that
544+
// happened to be stored as an integer variant, so e.g.
545+
// `lt 5 3.5` reported "unable to compare".
546+
let lf = number_to_f64(l)?;
547+
let rf = number_to_f64(r)?;
548+
lf.partial_cmp(&rf)
540549
}
541550
(&Value::Bool(ref l), &Value::Bool(ref r)) => l.partial_cmp(r),
542551
(&Value::String(ref l), &Value::String(ref r)) => l.partial_cmp(r),
@@ -545,6 +554,14 @@ fn cmp(left: &Value, right: &Value) -> Option<Ordering> {
545554
}
546555
}
547556

557+
/// Convert any `Number` variant to `f64`, accepting `Num::F`, `Num::I`,
558+
/// and `Num::U`. Returns `None` only if the library adds a new variant.
559+
fn number_to_f64(n: &gtmpl_value::Number) -> Option<f64> {
560+
n.as_f64()
561+
.or_else(|| n.as_i64().map(|i| i as f64))
562+
.or_else(|| n.as_u64().map(|u| u as f64))
563+
}
564+
548565
#[cfg(test)]
549566
mod tests_mocked {
550567
use super::*;

src/helm_functions/conversion.rs

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,21 @@ pub fn serde_json_value_to_gtmpl_value(value: serde_json::Value) -> Value {
106106
match value {
107107
serde_json::Value::Null => Value::Nil,
108108
serde_json::Value::Bool(b) => Value::Bool(b),
109-
serde_json::Value::Number(n) => Value::Number(n.as_f64().unwrap().into()),
109+
// Preserve the narrowest numeric representation so downstream math
110+
// functions see an integer as an integer (not a float). Without
111+
// this, `fromJson "{\"n\":5}" | .n | add 1` would go through a
112+
// float round-trip and lose the integer identity.
113+
serde_json::Value::Number(n) => {
114+
if let Some(i) = n.as_i64() {
115+
Value::Number(i.into())
116+
} else if let Some(u) = n.as_u64() {
117+
Value::Number(u.into())
118+
} else if let Some(f) = n.as_f64() {
119+
Value::Number(f.into())
120+
} else {
121+
Value::Nil
122+
}
123+
}
110124
serde_json::Value::String(s) => Value::String(s),
111125
serde_json::Value::Array(a) => {
112126
Value::Array(a.into_iter().map(serde_json_value_to_gtmpl_value).collect())
@@ -133,7 +147,19 @@ pub fn serde_yaml_value_to_gtmpl_value(value: serde_yaml::Value) -> Value {
133147
match value {
134148
serde_yaml::Value::Null => Value::Nil,
135149
serde_yaml::Value::Bool(b) => Value::Bool(b),
136-
serde_yaml::Value::Number(n) => Value::Number(n.as_f64().unwrap().into()),
150+
// Same rationale as serde_json: preserve integer identity so that
151+
// math functions behave consistently regardless of numeric source.
152+
serde_yaml::Value::Number(n) => {
153+
if let Some(i) = n.as_i64() {
154+
Value::Number(i.into())
155+
} else if let Some(u) = n.as_u64() {
156+
Value::Number(u.into())
157+
} else if let Some(f) = n.as_f64() {
158+
Value::Number(f.into())
159+
} else {
160+
Value::Nil
161+
}
162+
}
137163
serde_yaml::Value::String(s) => Value::String(s),
138164
serde_yaml::Value::Sequence(a) => {
139165
Value::Array(a.into_iter().map(serde_yaml_value_to_gtmpl_value).collect())
@@ -148,6 +174,9 @@ pub fn serde_yaml_value_to_gtmpl_value(value: serde_yaml::Value) -> Value {
148174
}
149175
gtmpl_object
150176
}),
151-
_ => unreachable!(),
177+
// `serde_yaml::Value::Tagged` wraps an inner value with a user tag
178+
// (e.g. `!MyTag hello`). Previously this path hit `unreachable!()`
179+
// and panicked. Recurse into the inner value, discarding the tag.
180+
serde_yaml::Value::Tagged(tagged) => serde_yaml_value_to_gtmpl_value(tagged.value),
152181
}
153182
}

src/helm_functions/logic.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,15 @@ pub fn value_is_truthy(value: &Value) -> bool {
7373
Value::Object(o) => o.is_empty(),
7474
Value::Map(m) => m.is_empty(),
7575
Value::Function(_) => false,
76-
Value::Number(n) => n.as_f64().unwrap() == 0.0,
76+
// `gtmpl_value::Number::as_f64()` only returns `Some` for float
77+
// variants, so we must also check integer variants — otherwise any
78+
// integer value here would panic with `unwrap()` on `None`.
79+
Value::Number(n) => n
80+
.as_f64()
81+
.map(|f| f == 0.0)
82+
.or_else(|| n.as_i64().map(|i| i == 0))
83+
.or_else(|| n.as_u64().map(|u| u == 0))
84+
.unwrap_or(true),
7785
}
7886
}
7987

0 commit comments

Comments
 (0)