-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: support ldap v3 search #7
Conversation
The loop of |
@kingluo My concern is that if the data has already been read (i.e. there is no more data in the buffer), then calling the reader again will block until read timeout, at which point what is its behavior? 🤔 |
|
Get it, this means that that HTTP request that triggered the LDAP operation is stuck and doesn't block other requests, which is great. |
lib/resty/ldap/client.lua
Outdated
if err then | ||
return nil, "Invalid LDAP message encoding: " .. err | ||
-- Get the data of the specified length | ||
local packet = socket:receive(packet_len) |
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.
Do we need to handle read err here?
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.
added
lib/resty/ldap/client.lua
Outdated
@@ -16,6 +18,15 @@ local _M = {} | |||
local mt = { __index = _M } | |||
|
|||
|
|||
local function to_hex_sequence(data) |
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.
We can use to_hex
in https://github.com/openresty/lua-resty-string/
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.
moved
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.
LGTM
.github/workflows/ci.yml
Outdated
@@ -40,10 +40,11 @@ jobs: | |||
sudo apt -y install lua5.1 luarocks | |||
sudo luarocks install lua_pack | |||
sudo luarocks install lpeg | |||
sudo luarocks install lua-resty-string |
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.
Interesting. Isn't the lua-resty-string a part of the std library?
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.
Maybe, it's a blind spot in my knowledge, let me delete it.
@@ -15,6 +15,7 @@ description = { | |||
dependencies = { | |||
"lua_pack = 2.0.0-0" | |||
"lpeg = 1.0.2-1" | |||
"lua-resty-string = 0.09-0" |
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.
We can remove it too
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.
Let me remove it.
Support building LDAPv3 search request and response data parsing.
In addition, the code uses receiveuntil and while true for reading data, which in my perception is a risk of blocking the worker, and I wonder if there is a good way to improve it (its description and reasons are in the code comments).