-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feature: add tcpsock:bind api #712
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
Conversation
optimize: palloc instead of pcalloc. bugfix: fixed alloc 'ngx_addr_t', not reference it. change: modified some logging code. feature: added test cases for tcp bind. (squash by doujiang24)
I complied the patch and used some simple scripts to test them, everything looks good. (Not included in files) Additionally, I tested binding to a bad address, test failed (correct behavior) |
I'll look into this as soon as I can manage :) |
I do have one comment, but it is not directly related to sock:bind() but I found it while testing sock:bind(), so I am posting it here. This applies to all sock:* calls and is related to the luaL_error() call. There is a different behavior for different types of errors. For instance if I try to bind to the ip "A.B.C.D" I get a error as the 2nd return value of the call Whereas if I put some garbage arguments on the end of the sock:bind() call It just seems like there should be only one error path Anyways, not a big deal, but it's a problem I ran into, and this is my (lazy) way of documenting it :) |
@agentzh @JakSprats where would be a better location to set this flag? adding a parameter to @JakSprats, just for my understanding, why did you choose to implement a separate Thanks! |
@JakSprats We distinguish between bad argument errors due to API misuse and other kinds of errors. The former usually results in Lua exceptions so that the user cannot (easily) ignore it. @alonbg I think we'd better add support for the http://w3.impa.br/~diego/software/luasocket/tcp.html#setoption |
@agentzh Thanks! |
@agentzh Please, review and add this code into main branch. Almost two years have passed. |
love to see the bind option merged as well, it is really usefull |
What is the plan with this pull request? We would like to leverage |
This pull request is now in conflict :( |
f924579
to
fef2581
Compare
This pull request is now in conflict :( |
This pull request is now in conflict :( |
This pull request is now in conflict :( |
Stop
…On Fri, Sep 22, 2023, 7:45 PM mergify[bot] ***@***.***> wrote:
This pull request is now in conflict :(
—
Reply to this email directly, view it on GitHub
<#712 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A6QCT4MV4NGCR3HDLEMSYODX3YWJ5ANCNFSM4B6ONP2Q>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
@agentzh @alonbg Is anything in the linked LuaSockets issue still relevant? I'm one of the new maintainers but for that particular issue I'm a bit lost. I'm happy to facilitate if somebody has a contribution and concrete reason why it is needed, but there isn't enough to go on and move it forward as it stands. Just FYI. |
The feature has been merged from the command line. I forgot to close this PR. |
Just like the standard proxy_bind directive, this api makes the outgoing connection to a upstream server originate from the specified local IP address.