Bug due to Parentheses Simplification

[alixaxel]

I have a very simple rule like this:

path /profile/{key1} {
  create() {
    isUser(key1);
  }

  update() {
    isUser(key1);
  }
}

Using these functions:

function isAuth() {
  (auth != null);
}

function isUser(user) {
  isAuth() && (user == auth.uid);
}

I'd expect the .write output of this to be something along the lines of:

(data.val() == null && (auth != null && $key1 == auth.uid))
||
(data.val() != null && newData.val() != null && (auth != null && $key1 == auth.uid))

Instead, what I get is (note the missing parentheses):

data.val() == null && (auth != null && $key1 == auth.uid)
||
data.val() != null && newData.val() != null && (auth != null && $key1 == auth.uid)

Which is not logically the same - not sure how to fix it since create and update are handled by Bolt itself.

---

Similarly, if I have a compound rule like this:

path /users/{key1} {
  read() {
    isUser(key1) || isAdmin();
  }
}

Relying on these functions:

function isAuth() {
  (auth != null);
}

function isUser(user) {
  isAuth() && (user == auth.uid);
}

function isAdmin() {
  isAuth() && (root.child('admins').child(auth.uid).val() != null);
}

The output I get on .read is the following:

auth != null && $key1 == auth.uid
||
auth != null && root.child('admins').child(auth.uid).val() != null

When I would expect it to be:

(auth != null && $key1 == auth.uid)
||
(auth != null && root.child('admins').child(auth.uid).val() != null)

Adding a redundant boolean check to the function calls:

path /users/{key1} {
  read() {
    (isUser(key1) == true) || (isAdmin() == true);
  }
}

Produces a logically valid output of:

(auth != null && $key1 == auth.uid) == true
||
(auth != null && root.child('access-control').child('admins').child(auth.uid).val() != null) == true

However, it's kind of an obscure behavior.

---

Are these bugs known in Bolt? And if so, is there any way around them without patching the core?

[alixaxel]

Related to #230

[spencerwilson]

Sorry to reanimate this 3 y/o thread, but something is puzzling me about this issue and #230.

@alixaxel, in the first message you asserted that

(data.val() == null && (auth != null && $key1 == auth.uid))
||
(data.val() != null && newData.val() != null && (auth != null && $key1 == auth.uid))

and

data.val() == null && (auth != null && $key1 == auth.uid)
||
data.val() != null && newData.val() != null && (auth != null && $key1 == auth.uid)

are not logically equivalent. I don't follow how that could be, given that according to the table here (reproduced below), && is higher-precedence than ||, and so all the &&-connected expressions should naturally group together.

By the same token, the two rules samples in #230 seem equivalent.

Possible explanations for my confusion:

• Maybe I'm misunderstanding something.
• Maybe in 2018 && and || were equal precedence and so this and #230 reported actual samples of bugs. Sometime between 2018 and 2021, the grammar was changed to give && higher precedence, obviating the need for the parens added in #233.
• Maybe these two issues were never bugs in the first place.
• ...?

cc @rockwotj or @tomlarkworthy

[alixaxel]

@spencerwilson The table that you reproduced, and the following conclusion would be valid for this particular example (I think - quite late here to be thinking straight 😄) however the table you're referring to is for Firestore Security Rules and not for Bolt (which targets Realtime Database Security Rules only) - I believe your confusion arises from this.

You can see Realtime Database operators on https://firebase.google.com/docs/reference/security/database#_and:

Either way, Bolt shouldn't be eating parentheses as that dictates precedence over operator / logical precedence.

[spencerwilson]

Oof, thank you! I had failed to find that last docs page you linked to.

Now that I'm looking at the relevant docs I understand the following to be the case:

• Bolt doesn't specify its operator precedence. (ref) Indeed, its language reference doesn't even name && or || among its supported operators (though they appear in examples).
• Realtime Database also doesn't specify its operator precedence. (ref)
• Strictly speaking, the above imply that operator precedence in both languages is undefined. In turn, there's no sound way to decide in general whether a pair of parentheses is redundant or not.
• => The only reliable way to write Bolt or Realtime Database rules is with parentheses around every sub-expression.

Either way, Bolt shouldn't be eating parentheses as that dictates precedence over operator / logical precedence.

I had previously thought that maybe what you call eating parentheses may have been Bolt intelligently omitting parentheses that it knew would be redundant in Realtime Database's security rule language. But now I know that can't be the case.

I'm now fully on board with the changes you made in #233. Not that my opinion matters 😆 Thank you for taking some time to explain!

[rockwotj]

For what it's worth the RTDB is using JavaScript for it's security rules language, and the AST parser respects the operator precedence rules according to: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence