jqlang / jq

Command-line JSON processor
https://jqlang.github.io/jq/
Other
30.23k stars 1.57k forks source link

Incorrect `Invalid path expression` in destructuring expressions #3128

Open nicowilliams opened 4 months ago

nicowilliams commented 4 months ago

Compare jq -n 'path({} as {$a} | .)' to jq -n 'path(. as {$a} | .)'. The first yields an error: Invalid path expression near attempt to access element "a" of {}, while the latter yields ["a"], but in both cases the expression whose values are being destructured do not contribute to path traversal -- they contribute only to establishment of bindings for the the expression to the right.

This issue was reported by @wader. gojq does not have this bug.

The following patch looks right but needs testing (I'm not sure about which should come first, the SUBEXP_END or the POP):

diff --git a/src/compile.c b/src/compile.c
index e5e65f20..3b48683b 100644
--- a/src/compile.c
+++ b/src/compile.c
@@ -789,7 +789,7 @@ static block bind_alternation_matchers(block matchers, block body) {

   // We don't have any alternations here, so we can use the simplest case.
   if (altmatchers.first == NULL) {
-    return bind_matcher(final_matcher, body);
+    return bind_matcher(final_matcher, BLOCK(gen_op_simple(SUBEXP_END), gen_op_simple(POP), body));
   }

   // Collect var names
@@ -824,7 +824,7 @@ static block bind_alternation_matchers(block matchers, block body) {
   // We're done with these insts now.
   block_free(altmatchers);

-  return bind_matcher(preamble, BLOCK(mb, final_matcher, body));
+  return bind_matcher(preamble, BLOCK(mb, final_matcher, gen_op_simple(SUBEXP_END), gen_op_simple(POP), body));
 }

 block gen_reduce(block source, block matcher, block init, block body) {
@@ -1015,7 +1015,7 @@ block gen_destructure(block var, block matchers, block body) {
     top = BLOCK(top, gen_op_simple(DUP));
   }

-  return BLOCK(top, gen_subexp(var), gen_op_simple(POP), bind_alternation_matchers(matchers, body));
+  return BLOCK(top, gen_op_simple(SUBEXP_BEGIN), gen_subexp(var), gen_op_simple(POP), bind_alternation_matchers(matchers, body));
 }

 // Like gen_var_binding(), but bind `break`'s wildcard unbound variable