joshbuddy/jsonpath

Compound queries do not evaluate false expressions properly

gongfarmer opened this issue · 1 comments

When a compound query evaluates down to the expression true && false && true, the result is true.
It should be false.

Example:

$ cat data.json
{
  "data": [
    {
      "hasProperty1": true,
      "hasProperty2": false,
      "name": "testname1"
    },
    {
      "hasProperty1": false,
      "hasProperty2": true,
      "name": "testname2"
    },
    {
      "hasProperty1": true,
      "hasProperty2": true,
      "name": "testname3"
    }
  ]
}

$ ruby -rjson -rjsonpath -e 'data = JSON.parse(IO.read(ARGV.first)); pp JsonPath.new(ARGV[1]).on(data)' data.json '$.data[?( @.hasProperty1 && @.hasProperty2 && @.hasProperty1 )].name'
["testname1", "testname3"]

Expected result: only ["testname3"]

Root cause:
In "parser.rb", method "#parse_parentheses", the sub-expressions have been evaluated and replaced with "true" or "false".
These values are then evaluated against the operators && or ||.
The first of the compound values has its "true" or "false" string passed to #bool_or_exp which converts it to a boolean value.
However the remaining "true" or "false" strings are never evaluated and are passed to &&= or ||= directly, where they are always treated as a true result.

Fixed by this:

diff --git a/lib/jsonpath/parser.rb b/lib/jsonpath/parser.rb
index 79feb8c..7838a8e 100644
--- a/lib/jsonpath/parser.rb
+++ b/lib/jsonpath/parser.rb
@@ -159,9 +159,11 @@ class JsonPath
       res = bool_or_exp(top.shift)
       top.each_with_index do |item, index|
         if item == '&&'
-          res &&= top[index + 1]
+          next_value = bool_or_exp(top[index + 1])
+          res &&= next_value
         elsif item == '||'
-          res ||= top[index + 1]
+          next_value = bool_or_exp(top[index + 1])
+          res ||= next_value
         end
       end

diff --git a/test/test_jsonpath.rb b/test/test_jsonpath.rb
index 5bd18b6..801fc15 100644
--- a/test/test_jsonpath.rb
+++ b/test/test_jsonpath.rb
@@ -80,10 +80,17 @@ class TestJsonpath < MiniTest::Unit::TestCase

   def test_or_operator
     assert_equal [@object['store']['book'][1], @object['store']['book'][3]], JsonPath.new("$..book[?(@['price'] == 13 || @['price'] == 23)]").on(@object)
+    result = ["Sayings of the Century", "Sword of Honour", "Moby Dick", "The Lord of the Rings"]
+    assert_equal result, JsonPath.new("$..book[?(@.price==13 || @.price==9 || @.price==23)].title").on(@object)
+    assert_equal result, JsonPath.new("$..book[?(@.price==9 || @.price==23 || @.price==13)].title").on(@object)
+    assert_equal result, JsonPath.new("$..book[?(@.price==23 || @.price==13 || @.price==9)].title").on(@object)
   end

   def test_and_operator
     assert_equal [], JsonPath.new("$..book[?(@['price'] == 13 && @['price'] == 23)]").on(@object)
+    assert_equal [], JsonPath.new("$..book[?(@.price==13 && @.category==fiction && @.title==no_match)]").on(@object)
+    assert_equal [], JsonPath.new("$..book[?(@.title==no_match && @.category==fiction && @.price==13)]").on(@object)
+    assert_equal [], JsonPath.new("$..book[?(@.price==13 && @.title==no_match && @.category==fiction)]").on(@object)
   end

   def test_and_operator_with_more_results```