Skip to content
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

HTTP calls do not work with lua 5.4.3 #331

Closed
shoeb751 opened this issue May 26, 2021 · 14 comments · Fixed by #334
Closed

HTTP calls do not work with lua 5.4.3 #331

shoeb751 opened this issue May 26, 2021 · 14 comments · Fixed by #334

Comments

@shoeb751
Copy link

Luasocket bug with Lua 5.4.3

Short Description

HTTP calls do not work with Luasocket 5.4.3 and luasocket.

System Info

Lua: 5.4.3
Luasocket: LuaSocket 3.0-rc1
Distro Archlinux

Test code:

Requires lua-socket and lua-sec to be installed.
If testing using lua5.3, then lua53-socket lua53-sec need to be installed.

local http = require "socket.http"

print("---")
print("Testing https")

print(http.request("https://ffl.shoeb.pw"))

print("---")
print("Testing http")


print(http.request("http://ffl.shoeb.pw"))

print("---")
print("End http")

Expected output:

$ lua5.3 t2.lua 
---
Testing https
Welcome to ffl.shoeb.pw. We shall be serving you soon!!!        200     table: 0x55a7c9e0f4d0   HTTP/1.1 200 OK
---
Testing http
Welcome to ffl.shoeb.pw. We shall be serving you soon!!!        200     table: 0x55a7c9e06510   HTTP/1.1 200 OK
---
End http

Observed output:

$ lua t2.lua 
---
Testing https
Welcome to ffl.shoeb.pw. We shall be serving you soon!!!        200     table: 0x5639643d1800   HTTP/1.1 200 OK
---
Testing http
lua: /usr/share/lua/5.4/socket/http.lua:54: bad argument #1 to 'receive' (string expected, got light userdata)
stack traceback:
        [C]: in function 'socket.http.request'
        t2.lua:12: in main chunk
        [C]: in ?

Explanation

HTTP call does not work. Diving into the source code to see what might be the cause yields the line:

 50 local function receiveheaders(sock, headers)
 51     local line, name, value, err
 52     headers = headers or {}
 53     -- get first line
 54     line, err = sock:receive()
 55     if err then return nil, err end
 56     -- headers go until a blank line is found
 57     while line ~= "" do

As you can see in the above code block, line 54 does not pass any arguments, so the argument that is passed will be self because of the : used in the function call.

I can confirm that this was working before my recent update to lua 5.4.3
So, mostly the recent update is what broke it as luasocket has not been updated for quite some time now.

Not sure if there needs to be some support for 5.4.3 that needs to be built into luasocket

@shoeb751
Copy link
Author

shoeb751 commented May 26, 2021

I can also confirm that this got broken in lua 5.4.3 and used to work with lua 5.4.0 - 5.4.2

To do this those lua versions were locally compiled and tested against.

$ lua-5.4.0/src/lua ~/t2.lua 
---
Testing https
Welcome to ffl.shoeb.pw. We shall be serving you soon!!!        200     table: 0x559a178b0120   HTTP/1.1 200 OK
---
Testing http
Welcome to ffl.shoeb.pw. We shall be serving you soon!!!        200     table: 0x559a178c0320   HTTP/1.1 200 OK
---
End http

$ lua-5.4.1/src/lua ~/t2.lua 
---
Testing https
Welcome to ffl.shoeb.pw. We shall be serving you soon!!!        200     table: 0x556634ef5160   HTTP/1.1 200 OK
---
Testing http
Welcome to ffl.shoeb.pw. We shall be serving you soon!!!        200     table: 0x556634efb4c0   HTTP/1.1 200 OK
---
End http

$ lua-5.4.2/src/lua ~/t2.lua 
---
Testing https
Welcome to ffl.shoeb.pw. We shall be serving you soon!!!        200     table: 0x5573eed77830   HTTP/1.1 200 OK
---
Testing http
Welcome to ffl.shoeb.pw. We shall be serving you soon!!!        200     table: 0x5573eed87120   HTTP/1.1 200 OK
---
End http

$ lua-5.4.3/src/lua ~/t2.lua 
---
Testing https
Welcome to ffl.shoeb.pw. We shall be serving you soon!!!        200     table: 0x55e80ddf0830   HTTP/1.1 200 OK
---
Testing http
lua-5.4.3/src/lua: /usr/share/lua/5.4/socket/http.lua:54: bad argument #1 to 'receive' (string expected, got light userdata)
stack traceback:
        [C]: in function 'socket.http.request'
        /home/yui/t2.lua:12: in main chunk
        [C]: in ?

@pkulchenko
Copy link
Contributor

The fix is easy: all receive() calls in http.lua and tp.lua need to be replaced with receive("*l").

Unfortunately this issue affects all clients that use receive() method from luasocket without parameters (which is a default).

pkulchenko added a commit to pkulchenko/luasocket that referenced this issue Jun 22, 2021
…n receive().

This is the same issue that was fixed in MIME code in f329aae.

Closes lunarmodules#331.
@atealxt
Copy link

atealxt commented Jun 24, 2021

The fix is easy: all receive() calls in http.lua and tp.lua need to be replaced with receive("*l").

Unfortunately this issue affects all clients that use receive() method from luasocket without parameters (which is a default).

I tried haproxytech / haproxy-lua-http which use receive("*l") but still face issue:

http.lua:670: Expect a 'string' for the prefix 

@pkulchenko
Copy link
Contributor

@atealxt, I'm not sure where this error is coming from, but try specifying an empty string as the second parameter (the prefix) to see if this helps: receive("*l", "").

@maikaliu
Copy link

maikaliu commented Aug 8, 2021

the default parameter for receive is not working
in buffer_meth_receive method of buffer.c , put const char* p = luaL_optstring(L, 2, "*l"); to the front of the method can solve this

@atan2005
Copy link

atan2005 commented Sep 5, 2021

Finally, after update buffer.c file to the one as mentioned on brunoos/luasec@34252fb or download buffer.c from https://github.com/brunoos/luasec/tree/master/src/luasocket (v1.0.2), this will make luasocket work with Lua 5.4.3.

On my Windows 10 Pro setup at c:\msys64\mingw64, I have to modify the src/makefile to remove /lua from LDIR_mingw?=lua/$(LUAV)/lua so that it is the same as CDIR_mingw?=lua/$(LUAV), so that it has the "directory structure" of https://w3.impa.br/~diego/software/luasocket/installation.html

I have also issued below for build & install:-
make LUAV=5.4 PLAT=mingw LUAINC_mingw_base=/mingw64/include LUALIB_mingw_base=/mingw64/bin LUAPREFIX_mingw=/mingw64/bin
make LUAV=5.4 PLAT=mingw LUAINC_mingw_base=/mingw64/include LUALIB_mingw_base=/mingw64/bin LUAPREFIX_mingw=/mingw64/bin install

Note1: If mingw was specified, the makefile should have already defined the correct directories, not having user to specify any other locations.
Note2: The linker is linking against lua54.dll but it should be against -llua instead.

@Zash
Copy link
Contributor

Zash commented Oct 17, 2021

Could this be a bug in Lua 5.4 itself?

@Zash
Copy link
Contributor

Zash commented Oct 17, 2021

Oh, or is it choking on the luaL_Buffer, which might end up in the stack slot 2? That would explain "got light userdata".

https://github.com/diegonehab/luasocket/blob/5b18e475f38fcf28429b1cc4b17baee3b9793a62/src/buffer.c#L107-L117

Zash added a commit to Zash/luasocket that referenced this issue Oct 17, 2021
It seems that on Lua 5.4 when calling conn:receive() without arguments,
a lightuserdata related to the luaL_Buffer ends up in position 2. Since
this is not a number, luaL_optstring() complains.
@pkulchenko
Copy link
Contributor

@zach, yes, that's the issue. I submitted a fix in #334, but the fix you're using should work as well.

@Zash
Copy link
Contributor

Zash commented Oct 17, 2021

@pkulchenko Oh sorry I must have scrolled past that PR. Subtly different behavior from what I did tho.

@ttys3
Copy link

ttys3 commented Oct 19, 2021

Before #334 get merged, we can use @pkulchenko 's fork to install, I made a gist:

cd /tmp/ && curl -LO https://gist.github.com/ttys3/31dbf88ee7d708294d8ae5b0a4954424/raw/c74afc3edc1a48c0f7e1c2c9992750301bbb74ff/luasocket-scm-3.rockspec
luarocks install ./luasocket-scm-3.rockspec

@Nomarian
Copy link

Nomarian commented Feb 4, 2022

will this ever be patched or is luasocket abandoned?

@alerque
Copy link
Member

alerque commented Feb 4, 2022

@Nomarian It's not quite abandoned. Per #292 the original author is interested in another maintainer stepping up. I'm interested in doing that and we have several other people who are willing to help, but so far they haven't pulled the trigger and passed the buck. I do hope to get this patched up — this issue in particular has started to break some of my old projects and needs fixing urgently.

greg-el added a commit to snowplow/snowplow-lua-tracker that referenced this issue Feb 25, 2022
- Bump supported lua versions

Lua version 5.1 and up are all widely used across the Lua ecosystem, therefor supporting Lua >= 5.1 is preferable.

- Replace luasocket with lua-curl

Luasocket is likely a simplier option, as on mac/brew, the CURL_DIR flag is required for luacurl to be able to find curl, however luasocket currently cannot make HTTP requests with Lua >= 5.4.3:
lunarmodules/luasocket#331

lua-curl is a suitable alternative.

- Replace bit + base64 modules with base64 package

The bit module, which base64 relied on, was throwing an error while running the base64 test file.

As these were files copied from github with no newer versions, both lib/bit.lua and lib/base64.lua have been replaced with the base64 package:

https://github.com/iskolbin/lbase64

There are two differences:

1. The base64 package adds padding, whereas the previous implementation didn't - this doesn't matter however, as the collector doesn't care if padding is present in base64

2. The previous implemenation threw an error when trying to encode an empty string, this behaviour has been replaced by checking for an empty string on the variable passed into base64.encode
greg-el added a commit to snowplow/snowplow-lua-tracker that referenced this issue Feb 25, 2022
Bump supported lua versions

Lua version 5.1 and up are all widely used across the Lua ecosystem, therefor supporting Lua >= 5.1 is preferable.

Replace luasocket with lua-curl

Luasocket is likely a simplier option, as on mac/brew, the CURL_DIR flag is required for luacurl to be able to find curl, however luasocket currently cannot make HTTP requests with Lua >= 5.4.3:
lunarmodules/luasocket#331

lua-curl is a good alternative.

Replace bit + base64 modules with base64 package

The bit module, which base64 relied on, was throwing an error while running the base64 test file.

As these were files copied from github with no newer versions, both lib/bit.lua and lib/base64.lua have been replaced with the base64 package:

https://github.com/iskolbin/lbase64

There are two differences:

1. The base64 package adds padding, whereas the previous implementation didn't - this doesn't matter however, as the collector doesn't care if padding is present in base64

2. The previous implemenation threw an error when trying to encode an empty string, this behaviour has been replaced by checking for an empty string on the variable passed into base64.encode

Update set and fix tests

Make :toString() return in key sorted order, for use in error message comparison
greg-el added a commit to snowplow/snowplow-lua-tracker that referenced this issue Feb 25, 2022
Bump supported lua versions

Lua version 5.1 and up are all widely used across the Lua ecosystem, therefor supporting Lua >= 5.1 is preferable.

Replace luasocket with lua-curl

Luasocket is likely a simplier option, as on mac/brew, the CURL_DIR flag is required for luacurl to be able to find curl, however luasocket currently cannot make HTTP requests with Lua >= 5.4.3:
lunarmodules/luasocket#331

lua-curl is a good alternative.

Replace bit + base64 modules with base64 package

The bit module, which base64 relied on, was throwing an error while running the base64 test file.

As these were files copied from github with no newer versions, both lib/bit.lua and lib/base64.lua have been replaced with the base64 package:

https://github.com/iskolbin/lbase64

There are two differences:

1. The base64 package adds padding, whereas the previous implementation didn't - this doesn't matter however, as the collector doesn't care if padding is present in base64

2. The previous implemenation threw an error when trying to encode an empty string, this behaviour has been replaced by checking for an empty string on the variable passed into base64.encode

Update set and fix tests

Make :toString() return in key sorted order, for use in error message comparison
greg-el added a commit to snowplow/snowplow-lua-tracker that referenced this issue Feb 28, 2022
* Refurbish dependencies (close #17)

Bump supported lua versions

Lua version 5.1 and up are all widely used across the Lua ecosystem, therefor supporting Lua >= 5.1 is preferable.

Replace luasocket with lua-curl

Luasocket is likely a simpler option, as on mac/brew, the CURL_DIR flag is required for luacurl to be able to find curl, however luasocket currently cannot make HTTP requests with Lua >= 5.4.3:
lunarmodules/luasocket#331

lua-curl is a good alternative.

Replace bit + base64 modules with base64 package

The bit module, which base64 relied on, was throwing an error while running the base64 test file.

As these were files copied from github with no newer versions, both lib/bit.lua and lib/base64.lua have been replaced with the base64 package:

https://github.com/iskolbin/lbase64

There are two differences:

1. The base64 package adds padding, whereas the previous implementation didn't - this doesn't matter, however, as the collector doesn't care if padding is present in base64

2. The previous implementation threw an error when trying to encode an empty string, this behaviour has been replaced by checking for an empty string on the variable passed into base64.encode

Update set and fix tests

Make :toString() return in key sorted order, for use in error message comparison

Add maintainer quickstart to README
@daurnimator
Copy link
Contributor

The repository is now under the lunarmodules github organisation. We should be able to move forward with a solution now.

greg-el added a commit to snowplow/snowplow-lua-tracker that referenced this issue Mar 3, 2022
PR: #19

Bump supported lua versions

Lua version 5.1 and up are all widely used across the Lua ecosystem, therefor supporting Lua >= 5.1 is preferable.

Replace luasocket with lua-curl

Luasocket is likely a simpler option, as on mac/brew, the CURL_DIR flag is required for luacurl to be able to find curl, however luasocket currently cannot make HTTP requests with Lua >= 5.4.3:
lunarmodules/luasocket#331

lua-curl is a good alternative.

Replace bit + base64 modules with base64 package

The bit module, which base64 relied on, was throwing an error while running the base64 test file.

As these were files copied from github with no newer versions, both lib/bit.lua and lib/base64.lua have been replaced with the base64 package:

https://github.com/iskolbin/lbase64

There are two differences:

1. The base64 package adds padding, whereas the previous implementation didn't - this doesn't matter, however, as the collector doesn't care if padding is present in base64

2. The previous implementation threw an error when trying to encode an empty string, this behaviour has been replaced by checking for an empty string on the variable passed into base64.encode

Update set and fix tests

Make :toString() return in key sorted order, for use in error message comparison

Add maintainer quickstart to README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
10 participants