Hi Tim,
Thanks for the patch. Good catch !
Willy, you can apply it.
Thierry
> On 4 Jan 2018, at 19:32, Tim Duesterhus <t...@bastelstu.be> wrote:
>
> The default value of the pattern in `Socket.receive` is `*l` according
> to the documentation and in the `socket.tcp.receive` method of Lua.
>
> The default value of `wanted` in `int hlua_socket_receive(struct lua_State *)`
> reflects this requirement, but the function fails to ensure this
> nonetheless:
>
> If no parameter is given the top of the Lua stack will have the index 1.
> `lua_pushinteger(L, wanted);` then pushes the default value onto the stack
> (with index 2).
> The following `lua_replace(L, 2);` then pops the top index (2) and tries to
> replace the index 2 with it.
> I am not sure why exactly that happens (possibly, because one cannot replace
> non-existent stack indicies), but this causes the stack index to be lost.
>
> `hlua_socket_receive_yield` then tries to read the stack index 2, to
> determine what to read and get the value `0`, instead of the correct
> HLSR_READ_LINE, thus taking the wrong branch.
>
> Fix this by ensuring that the top of the stack is not replaced by itself.
>
> This bug was introduced in commit 7e7ac32dad1e15c19152d37aaf9ea6b3f00a7226
> (which is the very first commit adding the Socket class to Lua). This
> bugfix should be backported to every branch containing that commit:
> - 1.6
> - 1.7
> - 1.8
>
> A test case for this bug is as follows:
>
> The 'Test' response header will contain an HTTP status line with the
> patch applied and will be empty without the patch applied. Replacing
> the `sock:receive()` with `sock:receive("*l")` will cause the status
> line to appear with and without the patch
>
> http.lua:
> core.register_action("bug", { "http-req" }, function(txn)
> local sock = core.tcp()
> sock:settimeout(60)
> sock:connect("127.0.0.1:80")
> sock:send("GET / HTTP/1.0\r\n\r\n")
> response = sock:receive()
> sock:close()
> txn:set_var("txn.foo", response)
> end)
>
> haproxy.cfg (bits omitted for brevity):
> global
> lua-load /scratch/haproxy/http.lua
>
> frontend fe
> bind 127.0.0.1:8080
> http-request lua.bug
> http-response set-header Test %[var(txn.foo)]
>
> default_backend be
>
> backend be
> server s 127.0.0.1:80
> ---
> src/hlua.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/hlua.c b/src/hlua.c
> index abd096d03..285d25589 100644
> --- a/src/hlua.c
> +++ b/src/hlua.c
> @@ -1869,7 +1869,10 @@ __LJMP static int hlua_socket_receive(struct lua_State
> *L)
>
> /* Set pattern. */
> lua_pushinteger(L, wanted);
> - lua_replace(L, 2);
> +
> + /* Check if we would replace the top by itself. */
> + if (lua_gettop(L) != 2)
> + lua_replace(L, 2);
>
> /* init bufffer, and fiil it wih prefix. */
> luaL_buffinit(L, &socket->b);
> --
> 2.15.1
>
