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.

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

JSONPath.toPathArray returns different values for the same input #102

Closed
andalal opened this issue Aug 21, 2019 · 7 comments · Fixed by #138
Closed

JSONPath.toPathArray returns different values for the same input #102

andalal opened this issue Aug 21, 2019 · 7 comments · Fixed by #138

Comments

@andalal
Copy link

andalal commented Aug 21, 2019

I am using the toPathArray method in the following way

const jsonpathTokens: string[] = JSONPath.toPathArray(propertyRef.propertyPath);

When I pass in the string "$.subsensor.temp" for the first time, it will return ["$", "subsensor", "temp"] as expected. However when the toPathArray is called again with the same input, it returns ["subsensor", "temp"]. This happens for other inputs of a similar type.

When I stepped through the debugger it seems like there is a JSONPath.cache object which is checked before the regex is run. I checked that the value saved to the cache exprList is correct. However on the immediate next call the cache object was already modified to exclude the "$".

I think this is a bug because the toPathArray method should return the same values for the same input.

I am using v1.0.0 and I'm using the library within typescript.

@brettz9
Copy link
Collaborator

brettz9 commented Aug 29, 2019

Are you able to replicate in plain JavaScript?

Running the following in Node (if that is your environment), shows no differences:

const {JSONPath} = require('./');

const a = JSONPath.toPathArray('$.subsensor.temp');

console.log('jsonpath', JSON.stringify(a), JSONPath.cache);

const b = JSONPath.toPathArray('$.subsensor.temp');

console.log('jsonpath', JSON.stringify(b), JSONPath.cache);

And this in the browser:

<!DOCTYPE html>
<html lang="en" dir="ltr">
  <head>
    <meta charset="utf-8">
    <title>Test</title>
  </head>
  <body>
    <script type="module">
      import {JSONPath} from './dist/index-es.js';

      const a = JSONPath.toPathArray('$.subsensor.temp');

      console.log('jsonpath', JSON.stringify(a), JSONPath.cache);

      const b = JSONPath.toPathArray('$.subsensor.temp');

      console.log('jsonpath', JSON.stringify(b), JSONPath.cache);
    </script>
  </body>
</html>

@bjornharrtell
Copy link

I'm seeing the same issue, setting JSONPath.cache = {} before calling JSONPath.toPathArray will work around it.

@brettz9
Copy link
Collaborator

brettz9 commented Oct 11, 2019

Can you prepare a test case or some code to reproduce, @bjornharrtell ?

@bjornharrtell
Copy link

I will try. :)

@tejodorus
Copy link
Contributor

tejodorus commented Nov 19, 2020

I encounter this issue and also have the cause and solution. At the end of toPathArray, the plain cached object is returned:
return cache[expr]
but it should be a clone (return cache[expr].concat()), like before in that function "if (cache[expr]) { return cache[expr].concat();"

This toPathArray is used within "evaluate", and there the result is shifted, which removes the first element. Because it currently is not cloned, the cached item is also shifted, which is not what should happen.

So, the proposed solution is to return cache[expr].concat()

I have pushed my fixes to pull request "Fix for issue 102". I am unable to link this pull request to this issue. #138

@sdolski
Copy link
Contributor

sdolski commented Apr 9, 2021

I am facing the same problem and also tracked it down to the cache.
I can confirm that the analysis, as well as the proposed solution of @tejodorus is correct.

Any chance of geting the linked PR merged?

@brettz9 brettz9 mentioned this issue Apr 9, 2021
3 tasks
brettz9 added a commit that referenced this issue Apr 9, 2021
- Fix: Avoid cache corruption when the returned structure is modified.
    Fixes #102. (@tejodorus)

Co-authored-by: Brett Zamir <brettz9@yahoo.com>
@brettz9
Copy link
Collaborator

brettz9 commented Apr 9, 2021

Released in v5.0.5. Thanks for the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants