Skip to content

Commit

Permalink
ast: Disallowing partial object rules to have other partial object ru…
Browse files Browse the repository at this point in the history
…le within their immediate extent (#5864)

The compiler should disallow single-value rules to have other rules within their extent.
Previously, the compiler would erroneously allow such overlaps, where the immediate "child" rule
of a partial object rule in the rule tree was also a partial object rule.
This condition is not expected during evaluation, which will result in a broken object merge.

Fixes: #5855
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
  • Loading branch information
johanfylling committed Apr 25, 2023
1 parent f4af919 commit 3311587
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 0 deletions.
21 changes: 21 additions & 0 deletions ast/compile.go
Expand Up @@ -892,6 +892,9 @@ func (c *Compiler) checkRuleConflicts() {
//
// data.p.q[r] { r := input.r } # data.p.q could be { "r": true }
// data.p.q.r.s { true }
//
// data.p[r] := x { r = input.key; x = input.bar }
// data.p.q[r] := x { r = input.key; x = input.bar }

// But this is allowed:
// data.p.q[r] = 1 { r := "r" }
Expand All @@ -900,7 +903,25 @@ func (c *Compiler) checkRuleConflicts() {
if r.Head.RuleKind() == SingleValue && len(node.Children) > 0 {
if len(ref) > 1 && !ref[len(ref)-1].IsGround() { // p.q[x] and p.q.s.t => check grandchildren
for _, c := range node.Children {
grandchildrenFound := false

if len(c.Values) > 0 {
childRules := extractRules(c.Values)
for _, childRule := range childRules {
childRef := childRule.Ref()
if childRule.Head.RuleKind() == SingleValue && !childRef[len(childRef)-1].IsGround() {
// The child is a partial object rule, so it's effectively "generating" grandchildren.
grandchildrenFound = true
break
}
}
}

if len(c.Children) > 0 {
grandchildrenFound = true
}

if grandchildrenFound {
singleValueConflicts = node.flattenChildren()
break
}
Expand Down
36 changes: 36 additions & 0 deletions ast/compile_test.go
Expand Up @@ -1951,6 +1951,42 @@ func TestCompilerCheckRuleConflictsDotsInRuleHeads(t *testing.T) {
`),
err: "rego_type_error: single-value rule data.pkg.p.q[r] conflicts with [data.pkg.p.q.r.s]",
},
{
note: "single-value partial object with other partial object rule overlap, unknown keys (regression test for #5855)",
modules: modules(
`package pkg
p[r] := x { r = input.key; x = input.bar }
p.q[r] := x { r = input.key; x = input.bar }
`),
err: "rego_type_error: single-value rule data.pkg.p[r] conflicts with [data.pkg.p.q[r]]",
},
{
note: "single-value partial object with other partial object (implicit 'true' value) rule overlap, unknown keys",
modules: modules(
`package pkg
p[r] := x { r = input.key; x = input.bar }
p.q[r] { r = input.key }
`),
err: "rego_type_error: single-value rule data.pkg.p[r] conflicts with [data.pkg.p.q[r]]",
},
{
note: "single-value partial object with multi-value rule (ref head) overlap, unknown key",
modules: modules(
`package pkg
import future.keywords
p[r] := x { r = input.key; x = input.bar }
p.q contains r { r = input.key }
`),
},
{
note: "single-value partial object with multi-value rule overlap, unknown key",
modules: modules(
`package pkg
p[r] := x { r = input.key; x = input.bar }
p.q { true }
`),
err: "rego_type_error: conflicting rules data.pkg.p found",
},
{
note: "single-value rule with known and unknown key",
modules: modules(
Expand Down

0 comments on commit 3311587

Please sign in to comment.