Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ast: Marshal non-string object keys when converting to interface{} #2577

Merged
merged 2 commits into from
Jul 27, 2020
Merged

ast: Marshal non-string object keys when converting to interface{} #2577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f506997
ast: Marshal non-string object keys when converting to interface{}
tsandall Jul 24, 2020
5bf4bf0
Merge branch 'master' into non-string-object-keys
tsandall Jul 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions ast/check_test.go
View file
Open in desktop
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
View file
Open in desktop
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of a clash ({"1":0, 1:0}), the key processed second will overwrite the one processed first, right?

I see no solid solution here, only ways to make it less likely, like marshaling the example thing to {"[1]":0,"1":0}.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried it briefly to verify:

> { 1: true, "1": false, "{}": "foo", {}: "bar" }
{
  "1": false,
  "{}": "bar"
}
>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find. Agreed, there's no good solution here. I think this is still better than having policy evaluation return an error. I'm going to go ahead and merge.

return nil
})
if err != nil {
Expand Down
32 changes: 32 additions & 0 deletions docs/content/policy-language.md
View file
Open in desktop
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
View file
Open in desktop
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