Non-eval version #4

Closed
wants to merge 9 commits into
from

Conversation

Projects
None yet
6 participants

dkastner commented Oct 8, 2011

Just wanted to run this by you. I created a version of JSONPath that doesn't use eval(). The reason for this was to satisfy some security concerns that reviewers at Mozilla Addons had about a plugin I developed that made use of JSONPath. This implementation limits the operations that can be tested on a node to basic comparisons and arithmetic. However, it could be possible to add more features, possibly with some sort of JS parsing library.

For now, this version works well enough for many of the base cases and it covers the suite of integration tests you had defined. (N.B. I am using a different unit testing framework that has better output, run options, and test organization)

Thoughts?

Owner

s3u commented Oct 8, 2011

Thanks for doing this. I will check it out.

Question question - have you done any perf analysis if this change had any negative or positive impact on perf?

Any chance of getting this pulled in? the fact that the function is called eval breaks jshint. Very frustrating.

+1 for pulling in ;)

👍

Owner

s3u commented Dec 7, 2014

Unfortunately there are two issues with this PR

  • Does not merge cleanly
  • Once merged, most tests break as expected results are getting wrapped inside an array of size one.

brettz9 referenced this pull request Dec 10, 2014

Merged

Prototype based #31

Collaborator

brettz9 commented Dec 10, 2014

Any way the original poster is still interested to update this? If it is primarily an issue of removing wrapping when the config is set, I would think that shouldn't be too hard for the OP to fix.

s3u closed this Jun 7, 2015

Owner

s3u commented Jun 7, 2015

Can not be merged at present

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