From 8dd167ce6e28fc0407985bf6c73ed33a8e7af4d9 Mon Sep 17 00:00:00 2001 From: vitaliytv Date: Mon, 7 Sep 2026 06:43:56 +0300 Subject: [PATCH 1/4] feat(sprintf): support full q format specifications --- src/builtins/strings.rs | 352 +++++++++++++----- src/builtins/strings/sprintf_format.rs | 137 +++++++ .../cases/builtins/strings/sprintf.yaml | 56 ++- 3 files changed, 445 insertions(+), 100 deletions(-) create mode 100644 src/builtins/strings/sprintf_format.rs diff --git a/src/builtins/strings.rs b/src/builtins/strings.rs index dcef0207..c5d9181e 100644 --- a/src/builtins/strings.rs +++ b/src/builtins/strings.rs @@ -21,6 +21,7 @@ use crate::*; use anyhow::{bail, Result}; mod go_is_print; +mod sprintf_format; pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn>) { m.insert("concat", (concat, 2)); @@ -227,6 +228,35 @@ enum Width { Decimals(usize), } +#[derive(Clone, Copy, Default)] +struct FormatFlags { + alternate: bool, + zero: bool, + plus: bool, + minus: bool, + space: bool, +} + +#[derive(Clone, Copy, Default)] +struct FormatSpec { + flags: FormatFlags, + width: Option, + precision: Option, +} + +impl FormatSpec { + fn legacy_width(self) -> Width { + match (self.width, self.precision) { + (_, Some(precision)) => Width::Decimals(precision), + (Some(width), None) if self.flags.zero && !self.flags.minus => { + Width::LeadingZeros(width) + } + (Some(width), None) => Width::Cell(width), + (None, None) => Width::None, + } + } +} + fn apply_width(w: Width, s: String) -> String { match w { Width::LeadingZeros(n) if n > s.len() => "0".repeat(n - s.len()) + &s, @@ -237,29 +267,60 @@ fn apply_width(w: Width, s: String) -> String { const LOWER_HEX: &[u8; 16] = b"0123456789abcdef"; -// Append a string quoted like Go's `strconv.Quote` (and therefore OPA's `%q`). -// Precision truncates the input by Unicode scalar values before quoting, while -// width pads the quoted result by Unicode scalar values. -fn append_go_quoted(out: &mut String, input: &str, width: Width) -> Result<()> { - let input = match width { - Width::Decimals(precision) => truncate_chars(input, precision), - _ => input, +// Append a string quoted like Go's fmt `%q`. Precision truncates the input by +// Unicode scalar values before quoting, while width pads the quoted result by +// Unicode scalar values. `%+q` forces ASCII escapes and `%#q` uses a raw string +// whenever strconv.CanBackquote permits it. +fn append_go_quoted(out: &mut String, input: &str, spec: FormatSpec) -> Result<()> { + let input = spec + .precision + .map_or(input, |precision| truncate_chars(input, precision)); + let raw = spec.flags.alternate && can_backquote(input); + let content_len = if raw { + input.chars().count().saturating_add(2) + } else { + go_quoted_len(input, spec.flags.plus, '"') }; - - let (padding, padding_char) = match width { - Width::Cell(width) => (width.saturating_sub(go_quoted_len(input)), ' '), - Width::LeadingZeros(width) => (width.saturating_sub(go_quoted_len(input)), '0'), - Width::None | Width::Decimals(_) => (0, ' '), + let padding = spec.width.unwrap_or_default().saturating_sub(content_len); + let padding_char = if spec.flags.zero && !spec.flags.minus { + '0' + } else { + ' ' }; - for _ in 0..padding { - out.push(padding_char); + + if !spec.flags.minus { + append_padding(out, padding, padding_char)?; + } + + if raw { + out.push('`'); + out.push_str(input); enforce_limit()?; + out.push('`'); + enforce_limit()?; + } else { + append_go_quoted_body(out, input, spec.flags.plus, '"')?; } - out.push('"'); + if spec.flags.minus { + append_padding(out, padding, ' ')?; + } + Ok(()) +} + +fn append_go_quoted_body( + out: &mut String, + input: &str, + ascii_only: bool, + quote: char, +) -> Result<()> { + out.push(quote); for c in input.chars() { match c { - '"' => out.push_str("\\\""), + c if c == quote => { + out.push('\\'); + out.push(c); + } '\\' => out.push_str("\\\\"), '\u{0007}' => out.push_str("\\a"), '\u{0008}' => out.push_str("\\b"), @@ -268,30 +329,42 @@ fn append_go_quoted(out: &mut String, input: &str, width: Width) -> Result<()> { '\r' => out.push_str("\\r"), '\t' => out.push_str("\\t"), '\u{000B}' => out.push_str("\\v"), - c if go_is_print::is_print(c) => out.push(c), + c if go_is_print::is_print(c) && (!ascii_only || c.is_ascii()) => out.push(c), c if (c as u32) <= 0x7f => append_hex_escape(out, 'x', c as u32, 2), c if (c as u32) <= 0xffff => append_hex_escape(out, 'u', c as u32, 4), c => append_hex_escape(out, 'U', c as u32, 8), } enforce_limit()?; } - out.push('"'); + out.push(quote); enforce_limit() } +fn append_padding(out: &mut String, count: usize, padding_char: char) -> Result<()> { + for _ in 0..count { + out.push(padding_char); + enforce_limit()?; + } + Ok(()) +} + +fn can_backquote(s: &str) -> bool { + s.chars() + .all(|c| c != '`' && c != '\u{FEFF}' && c != '\u{007F}' && (c >= ' ' || c == '\t')) +} + fn truncate_chars(s: &str, count: usize) -> &str { s.char_indices() .nth(count) .map_or(s, |(byte_index, _)| &s[..byte_index]) } -fn go_quoted_len(s: &str) -> usize { +fn go_quoted_len(s: &str, ascii_only: bool, quote: char) -> usize { s.chars().fold(2usize, |len, c| { let escaped_len = match c { - '"' | '\\' | '\u{0007}' | '\u{0008}' | '\u{000C}' | '\n' | '\r' | '\t' | '\u{000B}' => { - 2 - } - c if go_is_print::is_print(c) => 1, + c if c == quote => 2, + '\\' | '\u{0007}' | '\u{0008}' | '\u{000C}' | '\n' | '\r' | '\t' | '\u{000B}' => 2, + c if go_is_print::is_print(c) && (!ascii_only || c.is_ascii()) => 1, c if (c as u32) <= 0x7f => 4, c if (c as u32) <= 0xffff => 6, _ => 10, @@ -300,6 +373,61 @@ fn go_quoted_len(s: &str) -> usize { }) } +fn append_go_quoted_rune(out: &mut String, value: i64, spec: FormatSpec) -> Result<()> { + let rune = u32::try_from(value) + .ok() + .and_then(char::from_u32) + .unwrap_or('\u{FFFD}'); + let mut encoded = [0u8; 4]; + let rune = rune.encode_utf8(&mut encoded); + let content_len = go_quoted_len(rune, spec.flags.plus, '\''); + let padding = spec.width.unwrap_or_default().saturating_sub(content_len); + let padding_char = if spec.flags.zero && !spec.flags.minus { + '0' + } else { + ' ' + }; + + if !spec.flags.minus { + append_padding(out, padding, padding_char)?; + } + append_go_quoted_body(out, rune, spec.flags.plus, '\'')?; + if spec.flags.minus { + append_padding(out, padding, ' ')?; + } + Ok(()) +} + +fn append_go_quoted_value(out: &mut String, value: &Value, spec: FormatSpec) -> Result<()> { + match value { + Value::String(value) => append_go_quoted(out, value.as_ref(), spec), + Value::Number(Number::Int(value)) => append_go_quoted_rune(out, *value, spec), + Value::Number(Number::UInt(value)) if *value <= i64::MAX as u64 => { + append_go_quoted_rune(out, *value as i64, spec) + } + Value::Number(number @ (Number::UInt(_) | Number::BigInt(_))) => { + out.push_str("%!q(big.Int="); + out.push_str(&number.format_decimal()); + out.push(')'); + enforce_limit() + } + Value::Number(Number::Float(value)) => { + out.push_str("%!q(float64="); + if spec.flags.plus && value.is_sign_positive() { + out.push('+'); + } + out.push_str(&value.to_string()); + out.push(')'); + enforce_limit() + } + value => { + let value = to_string(value, false); + enforce_limit()?; + append_go_quoted(out, &value, spec) + } + } +} + fn append_hex_escape(out: &mut String, prefix: char, value: u32, digits: usize) { out.push('\\'); out.push(prefix); @@ -317,57 +445,44 @@ fn sprintf(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> let mut s = String::default(); let mut args_idx = 0usize; - let mut chars = fmt.chars().peekable(); + let mut cursor = 0usize; + let mut reordered = false; let args_span = params[1].span(); + let format_span = params[0].span(); loop { - let (verb, width) = match chars.next() { - Some('%') => match chars.next() { - Some('%') => { - s.push('%'); - continue; - } - Some(c) if c == '.' || c.is_numeric() => { - let first_char = c; - let mut w = 0; - if c != '.' { - w = c.to_digit(10).expect("could not get digit from char"); - } - - while chars.peek().map(|c| c.is_numeric()) == Some(true) { - w = w * 10 - + chars - .next() - .expect("could not get next digit") - .to_digit(10) - .expect("could not get digit from char"); - } - let width = match first_char { - '0' => Width::LeadingZeros(w as usize), - '.' => Width::Decimals(w as usize), - _ => Width::Cell(w as usize), - }; - match chars.next() { - Some(c) => (c, width), - _ => { - let span = params[0].span(); - bail!(span.error( - "missing format verb after `%width` at end of format string" - )); - } - } - } - Some(c) => (c, Width::None), - None => { - let span = params[0].span(); - bail!(span.error("missing format verb after `%` at end of format string")); - } - }, - Some(c) => { - s.push(c); - continue; - } - None => break, + let Some(percent_offset) = fmt[cursor..].find('%') else { + s.push_str(&fmt[cursor..]); + enforce_limit()?; + break; }; + let percent = cursor + percent_offset; + s.push_str(&fmt[cursor..percent]); + enforce_limit()?; + + let (spec, verb, next_cursor) = sprintf_format::parse( + fmt.as_ref(), + percent + 1, + args.as_ref(), + &mut args_idx, + &mut reordered, + args_span, + format_span, + )?; + cursor = next_cursor; + + if verb == '%' { + s.push('%'); + enforce_limit()?; + continue; + } + + if verb != 'q' + && (spec.flags.alternate || spec.flags.plus || spec.flags.minus || spec.flags.space) + { + bail!(format_span.error( + "sprintf flags '#', '+', '-' and space are currently supported only for %q" + )); + } if args_idx >= args.len() { bail!(args_span @@ -375,13 +490,11 @@ fn sprintf(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> } let arg = &args[args_idx]; args_idx += 1; + let width = spec.legacy_width(); // Handle Golang flags. let emit_sign = false; let leave_space_for_elided_sign = false; - // Note: Golang flags come BEFORE the format verb, not after. - // This code was incorrectly consuming characters after the verb. - // Removing the incorrect flag handling to fix sprintf spacing. let get_sign_value = |f: &Number| match (emit_sign, f) { (_, v) if v < &Number::from(0.0) => ("-", v.clone()), @@ -480,25 +593,20 @@ fn sprintf(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> s += format!("{v}").as_str() } } + ('q', value) => append_go_quoted_value(&mut s, value, spec)?, + (_, Value::Number(_)) => { bail!(args_span.error(&format!("number specified for format verb {verb}."))); } - ('q', Value::String(sv)) => append_go_quoted(&mut s, sv.as_ref(), width)?, - - ('+', _) if chars.next() == Some('v') => { - bail!(args_span.error("Go-syntax fields names format verm %#v is not supported.")); - } - ('T', _) | ('#', _) | ('q', _) | ('p', _) => { - bail!( - args_span.error("Go-syntax format verbs %#v. %q, %p and %T are not supported.") - ); + ('T', _) | ('p', _) => { + bail!(args_span.error("Go-syntax format verbs %#v, %p and %T are not supported.")); } _ => {} } } - if args_idx < args.len() { + if !reordered && args_idx < args.len() { bail!(args_span.error( format!( "extra arguments ({}) specified for {args_idx} format verbs.", @@ -761,7 +869,7 @@ mod tests { fn go_quote_string(s: &str) -> String { let mut out = String::new(); - append_go_quoted(&mut out, s, Width::None).expect("quoting must succeed"); + append_go_quoted(&mut out, s, FormatSpec::default()).expect("quoting must succeed"); out } @@ -811,17 +919,75 @@ mod tests { #[test] fn quote_string_applies_supported_width_and_precision() { - let quote = |input, width| { + let quote = |input, spec| { let mut out = String::new(); - append_go_quoted(&mut out, input, width).expect("quoting must succeed"); + append_go_quoted(&mut out, input, spec).expect("quoting must succeed"); out }; - assert_eq!(quote("foo", Width::Cell(10)), " \"foo\""); - assert_eq!(quote("a", Width::LeadingZeros(5)), "00\"a\""); - assert_eq!(quote("abcdef", Width::Decimals(3)), "\"abc\""); - assert_eq!(quote("abc", Width::Decimals(0)), "\"\""); - assert_eq!(quote("\u{1F642}", Width::Cell(6)), " \"\u{1F642}\""); - assert_eq!(quote("\u{1F642}x", Width::Decimals(1)), "\"\u{1F642}\""); + assert_eq!( + quote( + "foo", + FormatSpec { + width: Some(10), + ..FormatSpec::default() + } + ), + " \"foo\"" + ); + assert_eq!( + quote( + "a", + FormatSpec { + flags: FormatFlags { + zero: true, + ..FormatFlags::default() + }, + width: Some(5), + precision: None, + } + ), + "00\"a\"" + ); + assert_eq!( + quote( + "abcdef", + FormatSpec { + precision: Some(3), + ..FormatSpec::default() + } + ), + "\"abc\"" + ); + assert_eq!( + quote( + "abc", + FormatSpec { + precision: Some(0), + ..FormatSpec::default() + } + ), + "\"\"" + ); + assert_eq!( + quote( + "\u{1F642}", + FormatSpec { + width: Some(6), + ..FormatSpec::default() + } + ), + " \"\u{1F642}\"" + ); + assert_eq!( + quote( + "\u{1F642}x", + FormatSpec { + precision: Some(1), + ..FormatSpec::default() + } + ), + "\"\u{1F642}\"" + ); } } diff --git a/src/builtins/strings/sprintf_format.rs b/src/builtins/strings/sprintf_format.rs new file mode 100644 index 00000000..5c2f74da --- /dev/null +++ b/src/builtins/strings/sprintf_format.rs @@ -0,0 +1,137 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use crate::lexer::Span; +use crate::value::{Array, Value}; + +use alloc::format; +use anyhow::{anyhow, bail, Result}; + +use super::FormatSpec; + +const MAX_FORMAT_VALUE: usize = 1_000_000; + +fn parse_usize(bytes: &[u8], cursor: &mut usize) -> Result> { + let start = *cursor; + let mut value = 0usize; + while *cursor < bytes.len() && bytes[*cursor].is_ascii_digit() { + value = value + .checked_mul(10) + .and_then(|value| value.checked_add((bytes[*cursor] - b'0') as usize)) + .filter(|value| *value <= MAX_FORMAT_VALUE) + .ok_or_else(|| anyhow!("sprintf width or precision is too large"))?; + *cursor += 1; + } + Ok((*cursor != start).then_some(value)) +} + +fn parse_index(bytes: &[u8], cursor: &mut usize) -> Result> { + if bytes.get(*cursor) != Some(&b'[') { + return Ok(None); + } + let mut end = *cursor + 1; + let Some(index) = parse_usize(bytes, &mut end)? else { + bail!("sprintf argument index must contain a decimal number"); + }; + if bytes.get(end) != Some(&b']') || index == 0 { + bail!("invalid sprintf argument index"); + } + *cursor = end + 1; + Ok(Some(index - 1)) +} + +fn take_integer(args: &Array, args_idx: &mut usize, args_span: &Span) -> Result { + let index = *args_idx; + let Some(value) = args.get(index) else { + bail!(args_span.error(format!("no argument specified for format verb {index}").as_str())); + }; + *args_idx += 1; + match value { + Value::Number(number) if number.is_integer() => number.as_i64().ok_or_else(|| { + args_span.error("sprintf width or precision is outside the supported range") + }), + _ => bail!(args_span.error("sprintf width or precision must be an integer")), + } +} + +fn checked_dynamic_value(value: u64, args_span: &Span) -> Result { + usize::try_from(value) + .ok() + .filter(|value| *value <= MAX_FORMAT_VALUE) + .ok_or_else(|| args_span.error("sprintf width or precision is outside the supported range")) +} + +pub(super) fn parse( + format: &str, + start: usize, + args: &Array, + args_idx: &mut usize, + reordered: &mut bool, + args_span: &Span, + format_span: &Span, +) -> Result<(FormatSpec, char, usize)> { + let bytes = format.as_bytes(); + let mut cursor = start; + let mut spec = FormatSpec::default(); + + while cursor < bytes.len() { + match bytes[cursor] { + b'#' => spec.flags.alternate = true, + b'0' => spec.flags.zero = true, + b'+' => spec.flags.plus = true, + b'-' => spec.flags.minus = true, + b' ' => spec.flags.space = true, + _ => break, + } + cursor += 1; + } + + if let Some(index) = parse_index(bytes, &mut cursor)? { + *args_idx = index; + *reordered = true; + } + + if bytes.get(cursor) == Some(&b'*') { + cursor += 1; + let width = take_integer(args, args_idx, args_span)?; + if width < 0 { + spec.flags.minus = true; + spec.flags.zero = false; + spec.width = Some(checked_dynamic_value(width.unsigned_abs(), args_span)?); + } else { + spec.width = Some(checked_dynamic_value(width as u64, args_span)?); + } + } else { + spec.width = parse_usize(bytes, &mut cursor)?; + } + + if bytes.get(cursor) == Some(&b'.') { + cursor += 1; + if let Some(index) = parse_index(bytes, &mut cursor)? { + *args_idx = index; + *reordered = true; + } + if bytes.get(cursor) == Some(&b'*') { + cursor += 1; + let precision = take_integer(args, args_idx, args_span)?; + if precision >= 0 { + spec.precision = Some(checked_dynamic_value(precision as u64, args_span)?); + } + } else { + spec.precision = Some(parse_usize(bytes, &mut cursor)?.unwrap_or_default()); + } + } + + if let Some(index) = parse_index(bytes, &mut cursor)? { + *args_idx = index; + *reordered = true; + } + + let Some(rest) = format.get(cursor..) else { + bail!(format_span.error("invalid byte offset in sprintf format string")); + }; + let Some(verb) = rest.chars().next() else { + bail!(format_span.error("missing format verb at end of format string")); + }; + Ok((spec, verb, cursor + verb.len_utf8())) +} diff --git a/tests/interpreter/cases/builtins/strings/sprintf.yaml b/tests/interpreter/cases/builtins/strings/sprintf.yaml index 6ada0858..3a03378c 100644 --- a/tests/interpreter/cases/builtins/strings/sprintf.yaml +++ b/tests/interpreter/cases/builtins/strings/sprintf.yaml @@ -145,22 +145,64 @@ cases: unicode_width: " \"🙂\"" unicode_precision: "\"🙂\"" - - note: "%q on a number still errors" + - note: "%q full string format specification" data: {} modules: - | package test - error_case := sprintf("%q", [42]) + ascii_only := sprintf("%+q", ["é🙂"]) + raw := sprintf("%#q", ["a\tb"]) + raw_fallback := sprintf("%#q", ["a\nb"]) + left_aligned := sprintf("%-8q", ["é"]) + width_and_precision := sprintf("%8.1q", ["éx"]) + ascii_width_and_precision := sprintf("%+14.1q", ["éx"]) + raw_width_and_precision := sprintf("%#8.2q", ["abx"]) + dynamic := sprintf("%*.*q", [8, 1, "éx"]) + indexed := sprintf("%[3]*.[2]*[1]q", ["éx", 1, 8]) query: data.test - error: "number specified for format verb q." - - - note: "%q on a non-string, non-number value still errors (only Value::String is supported)" + want_result: + ascii_only: "\"\\u00e9\\U0001f642\"" + raw: "`a\tb`" + raw_fallback: "\"a\\nb\"" + left_aligned: "\"é\" " + width_and_precision: " \"é\"" + ascii_width_and_precision: " \"\\u00e9\"" + raw_width_and_precision: " `ab`" + dynamic: " \"é\"" + indexed: " \"é\"" + + - note: "%q follows OPA value conversion semantics" data: {} modules: - | package test - error_case := sprintf("%q", [true]) + rune := sprintf("%q", [97]) + control_rune := sprintf("%q", [0]) + quote_rune := sprintf("%q", [39]) + invalid_rune := sprintf("%q", [-1]) + ascii_rune := sprintf("%+q", [233]) + padded_rune := sprintf("%8q", [97]) + fractional := sprintf("%q", [65.5]) + big_integer := sprintf("%q", [9223372036854775808]) + boolean := sprintf("%q", [true]) + null_value := sprintf("%q", [null]) + array := sprintf("%q", [[1, "x"]]) + set := sprintf("%q", [{true, 1, "x"}]) + object := sprintf("%q", [{"b": 2, "a": 1}]) query: data.test - error: "Go-syntax format verbs %#v. %q, %p and %T are not supported." + want_result: + rune: "'a'" + control_rune: "'\\x00'" + quote_rune: "'\\''" + invalid_rune: "'�'" + ascii_rune: "'\\u00e9'" + padded_rune: " 'a'" + fractional: "%!q(float64=65.5)" + big_integer: "%!q(big.Int=9223372036854775808)" + boolean: "\"true\"" + null_value: "\"null\"" + array: "\"[1, \\\"x\\\"]\"" + set: "\"{true, 1, \\\"x\\\"}\"" + object: "\"{\\\"a\\\": 1, \\\"b\\\": 2}\"" From 005262f6c6c3cffe143ac76bdf55391866e61bc7 Mon Sep 17 00:00:00 2001 From: vitaliytv Date: Thu, 10 Sep 2026 16:19:17 +0300 Subject: [PATCH 2/4] fix(sprintf): address q format review feedback --- src/builtins/strings.rs | 141 ++++++++++++++++-- src/builtins/strings/sprintf_format.rs | 8 + .../cases/builtins/strings/sprintf.yaml | 10 ++ tests/rvm/rego/cases/sprintf.yaml | 21 +++ 4 files changed, 171 insertions(+), 9 deletions(-) diff --git a/src/builtins/strings.rs b/src/builtins/strings.rs index c5d9181e..1615b0f6 100644 --- a/src/builtins/strings.rs +++ b/src/builtins/strings.rs @@ -242,6 +242,7 @@ struct FormatSpec { flags: FormatFlags, width: Option, precision: Option, + bad_precision: bool, } impl FormatSpec { @@ -294,8 +295,10 @@ fn append_go_quoted(out: &mut String, input: &str, spec: FormatSpec) -> Result<( if raw { out.push('`'); - out.push_str(input); - enforce_limit()?; + for c in input.chars() { + out.push(c); + enforce_limit()?; + } out.push('`'); enforce_limit()?; } else { @@ -413,19 +416,129 @@ fn append_go_quoted_value(out: &mut String, value: &Value, spec: FormatSpec) -> } Value::Number(Number::Float(value)) => { out.push_str("%!q(float64="); - if spec.flags.plus && value.is_sign_positive() { - out.push('+'); - } - out.push_str(&value.to_string()); + append_go_float_value(out, value, spec)?; out.push(')'); enforce_limit() } value => { - let value = to_string(value, false); - enforce_limit()?; - append_go_quoted(out, &value, spec) + let mut rendered = String::new(); + append_value_string(&mut rendered, value, false)?; + append_go_quoted(out, &rendered, spec) + } + } +} + +// Go renders an invalid %q float operand using the supplied flags, width, and +// precision as a nested %v conversion. +fn append_go_float_value(out: &mut String, value: &f64, spec: FormatSpec) -> Result<()> { + let value = if let Some(precision) = spec.precision { + format_go_float_v(*value, precision, spec.flags.alternate) + } else { + format_go_float_v(*value, 6, spec.flags.alternate) + }; + let padding = spec.width.unwrap_or_default().saturating_sub(value.len()); + let padding_char = if spec.flags.zero && !spec.flags.minus { + '0' + } else { + ' ' + }; + if !spec.flags.minus { + append_padding(out, padding, padding_char)?; + } + out.push_str(&value); + enforce_limit()?; + if spec.flags.minus { + append_padding(out, padding, ' ')?; + } + Ok(()) +} + +// Go's %v precision for floats is the number of significant digits, using the +// same general-format threshold as %g. The alternate form retains trailing +// zeroes up to that precision. +fn format_go_float_v(value: f64, precision: usize, alternate: bool) -> String { + if !value.is_finite() || value == 0.0 { + return value.to_string(); + } + let precision = precision.max(1); + let exponent = value.abs().log10().floor() as i32; + let scientific = exponent >= precision as i32 || exponent < -4; + let mut rendered = if scientific { + let fraction_digits = precision - 1; + let rendered = format!("{value:.fraction_digits$e}"); + let (mantissa, exponent) = rendered + .split_once('e') + .expect("scientific format has exponent"); + let mantissa = if alternate { + mantissa.to_owned() + } else { + mantissa + .trim_end_matches('0') + .trim_end_matches('.') + .to_owned() + }; + let exponent = exponent.parse::().expect("Rust exponent is numeric"); + format!("{mantissa}e{exponent:+03}") + } else { + let fraction_digits = (precision as i32 - exponent - 1).max(0) as usize; + format!("{value:.fraction_digits$}") + }; + if !alternate && !scientific { + rendered = rendered + .trim_end_matches('0') + .trim_end_matches('.') + .to_owned(); + } + rendered +} + +fn append_value_string(out: &mut String, value: &Value, unescape: bool) -> Result<()> { + match value { + Value::Null => out.push_str("null"), + Value::Bool(value) => out.push_str(&value.to_string()), + Value::String(value) if unescape => out.push_str( + &serde_json::to_string(value.as_ref()).unwrap_or_else(|_| value.as_ref().to_string()), + ), + Value::String(value) => out.push_str(value.as_ref()), + Value::Number(value) => out.push_str(&value.format_decimal()), + Value::Array(values) => { + out.push('['); + for (index, value) in values.iter().enumerate() { + if index > 0 { + out.push_str(", "); + } + append_value_string(out, value, true)?; + enforce_limit()?; + } + out.push(']'); } + Value::Set(values) => { + out.push('{'); + for (index, value) in values.iter().enumerate() { + if index > 0 { + out.push_str(", "); + } + append_value_string(out, value, true)?; + enforce_limit()?; + } + out.push('}'); + } + Value::Object(values) => { + out.push('{'); + for (index, (key, value)) in values.iter_sorted().enumerate() { + if index > 0 { + out.push_str(", "); + } + append_value_string(out, key, true)?; + out.push_str(": "); + append_value_string(out, value, true)?; + enforce_limit()?; + } + out.push('}'); + } + Value::Undefined => out.push_str("#undefined"), } + enforce_limit() } fn append_hex_escape(out: &mut String, prefix: char, value: u32, digits: usize) { @@ -484,12 +597,21 @@ fn sprintf(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> )); } + if verb != 'q' && spec.width.is_some() && spec.precision.is_some() { + bail!(format_span + .error("sprintf combined width and precision are currently supported only for %q")); + } + if args_idx >= args.len() { bail!(args_span .error(format!("no argument specified for format verb {args_idx}").as_str())); } let arg = &args[args_idx]; args_idx += 1; + if spec.bad_precision { + s.push_str("%!(BADPREC)"); + enforce_limit()?; + } let width = spec.legacy_width(); // Handle Golang flags. @@ -945,6 +1067,7 @@ mod tests { }, width: Some(5), precision: None, + ..FormatSpec::default() } ), "00\"a\"" diff --git a/src/builtins/strings/sprintf_format.rs b/src/builtins/strings/sprintf_format.rs index 5c2f74da..a1e0c27c 100644 --- a/src/builtins/strings/sprintf_format.rs +++ b/src/builtins/strings/sprintf_format.rs @@ -89,6 +89,9 @@ pub(super) fn parse( if let Some(index) = parse_index(bytes, &mut cursor)? { *args_idx = index; *reordered = true; + if matches!(bytes.get(cursor), Some(b'0'..=b'9' | b'.')) { + bail!(format_span.error("invalid sprintf argument index")); + } } if bytes.get(cursor) == Some(&b'*') { @@ -110,12 +113,17 @@ pub(super) fn parse( if let Some(index) = parse_index(bytes, &mut cursor)? { *args_idx = index; *reordered = true; + if matches!(bytes.get(cursor), Some(b'0'..=b'9' | b'.')) { + bail!(format_span.error("invalid sprintf argument index")); + } } if bytes.get(cursor) == Some(&b'*') { cursor += 1; let precision = take_integer(args, args_idx, args_span)?; if precision >= 0 { spec.precision = Some(checked_dynamic_value(precision as u64, args_span)?); + } else { + spec.bad_precision = true; } } else { spec.precision = Some(parse_usize(bytes, &mut cursor)?.unwrap_or_default()); diff --git a/tests/interpreter/cases/builtins/strings/sprintf.yaml b/tests/interpreter/cases/builtins/strings/sprintf.yaml index 3a03378c..ff67bd3a 100644 --- a/tests/interpreter/cases/builtins/strings/sprintf.yaml +++ b/tests/interpreter/cases/builtins/strings/sprintf.yaml @@ -185,6 +185,11 @@ cases: ascii_rune := sprintf("%+q", [233]) padded_rune := sprintf("%8q", [97]) fractional := sprintf("%q", [65.5]) + fractional_width := sprintf("%8q", [65.5]) + fractional_precision := sprintf("%.2q", [65.5]) + fractional_zero_padded := sprintf("%08q", [65.5]) + fractional_alternate := sprintf("%#q", [65.5]) + bad_dynamic_precision := sprintf("%.*q", [-1, "x"]) big_integer := sprintf("%q", [9223372036854775808]) boolean := sprintf("%q", [true]) null_value := sprintf("%q", [null]) @@ -200,6 +205,11 @@ cases: ascii_rune: "'\\u00e9'" padded_rune: " 'a'" fractional: "%!q(float64=65.5)" + fractional_width: "%!q(float64= 65.5)" + fractional_precision: "%!q(float64=66)" + fractional_zero_padded: "%!q(float64=000065.5)" + fractional_alternate: "%!q(float64=65.5000)" + bad_dynamic_precision: "%!(BADPREC)\"x\"" big_integer: "%!q(big.Int=9223372036854775808)" boolean: "\"true\"" null_value: "\"null\"" diff --git a/tests/rvm/rego/cases/sprintf.yaml b/tests/rvm/rego/cases/sprintf.yaml index f01de2a8..4233e52e 100644 --- a/tests/rvm/rego/cases/sprintf.yaml +++ b/tests/rvm/rego/cases/sprintf.yaml @@ -18,3 +18,24 @@ cases: quoted: "\"a\\u2028b\"" width: " \"é\"" precision: "\"é\"" + + - note: sprintf_q_dynamic_indexed_and_value_conversions + data: {} + modules: + - | + package test + + result := { + "dynamic": sprintf("%*.*q", [8, 1, "éx"]), + "indexed": sprintf("%[3]*.[2]*[1]q", ["éx", 1, 8]), + "rune": sprintf("%q", [97]), + "array": sprintf("%q", [[1, "x"]]), + "float": sprintf("%8q", [65.5]), + } + query: data.test.result + want_result: + dynamic: " \"é\"" + indexed: " \"é\"" + rune: "'a'" + array: "\"[1, \\\"x\\\"]\"" + float: "%!q(float64= 65.5)" From 96afe8eadb16737ad43cef120fd1dfd338e461f3 Mon Sep 17 00:00:00 2001 From: vitaliytv Date: Fri, 11 Sep 2026 06:20:59 +0300 Subject: [PATCH 3/4] fix(sprintf): emit Go bad index diagnostics --- src/builtins/strings.rs | 9 +++++++++ src/builtins/strings/sprintf_format.rs | 10 ++++++---- tests/interpreter/cases/builtins/strings/sprintf.yaml | 6 ++++++ tests/rvm/rego/cases/sprintf.yaml | 2 ++ 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/builtins/strings.rs b/src/builtins/strings.rs index 1615b0f6..2c0b00e5 100644 --- a/src/builtins/strings.rs +++ b/src/builtins/strings.rs @@ -243,6 +243,7 @@ struct FormatSpec { width: Option, precision: Option, bad_precision: bool, + bad_index: bool, } impl FormatSpec { @@ -589,6 +590,14 @@ fn sprintf(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> continue; } + if spec.bad_index { + s.push_str("%!"); + s.push(verb); + s.push_str("(BADINDEX)"); + enforce_limit()?; + continue; + } + if verb != 'q' && (spec.flags.alternate || spec.flags.plus || spec.flags.minus || spec.flags.space) { diff --git a/src/builtins/strings/sprintf_format.rs b/src/builtins/strings/sprintf_format.rs index a1e0c27c..9f4773cf 100644 --- a/src/builtins/strings/sprintf_format.rs +++ b/src/builtins/strings/sprintf_format.rs @@ -86,11 +86,16 @@ pub(super) fn parse( cursor += 1; } + let args_idx_before_index = *args_idx; if let Some(index) = parse_index(bytes, &mut cursor)? { *args_idx = index; *reordered = true; if matches!(bytes.get(cursor), Some(b'0'..=b'9' | b'.')) { - bail!(format_span.error("invalid sprintf argument index")); + spec.bad_index = true; + *args_idx = args_idx_before_index; + // A malformed explicit index neither consumes nor reorders an + // argument, but it suppresses the legacy extra-argument check. + *reordered = true; } } @@ -113,9 +118,6 @@ pub(super) fn parse( if let Some(index) = parse_index(bytes, &mut cursor)? { *args_idx = index; *reordered = true; - if matches!(bytes.get(cursor), Some(b'0'..=b'9' | b'.')) { - bail!(format_span.error("invalid sprintf argument index")); - } } if bytes.get(cursor) == Some(&b'*') { cursor += 1; diff --git a/tests/interpreter/cases/builtins/strings/sprintf.yaml b/tests/interpreter/cases/builtins/strings/sprintf.yaml index ff67bd3a..75884bd2 100644 --- a/tests/interpreter/cases/builtins/strings/sprintf.yaml +++ b/tests/interpreter/cases/builtins/strings/sprintf.yaml @@ -190,6 +190,9 @@ cases: fractional_zero_padded := sprintf("%08q", [65.5]) fractional_alternate := sprintf("%#q", [65.5]) bad_dynamic_precision := sprintf("%.*q", [-1, "x"]) + bad_index := sprintf("%[1]2q", ["x"]) + bad_index_precision := sprintf("%[1].2q", ["x"]) + bad_index_does_not_consume := sprintf("%[2]2q %q", ["x"]) big_integer := sprintf("%q", [9223372036854775808]) boolean := sprintf("%q", [true]) null_value := sprintf("%q", [null]) @@ -210,6 +213,9 @@ cases: fractional_zero_padded: "%!q(float64=000065.5)" fractional_alternate: "%!q(float64=65.5000)" bad_dynamic_precision: "%!(BADPREC)\"x\"" + bad_index: "%!q(BADINDEX)" + bad_index_precision: "%!q(BADINDEX)" + bad_index_does_not_consume: "%!q(BADINDEX) \"x\"" big_integer: "%!q(big.Int=9223372036854775808)" boolean: "\"true\"" null_value: "\"null\"" diff --git a/tests/rvm/rego/cases/sprintf.yaml b/tests/rvm/rego/cases/sprintf.yaml index 4233e52e..f27e1880 100644 --- a/tests/rvm/rego/cases/sprintf.yaml +++ b/tests/rvm/rego/cases/sprintf.yaml @@ -31,6 +31,7 @@ cases: "rune": sprintf("%q", [97]), "array": sprintf("%q", [[1, "x"]]), "float": sprintf("%8q", [65.5]), + "bad_index": sprintf("%[1]2q", ["x"]), } query: data.test.result want_result: @@ -39,3 +40,4 @@ cases: rune: "'a'" array: "\"[1, \\\"x\\\"]\"" float: "%!q(float64= 65.5)" + bad_index: "%!q(BADINDEX)" From 36fe599b1f1c1ac72514b5e9af4b8ef07cc55a94 Mon Sep 17 00:00:00 2001 From: Vitalii Tverdokhlib Date: Sun, 13 Sep 2026 08:17:32 +0300 Subject: [PATCH 4/4] fix(sprintf): restore no-std compatibility and bound padding --- src/builtins/strings.rs | 63 ++++++++++++++++--- src/builtins/strings/sprintf_format.rs | 8 +-- .../cases/builtins/strings/sprintf.yaml | 24 +++++++ tests/rvm/rego/cases/sprintf.yaml | 4 ++ 4 files changed, 85 insertions(+), 14 deletions(-) diff --git a/src/builtins/strings.rs b/src/builtins/strings.rs index 2c0b00e5..79c7d7f7 100644 --- a/src/builtins/strings.rs +++ b/src/builtins/strings.rs @@ -23,6 +23,11 @@ use anyhow::{bail, Result}; mod go_is_print; mod sprintf_format; +// Keep formatting directives bounded even when allocator memory limits are +// disabled. This caps both parsed width/precision values and the padding helper +// that materializes width. +const MAX_SPRINTF_WIDTH_OR_PRECISION: usize = 1_000_000; + pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn>) { m.insert("concat", (concat, 2)); m.insert("contains", (contains, 2)); @@ -345,6 +350,9 @@ fn append_go_quoted_body( } fn append_padding(out: &mut String, count: usize, padding_char: char) -> Result<()> { + if count > MAX_SPRINTF_WIDTH_OR_PRECISION { + bail!("sprintf padding is outside the supported range"); + } for _ in 0..count { out.push(padding_char); enforce_limit()?; @@ -462,7 +470,7 @@ fn format_go_float_v(value: f64, precision: usize, alternate: bool) -> String { return value.to_string(); } let precision = precision.max(1); - let exponent = value.abs().log10().floor() as i32; + let exponent = decimal_exponent(value); let scientific = exponent >= precision as i32 || exponent < -4; let mut rendered = if scientific { let fraction_digits = precision - 1; @@ -473,10 +481,7 @@ fn format_go_float_v(value: f64, precision: usize, alternate: bool) -> String { let mantissa = if alternate { mantissa.to_owned() } else { - mantissa - .trim_end_matches('0') - .trim_end_matches('.') - .to_owned() + trim_fraction_zeros(mantissa).to_owned() }; let exponent = exponent.parse::().expect("Rust exponent is numeric"); format!("{mantissa}e{exponent:+03}") @@ -485,14 +490,29 @@ fn format_go_float_v(value: f64, precision: usize, alternate: bool) -> String { format!("{value:.fraction_digits$}") }; if !alternate && !scientific { - rendered = rendered - .trim_end_matches('0') - .trim_end_matches('.') - .to_owned(); + rendered = trim_fraction_zeros(&rendered).to_owned(); } rendered } +// `f64::log10` requires libm in genuine no_std builds. Rust's core formatter +// already computes the normalized scientific representation we need, so parse +// its small decimal exponent instead of introducing a platform math symbol. +fn decimal_exponent(value: f64) -> i32 { + format!("{value:e}") + .rsplit_once('e') + .and_then(|(_, exponent)| exponent.parse().ok()) + .unwrap_or_default() +} + +fn trim_fraction_zeros(value: &str) -> &str { + if value.contains('.') { + value.trim_end_matches('0').trim_end_matches('.') + } else { + value + } +} + fn append_value_string(out: &mut String, value: &Value, unescape: bool) -> Result<()> { match value { Value::Null => out.push_str("null"), @@ -1122,4 +1142,29 @@ mod tests { "\"\u{1F642}\"" ); } + + #[test] + fn float_diagnostics_use_no_std_general_formatting() { + assert_eq!(format_go_float_v(10.0, 6, false), "10"); + assert_eq!(format_go_float_v(100.0, 6, false), "100"); + assert_eq!(format_go_float_v(9.99, 2, false), "10"); + assert_eq!(format_go_float_v(0.0001, 2, false), "0.0001"); + assert_eq!(format_go_float_v(0.00001, 2, false), "1e-05"); + assert_eq!(format_go_float_v(1e20, 6, false), "1e+20"); + assert_eq!(decimal_exponent(f64::MIN_POSITIVE), -308); + assert_eq!(decimal_exponent(f64::from_bits(1)), -324); + } + + #[test] + fn padding_has_a_hard_limit_without_allocator_limits() { + let error = append_padding( + &mut String::new(), + MAX_SPRINTF_WIDTH_OR_PRECISION.saturating_add(1), + ' ', + ) + .expect_err("oversized padding must be rejected"); + assert!(error + .to_string() + .contains("sprintf padding is outside the supported range")); + } } diff --git a/src/builtins/strings/sprintf_format.rs b/src/builtins/strings/sprintf_format.rs index 9f4773cf..a2d6c330 100644 --- a/src/builtins/strings/sprintf_format.rs +++ b/src/builtins/strings/sprintf_format.rs @@ -7,9 +7,7 @@ use crate::value::{Array, Value}; use alloc::format; use anyhow::{anyhow, bail, Result}; -use super::FormatSpec; - -const MAX_FORMAT_VALUE: usize = 1_000_000; +use super::{FormatSpec, MAX_SPRINTF_WIDTH_OR_PRECISION}; fn parse_usize(bytes: &[u8], cursor: &mut usize) -> Result> { let start = *cursor; @@ -18,7 +16,7 @@ fn parse_usize(bytes: &[u8], cursor: &mut usize) -> Result> { value = value .checked_mul(10) .and_then(|value| value.checked_add((bytes[*cursor] - b'0') as usize)) - .filter(|value| *value <= MAX_FORMAT_VALUE) + .filter(|value| *value <= MAX_SPRINTF_WIDTH_OR_PRECISION) .ok_or_else(|| anyhow!("sprintf width or precision is too large"))?; *cursor += 1; } @@ -57,7 +55,7 @@ fn take_integer(args: &Array, args_idx: &mut usize, args_span: &Span) -> Result< fn checked_dynamic_value(value: u64, args_span: &Span) -> Result { usize::try_from(value) .ok() - .filter(|value| *value <= MAX_FORMAT_VALUE) + .filter(|value| *value <= MAX_SPRINTF_WIDTH_OR_PRECISION) .ok_or_else(|| args_span.error("sprintf width or precision is outside the supported range")) } diff --git a/tests/interpreter/cases/builtins/strings/sprintf.yaml b/tests/interpreter/cases/builtins/strings/sprintf.yaml index 75884bd2..43af12e2 100644 --- a/tests/interpreter/cases/builtins/strings/sprintf.yaml +++ b/tests/interpreter/cases/builtins/strings/sprintf.yaml @@ -187,6 +187,8 @@ cases: fractional := sprintf("%q", [65.5]) fractional_width := sprintf("%8q", [65.5]) fractional_precision := sprintf("%.2q", [65.5]) + fractional_rounds_to_integer := sprintf("%.2q", [9.99]) + fractional_scientific_threshold := sprintf("%.2q", [0.00001]) fractional_zero_padded := sprintf("%08q", [65.5]) fractional_alternate := sprintf("%#q", [65.5]) bad_dynamic_precision := sprintf("%.*q", [-1, "x"]) @@ -210,6 +212,8 @@ cases: fractional: "%!q(float64=65.5)" fractional_width: "%!q(float64= 65.5)" fractional_precision: "%!q(float64=66)" + fractional_rounds_to_integer: "%!q(float64=10)" + fractional_scientific_threshold: "%!q(float64=1e-05)" fractional_zero_padded: "%!q(float64=000065.5)" fractional_alternate: "%!q(float64=65.5000)" bad_dynamic_precision: "%!(BADPREC)\"x\"" @@ -222,3 +226,23 @@ cases: array: "\"[1, \\\"x\\\"]\"" set: "\"{true, 1, \\\"x\\\"}\"" object: "\"{\\\"a\\\": 1, \\\"b\\\": 2}\"" + + - note: "sprintf rejects literal width above the hard limit" + data: {} + modules: + - | + package test + + result := sprintf("%1000001q", ["x"]) + query: data.test.result + error: "sprintf width or precision is too large" + + - note: "sprintf rejects dynamic width above the hard limit" + data: {} + modules: + - | + package test + + result := sprintf("%*q", [1000001, "x"]) + query: data.test.result + error: "sprintf width or precision is outside the supported range" diff --git a/tests/rvm/rego/cases/sprintf.yaml b/tests/rvm/rego/cases/sprintf.yaml index f27e1880..e6628eff 100644 --- a/tests/rvm/rego/cases/sprintf.yaml +++ b/tests/rvm/rego/cases/sprintf.yaml @@ -31,6 +31,8 @@ cases: "rune": sprintf("%q", [97]), "array": sprintf("%q", [[1, "x"]]), "float": sprintf("%8q", [65.5]), + "float_rounds_to_integer": sprintf("%.2q", [9.99]), + "float_scientific_threshold": sprintf("%.2q", [0.00001]), "bad_index": sprintf("%[1]2q", ["x"]), } query: data.test.result @@ -40,4 +42,6 @@ cases: rune: "'a'" array: "\"[1, \\\"x\\\"]\"" float: "%!q(float64= 65.5)" + float_rounds_to_integer: "%!q(float64=10)" + float_scientific_threshold: "%!q(float64=1e-05)" bad_index: "%!q(BADINDEX)"