Skip to content

Commit

Permalink
Improve sprintf number handling
Browse files Browse the repository at this point in the history
Previously, all numbers were converted to strings before invoking
fmt.Sprintf. As a result, policies could not use fmt verbs for integer
and floating point values.

With these changes, OPA will try to convert numbers to int or float64
before falling back to string for values greater than max/min
int64/float64.

Fixes open-policy-agent#748

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
  • Loading branch information
tsandall committed Jun 4, 2018
1 parent 5999b25 commit 17e7eed
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 10 deletions.
23 changes: 16 additions & 7 deletions topdown/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,17 +245,26 @@ func builtinSprintf(a, b ast.Value) (ast.Value, error) {
return nil, builtins.NewOperandTypeErr(2, b, "array")
}

strArr := []interface{}{}
args := make([]interface{}, len(astArr))

for i := range astArr {
if str, ok := astArr[i].Value.(ast.String); ok {
strArr = append(strArr, string(str))
} else {
strArr = append(strArr, astArr[i].Value.String())
switch v := astArr[i].Value.(type) {
case ast.Number:
if n, ok := v.Int(); ok {
args[i] = n
} else if f, ok := v.Float64(); ok {
args[i] = f
} else {
args[i] = v.String()
}
case ast.String:
args[i] = string(v)
default:
args[i] = astArr[i].String()
}
}

fmtStr := fmt.Sprintf(string(s), strArr...)
return ast.String(fmtStr), nil
return ast.String(fmt.Sprintf(string(s), args...)), nil
}

func init() {
Expand Down
8 changes: 5 additions & 3 deletions topdown/topdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1306,10 +1306,12 @@ func TestTopDownStrings(t *testing.T) {
{"trim: multi-cutset-none", []string{`p = x { trim("...foo.bar...", ".o", x) }`}, `"foo.bar"`},
{"sprintf: none", []string{`p = x { sprintf("hi", [], x) }`}, `"hi"`},
{"sprintf: string", []string{`p = x { sprintf("hi %s", ["there"], x) }`}, `"hi there"`},
{"sprintf: int", []string{`p = x { sprintf("hi %s", [5], x) }`}, `"hi 5"`},
{"sprintf: float", []string{`p = x { sprintf("hi %s", [3.14], x) }`}, `"hi 3.14"`},
{"sprintf: int", []string{`p = x { sprintf("hi %02d", [5], x) }`}, `"hi 05"`},
{"sprintf: hex", []string{`p = x { sprintf("hi %02X.%02X", [127, 1], x) }`}, `"hi 7F.01"`},
{"sprintf: float", []string{`p = x { sprintf("hi %.2f", [3.1415], x) }`}, `"hi 3.14"`},
{"sprintf: float too big", []string{`p = x { sprintf("hi %v", [2e308], x) }`}, `"hi 2e+308"`},
{"sprintf: bool", []string{`p = x { sprintf("hi %s", [true], x) }`}, `"hi true"`},
{"sprintf: composite", []string{`p = x { sprintf("hi %s", [["there", 5, 3.14]], x) }`}, `"hi [\"there\", 5, 3.14]"`},
{"sprintf: composite", []string{`p = x { sprintf("hi %v", [["there", 5, 3.14]], x) }`}, `"hi [\"there\", 5, 3.14]"`},
}

data := loadSmallTestData()
Expand Down

0 comments on commit 17e7eed

Please sign in to comment.