websockify
websockify copied to clipboard
TokenRedis plugin improvements
Various improvements to the TokenRedis plugin maintaining backwards compatibility:
- Avoid creating a new redis connection in each lookup, instead use redis connection pool (each Redis instance automatically creates its own connection pool, so we just have to create the instance in the init method, instead of creating a new instance each time the lookup method is called).
- Support for password protected redis servers
- Support to select the redis database to use
- Support both for json and text format tokens (json seems a bit of an overhead in most cases)
- Verification of the token decoding and return None in case of a decoding error
- Eliminate the dependency with simplejson module since the json module in the standard lib is more than enough for our basic usage of using the loads
- Additional tests to verify all the functionality
Integration testing already performed against a password protected redis server.
Thanks for the contribution! But could you split this to several commits. One for each thing you'd like to fix.
Yes, of course, I was not sure the best way but this was also my initial thinking.
Do you want me to create different PR for each of them so then you can choose which ones to merge?
Let's start with everything here. Hopefully everything is okay. :)
Done!
Sorry for the delays. This is on my to-do list, but I'm a bit too busy with other things at the moment. I will get to this eventually, though. :)
No problem! In the meanwhile I have added the possibility you mentioned for empty options when using default values.
I did a rebase to integrate the fix for the new jwcrypto versions so tests do not break. I hope to not have created a mess.
Now is hard to see what are the commits specific to this PR.
Support for empty options is here:
https://github.com/novnc/websockify/pull/518/commits/8eeef0f6d77be3e6e8174a04a521a29b7a4c74cb
I'm afraid something did indeed go wrong when rebasing. Many other commits are suddenly included. Could you try again? git rebase -i <upstream master branch>
and filtering the resulting list should do the right thing.
Fixed the problem with the rebase.
Everything looks fine now and as, a side effect, we now have a rebase to the latest upstream. Really needed after so many months.
Just a small commit history adjustment and I think it's ready for merging.
Thanks for your work!
Happy New Year!
Sorry for the delay, the history is now adjusted and it looks much better without the back and forth.
When this PR is merged I will create the new one with the connection pool part based on this code.