This repository has been archived by the owner on Sep 11, 2019. It is now read-only.
forked from jamesls/fakeredis
-
Notifications
You must be signed in to change notification settings - Fork 4
Support StrictRedis.eval for Lua scripts #9
Merged
Merged
Changes from all commits
Commits
Show all changes
57 commits
Select commit
Hold shift + click to select a range
b8d997d
Limited Lua support
blfoster da8a820
Bump patch version
blfoster 0845484
Remove nightly Python build
blfoster 7ea39cd
Return result from Lua
blfoster 74389b7
Fix table handling
blfoster 58d0763
Fix unit tests
blfoster 77685dc
Fix test
blfoster 0dd661f
Decode non-byte strings
blfoster 507a125
Add eval
blfoster 06e02b5
Restore nightly build
blfoster 0fd83e6
Revert "Restore nightly build"
blfoster f2086fd
Update readme
blfoster 6e0c1b3
Remove eval from unimplemented functions
blfoster 73cc1d7
Code review fallout
blfoster 87c2686
pep8:
blfoster e0e7718
Install lupa for CI
blfoster 121de8c
Code review fallout
blfoster 9de8754
pep8:
blfoster 9e6b32f
Install lupa for CI
blfoster 15232fa
Merge branch 'master' of ../fakeredis
blfoster 9bf4173
More fallout
blfoster bfa38c9
Remove lua from .travis.yml
blfoster 63949ba
More tests
blfoster 975f88c
pcall
blfoster 71a2c1e
Test nested ok response
blfoster 1def833
pep8
blfoster dd23fb1
Fix broken tst
blfoster c91f0b1
More fallout
blfoster ee53358
Remove lua from .travis.yml
blfoster a30fdef
More tests
blfoster b426f0f
pcall
blfoster 6153787
Test nested ok response
blfoster 7fb69f4
pep8
blfoster 49cf337
Merge branch 'master' of ../fakeredis
blfoster 392206c
Remove extra braces
blfoster d5e4ce2
Merge branch 'master' of ../fakeredis
blfoster fe9065d
Fix type
blfoster 296c2ff
Fix decoding
blfoster 95762e2
Merge branch 'master' of ../fakeredis
blfoster fde2b75
Mess with string types some more
blfoster 773f0f4
Merge branch 'master' of ../fakeredis
blfoster b1a0402
Fix test
blfoster 856a031
Merge branch 'master' of ../fakeredis
blfoster acc44b1
Test for nil in table
blfoster 23a43af
More tests
blfoster 525a29b
Fix test
blfoster 267e338
Remove unnecessary exception handler
blfoster 6377669
WIP: fallout
blfoster 244056b
Remove docstring
blfoster 8f869b4
More fallout
blfoster b2b6786
Catch LuaError
blfoster fa252bc
Check number of keys
blfoster f917498
More fallout
blfoster 82e7e65
More edge cases for numkeys
blfoster fdf9ecb
Remove braces
blfoster a6a7cd6
flake8
blfoster 9f4d910
to_bytes
blfoster File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,4 @@ | |
flake8<3.0.0 | ||
nose==1.3.4 | ||
redis==2.10.6 | ||
lupa==1.6 |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,5 +30,8 @@ | |
], | ||
install_requires=[ | ||
'redis', | ||
] | ||
], | ||
extras_require={ | ||
"lua": ['lupa'] | ||
} | ||
) |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is a good approach to preventing global variables being created: it doesn't make the script itself error out, and it doesn't actually prevent the globals from polluting the namespace.
Here is the code in redis itself that implements the protection. It may need some tweaks to adapt it - unfortunately I've never programmed in Lua so I'm not sure.
See also the function above the code in that link - it appears to disable
readfile
anddofile
, presumably for security reasons.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is actually safe, because if we try to set a global variable and error out, the LuaRuntime instance will never be used again; if we try to run another Lua script, we'll get a new LuaRuntime, which will not have that global variable set. Unless there's something I'm missing. I do think this way is a bit more readable, but perhaps that's because I don't know Lua. I could add a few more lines to the unit test asserting that trying to set a global variable can't have a side effect...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't spotted that the runtime gets discarded each time, so it's safer than I thought. I think it will still be different to real redis if a script sets a global variable, then modifies the database: in real redis it would error out as soon as it tries to set the global, whereas in this implementation it will get to modify the database before the error. Another case that will probably behave differently is a script that creates a global and then deletes it again, before your check.
We're starting to get to the point of diminishing returns. If you've got the time and energy to test it and fix things up, then go for it, but I realise that I've made you do a lot more work than you probably expected when you started. If you're running out of steam then this is something that can be left for another day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I don't think it should be possible to modify the database either after setting a global, because _lua_redis_call calls _check_for_lua_globals before executing any command that could change the database.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, I'd missed that subtlety too. So then probably the only case that'll behave differently is if the script creates a global and deletes it again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. I'll leave that alone for now if you're ok with it.