Skip to content

Commit

Permalink
ast: Marshal non-string object keys when converting to interface{}
Browse files Browse the repository at this point in the history
Previously, when OPA attempted to convert an ast.Object to interface{}
it would error if the ast.Object contained any keys that were not
ast.String values. This behaviour was implemented because JSON only
supports object keys as strings.

This commit changes the conversion implementation to simply JSON
marshal non-string object keys when they are encountered. This way
we avoid runtime errors which can be difficult to debug.

Fixes open-policy-agent#516

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
  • Loading branch information
tsandall committed Jul 24, 2020
1 parent 35b78b8 commit f506997
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 11 deletions.
13 changes: 6 additions & 7 deletions ast/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,13 @@ func TestCheckInference(t *testing.T) {
}},
{"object-object-key", `x = {{{}: 1}: 1}`, map[Var]types.Type{
Var("x"): types.NewObject(
nil,
types.NewDynamicProperty(
types.NewObject(
[]*types.StaticProperty{types.NewStaticProperty(map[string]interface{}{}, types.N)},
nil,
),
[]*types.StaticProperty{types.NewStaticProperty(
map[string]interface{}{
"{}": json.Number("1"),
},
types.N,
),
)},
nil,
),
}},
{"sets", `x = {1, 2}; y = {{"foo", 1}, x}`, map[Var]types.Type{
Expand Down
13 changes: 9 additions & 4 deletions ast/term.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,20 @@ func ValueToInterface(v Value, resolver Resolver) (interface{}, error) {
if err != nil {
return err
}
asStr, stringKey := ki.(string)
if !stringKey {
return fmt.Errorf("object value has non-string key (%T)", ki)
var str string
var ok bool
if str, ok = ki.(string); !ok {
var buf bytes.Buffer
if err := json.NewEncoder(&buf).Encode(ki); err != nil {
return err
}
str = strings.TrimSpace(buf.String())
}
vi, err := ValueToInterface(v.Value, resolver)
if err != nil {
return err
}
buf[asStr] = vi
buf[str] = vi
return nil
})
if err != nil {
Expand Down
32 changes: 32 additions & 0 deletions docs/content/policy-language.md
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,38 @@ d := {"a": a, "x": [b, c]}

By defining composite values in terms of variables and references, rules can define abstractions over raw data and other rules.

### Objects

Objects are unordered key-value collections. In Rego, any value type can be
used as an object key. For example, the following assignment maps port **numbers**
to a list of IP addresses (represented as strings).

```live:eg/objects:module:merge_down
ips_by_port := {
80: ["1.1.1.1", "1.1.1.2"],
443: ["2.2.2.1"],
}
```
```live:eg/objects/lookup:query:merge_down
ips_by_port[80]
```
```live:eg/objects/lookup:output:merge_down
```
```live:eg/objects/iteration:query:merge_down
some port; ips_by_port[port][_] == "2.2.2.1"
```
```live:eg/objects/iteration:output
```

When Rego values are converted to JSON non-string object keys are marshalled
as strings (because JSON does not support non-string object keys).

```live:eg/objects/marshal:query:merge_down
ips_by_port
```
```live:eg/objects/marshal:output
```

### Sets

In addition to arrays and objects, Rego supports set values. Sets are unordered
Expand Down
5 changes: 5 additions & 0 deletions topdown/topdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func TestTopDownCompleteDoc(t *testing.T) {
{`object/nested composites: {"a": [1], "b": [2], "c": [3]}`,
`p = {"a": [1], "b": [2], "c": [3]} { true }`,
`{"a": [1], "b": [2], "c": [3]}`},
{"object/non-string key:", `p = {1: 2, {3: 4}: 5} { true }`, `{"1": 2, "{\"3\":4}": 5}`},
{"set/nested: {{1,2},{2,3}}", `p = {{1, 2}, {2, 3}} { true }`, "[[1,2], [2,3]]"},
{"vars", `p = {"a": [x, y]} { x = 1; y = 2 }`, `{"a": [1,2]}`},
{"vars conflict", `p = {"a": [x, y]} { xs = [1, 2]; ys = [1, 2]; x = xs[_]; y = ys[_] }`,
Expand Down Expand Up @@ -189,6 +190,7 @@ func TestTopDownPartialObjectDoc(t *testing.T) {
"c": [3, {"v2": 4}]
}`},
{"same key/value pair", `p[k] = 1 { ks = ["a", "b", "c", "a"]; ks[_] = k }`, `{"a":1,"b":1,"c":1}`},
{"non-string key", `p[k] = 1 { ks = [1,{},null]; ks[_] = k }`, `{"1": 1, "{}": 1, "null": 1}`},
}

data := loadSmallTestData()
Expand Down Expand Up @@ -451,6 +453,7 @@ func TestTopDownVirtualDocs(t *testing.T) {
{"input: object dereference ground 2", []string{`p[v] { x = "a"; q[x][y] = v }`, `q[k] = v { k = "a"; v = data.a }`}, "[1,2,3,4]"},
{"input: object defererence non-ground", []string{`p = true { q[0][x][y] = false }`, `q[i] = x { x = c[i] }`}, "true"},
{"input: object ground var key", []string{`p[y] { x = "b"; q[x] = y }`, `q[k] = v { x = {"a": 1, "b": 2}; x[k] = v }`}, "[2]"},
{"input: object non-string key", []string{`p[y] { x = 1; q[x] = y }`, `q[k] = v { x = {2: 1, 1: 3}; x[k] = v }`}, "[3]"},
{"input: variable binding substitution", []string{
`p[x] = y { r[z] = y; q[x] = z }`,
`r[k] = v { x = {"a": 1, "b": 2, "c": 3, "d": 4}; x[k] = v }`,
Expand All @@ -465,6 +468,7 @@ func TestTopDownVirtualDocs(t *testing.T) {
{"output: set dereference deep", []string{`p[y] { q[i][j][k][x] = y }`, `q[{{[1], [2]}, {[3], [4]}}] { true }`}, "[1,2,3,4]"},
{"output: set falsy values", []string{`p[x] { q[x] }`, `q = {0, "", false, null, [], {}, set()} { true }`}, `[0, "", null, [], {}, []]`},
{"output: object key", []string{`p[x] { q[x] = 4 }`, `q[i] = x { a[i] = x }`}, "[3]"},
{"output: object non-string key", []string{`p[x] { q[x] = 1 }`, `q[k] = 1 { a[_] = k; k < 3 }`}, "[1,2]"},
{"output: object value", []string{`p[x] = y { q[x] = y }`, `q[k] = v { b[k] = v }`}, `{"v1": "hello", "v2": "goodbye"}`},
{"output: object embedded", []string{`p[k] = v { {k: [q[k]]} = {k: [v]} }`, `q[x] = y { b[x] = y }`}, `{"v1": "hello", "v2": "goodbye"}`},
{"output: object dereference ground", []string{`p[i] { q[i].x[1] = false }`, `q[i] = x { x = c[i] }`}, "[0]"},
Expand Down Expand Up @@ -1031,6 +1035,7 @@ func TestTopDownComprehensions(t *testing.T) {
}, "[1,2,3,4]"},

{"object simple", []string{`p[i] { xs = {s: x | x = a[_]; format_int(x, 10, s)}; y = xs[i]; y > 1 }`}, `["2","3","4"]`},
{"object non-string key", []string{`p[x] { xs = {k: 1 | a[_] = k}; xs[x]}`}, `[1,2,3,4]`},
{"object nested", []string{`p = r { r = {x: y | z = {i: q | i = b[q]}; x = z[y]}}`}, `{"v1": "hello", "v2": "goodbye"}`},
{"object embedded array", []string{`p[i] { xs = [{s: x | x = a[_]; format_int(x, 10, s)}]; xs[0][i] > 1 }`}, `["2","3","4"]`},
{"object embedded object", []string{`p[i] { xs = {"a": {s: x | x = a[_]; format_int(x, 10, s)}}; xs.a[i] > 1 }`}, `["2","3","4"]`},
Expand Down

0 comments on commit f506997

Please sign in to comment.