feature: implemented bsd style cosocket receive method. #1310
feature: implemented bsd style cosocket receive method. #1310spacewander wants to merge 5 commits intoopenresty:masterfrom
Conversation
| If a non-number-like string argument is specified, then it is interpreted as a "pattern". The following patterns are supported: | ||
|
|
||
| * `'*a'`: reads from the socket until the connection is closed. No end-of-line translation is performed; | ||
| * `'*b'`: the BSD style receive, return anything once socket receives; |
There was a problem hiding this comment.
Better be consistent: "return" => "returns".
Also, this is better:
"returns anything the socket has already received (won't return if it has not yet received any data)."
|
|
||
| if (bytes == 0) { | ||
| u->ft_type |= NGX_HTTP_LUA_SOCKET_FT_CLOSED; | ||
| return NGX_ERROR; |
There was a problem hiding this comment.
I'm not sure about this. Maybe it's better to return the empty Lua string when the reading part of the connection has been closed by the remote? This should not be considered a fatal error. And we need a test case to cover this code path.
There was a problem hiding this comment.
@agentzh
If we do so, we could not detect if the remote is closed as the same way with sock:receive('*l').
This case is already covered by this line: https://github.com/spacewander/lua-nginx-module/blob/77c34e1f3aec37acb84bf67661ae9cc22b21c2eb/t/058-tcp-socket.t#L3849
| 'Host: localhost\r\n', | ||
| 'Connection: close\r\n\r\n', | ||
| } | ||
| sock:send(req) |
There was a problem hiding this comment.
Better to check the return values of send() and handle any potential errors? Please fix other similar places (if any).
|
|
||
|
|
||
|
|
||
| === TEST 64: *b pattern for receive |
There was a problem hiding this comment.
I suggest you also duplicate this test case in a separate .t test file which requires` the use of mockeagain reading test mode for the extreme case of receiving one byte at a time:
t/058-tcp-socket.t .. 115/416
# Failed test 'TEST 64: *b pattern for receive - response_body - response is expected (repeated req 0, req 0)'
# at /home/agentzh/git/lua-nginx-module/../test-nginx/lib/Test/Nginx/Socket.pm line 1555.
# @@ -1,3 +1,14 @@
# 1
# -22
# +2
# -hello world
# +2
# +h
# +e
# +l
# +l
# +o
# +
# +w
# +o
# +r
# +l
# +d
# Failed test 'TEST 64: *b pattern for receive - response_body - response is expected (repeated req 1, req 0)'
# at /home/agentzh/git/lua-nginx-module/../test-nginx/lib/Test/Nginx/Socket.pm line 1555.
# @@ -1,3 +1,14 @@
# 1
# -22
# +2
# -hello world
# +2
# +h
# +e
# +l
# +l
# +o
# +
# +w
# +o
# +r
# +l
# +d
t/058-tcp-socket.t .. 379/416 # Looks like you failed 2 tests of 416.
There was a problem hiding this comment.
Oh, and maybe mockeagain writing mode too :)
|
Will change the method name and submit in another pr. |
4 participants
This feature is atop #861, with some minor issues fixed.
I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.