Non-eval version#4
Closed
erithmetic wants to merge 9 commits into
Closed
Conversation
Collaborator
|
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 ;) |
|
👍 |
Collaborator
|
Unfortunately there are two issues with this PR
|
Merged
Collaborator
|
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. |
This was referenced Dec 16, 2014
Collaborator
|
Can not be merged at present |
This was referenced Dec 13, 2015
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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?