From 759cea713bc8e39f45b844a75358fd780f75d480 Mon Sep 17 00:00:00 2001
From: Stephen Amar <stephen.amar@databricks.com>
Date: Tue, 5 Nov 2024 11:07:17 -0800
Subject: [PATCH] Add better error handling for format (#212)

Add a check to make sure the format array is of the right size and type

- fix https://github.com/databricks/sjsonnet/issues/139
- fix https://github.com/databricks/sjsonnet/issues/185
---
 readme.md                                    |  6 +++-
 sjsonnet/src/sjsonnet/Format.scala           | 36 +++++++++++++++-----
 sjsonnet/test/src/sjsonnet/FormatTests.scala | 13 +++++++
 3 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/readme.md b/readme.md
index f1fed13c..ff329f22 100644
--- a/readme.md
+++ b/readme.md
@@ -282,7 +282,11 @@ to ensure the output bytecode remains compatible with users on older JVMs.
 
 ### 0.4.11
 - Implement `std.isEmpty`, `std.xor`, `std.xnor`, `std.trim`,
-  `std.equalsIgnoreCase`, `std.sha1`, `std.sha256`, `std.sha512`, `std.sha3`
+  `std.equalsIgnoreCase`, `std.sha1`, `std.sha256`, `std.sha512`, `std.sha3` [#204](https://github.com/databricks/sjsonnet/pull/210)
+- fix: std.manifestJsonMinified and empty arrays/objects [#207](https://github.com/databricks/sjsonnet/pull/207)
+- fix: Use different chars for synthetic paths. [#208](https://github.com/databricks/sjsonnet/pull/208)
+- Fix sorting algorithm to work for all array types [#211](https://github.com/databricks/sjsonnet/pull/211)
+- Add better error handling for format [#212](https://github.com/databricks/sjsonnet/pull/212)
 
 ### 0.4.10
 
diff --git a/sjsonnet/src/sjsonnet/Format.scala b/sjsonnet/src/sjsonnet/Format.scala
index e9bfe8e6..1b21085f 100644
--- a/sjsonnet/src/sjsonnet/Format.scala
+++ b/sjsonnet/src/sjsonnet/Format.scala
@@ -101,7 +101,9 @@ object Format{
       val cooked0 = formatted.conversion match{
         case '%' => widenRaw(formatted, "%")
         case _ =>
-
+          if  (values.isInstanceOf[Val.Arr] && i >= values.cast[Val.Arr].length) {
+            Error.fail("Too few values to format: %d, expected at least %d".format(values.cast[Val.Arr].length, i + 1))
+          }
           val raw = formatted.label match{
             case None => values.cast[Val.Arr].force(i)
             case Some(key) =>
@@ -116,9 +118,12 @@ object Format{
           }
           i += 1
           value match{
-            case ujson.Str(s) => widenRaw(formatted, s)
+            case ujson.Str(s) =>
+              if (formatted.conversion != 's' && formatted.conversion != 'c')
+                Error.fail("Format required a number at %d, got string".format(i))
+              widenRaw(formatted, s)
             case ujson.Num(s) =>
-              formatted.conversion match{
+              formatted.conversion match {
                 case 'd' | 'i' | 'u' => formatInteger(formatted, s)
                 case 'o' => formatOctal(formatted, s)
                 case 'x' => formatHexadecimal(formatted, s)
@@ -132,20 +137,33 @@ object Format{
                 case 's' =>
                   if (s.toLong == s) widenRaw(formatted, s.toLong.toString)
                   else widenRaw(formatted, s.toString)
+                case _ => Error.fail("Format required a %s at %d, got string".format(raw.prettyName, i))
+              }
+            case ujson.Bool(s) =>
+              formatted.conversion match {
+                case 'd' | 'i' | 'u' => formatInteger(formatted, s.compareTo(false))
+                case 'o' => formatOctal(formatted, s.compareTo(false))
+                case 'x' => formatHexadecimal(formatted, s.compareTo(false))
+                case 'X' => formatHexadecimal(formatted, s.compareTo(false)).toUpperCase
+                case 'e' => formatExponent(formatted, s.compareTo(false)).toLowerCase
+                case 'E' => formatExponent(formatted, s.compareTo(false))
+                case 'f' | 'F' => formatFloat(formatted, s.compareTo(false))
+                case 'g' => formatGeneric(formatted, s.compareTo(false)).toLowerCase
+                case 'G' => formatGeneric(formatted, s.compareTo(false))
+                case 'c' => widenRaw(formatted, Character.forDigit(s.compareTo(false), 10).toString)
+                case 's' => widenRaw(formatted, s.toString)
+                case _ => Error.fail("Format required a %s at %d, got string".format(raw.prettyName, i))
               }
-            case ujson.True => widenRaw(formatted, "true")
-            case ujson.False => widenRaw(formatted, "false")
             case v => widenRaw(formatted, v.toString)
           }
-
       }
-
       output.append(cooked0)
       output.append(literal)
-
-
     }
 
+    if (values.isInstanceOf[Val.Arr] && i < values.cast[Val.Arr].length) {
+      Error.fail("Too many values to format: %d, expected %d".format(values.cast[Val.Arr].length, i))
+    }
     output.toString()
   }
 
diff --git a/sjsonnet/test/src/sjsonnet/FormatTests.scala b/sjsonnet/test/src/sjsonnet/FormatTests.scala
index a3e4f4a4..b44b8484 100644
--- a/sjsonnet/test/src/sjsonnet/FormatTests.scala
+++ b/sjsonnet/test/src/sjsonnet/FormatTests.scala
@@ -23,6 +23,15 @@ object FormatTests extends TestSuite{
     assert(formatted == expected)
   }
 
+  def checkErr(fmt: String, jsonStr: String, expectedErr: String) = {
+    try {
+      check(fmt, jsonStr, "")
+    } catch {
+      case e: Error =>
+        assert(e.getMessage == expectedErr)
+    }
+  }
+
   def tests = Tests{
     test("hash"){
       // #
@@ -290,6 +299,10 @@ object FormatTests extends TestSuite{
 
       // apparently you can pass in positional parameters to named interpolations
       check("XXX%(ignored_lols)sXXX %s", """[1.1, 2]""", "XXX1.1XXX 2")
+
+      checkErr("%s %s %s %s %s", """["foo"]""", "Too few values to format: 1, expected at least 2")
+      checkErr("%s %s", """["foo", "bar", "baz"]""", "Too many values to format: 3, expected 2")
+      checkErr("%d", """["foo"]""", "Format required a number at 1, got string")
     }
   }
 }