xpcall error handler result gets coerced incorrectly #94

Closed
opened 2021-08-13 00:36:09 +00:00 by technomancy · 8 comments
technomancy commented 2021-08-13 00:36:09 +00:00 (Migrated from github.com)

When you call error inside an xpcall and your error value is not a table, no matter what you return from your error handler, the resulting error will always be turned into a string.

local function err_handler() return {} end

local ok, err = xpcall(function() error({}) end, err_handler)
print("table err", type(err))

local ok, err = xpcall(function() error("oh no") end, err_handler)
print("string err", type(err))

You can see the difference between luaj and PUC lua:

$ luaj scratch.lua
table err	table
string err	string

$ lua scratch.lua
table err	table
string err	table

This bug prevents the luaunit test framework from working with luaj: https://luaunit.readthedocs.io/en/luaunit_v3_2_1/

When you call `error` inside an xpcall and your error value is not a table, no matter what you return from your error handler, the resulting error will always be turned into a string. ```lua local function err_handler() return {} end local ok, err = xpcall(function() error({}) end, err_handler) print("table err", type(err)) local ok, err = xpcall(function() error("oh no") end, err_handler) print("string err", type(err)) ``` You can see the difference between luaj and PUC lua: ``` $ luaj scratch.lua table err table string err string $ lua scratch.lua table err table string err table ``` This bug prevents the `luaunit` test framework from working with luaj: https://luaunit.readthedocs.io/en/luaunit_v3_2_1/
amsitlab commented 2021-09-29 02:57:01 +00:00 (Migrated from github.com)

but value is an table

local function err_handler() return {} end

local ok, err = xpcall(function() error({}) end, err_handler)
print("table err", err, type(err))

local ok, err = xpcall(function() error("oh no") end, err_handler)
print("string err", err, type(err))

Result

table err 	table: 67b92f0a 	table
string err 	table: 4f970963 	string
but value is an table ```lua local function err_handler() return {} end local ok, err = xpcall(function() error({}) end, err_handler) print("table err", err, type(err)) local ok, err = xpcall(function() error("oh no") end, err_handler) print("string err", err, type(err)) ``` Result ``` table err table: 67b92f0a table string err table: 4f970963 string ```
technomancy commented 2021-09-29 16:59:51 +00:00 (Migrated from github.com)

It looks like a table, but I think it's just because luaj accidentally called tostring on the table and returned that. Either that or the type function is blatantly lying.

It looks like a table, but I think it's just because luaj accidentally called `tostring` on the table and returned that. Either that or the `type` function is blatantly lying.
technomancy commented 2021-09-29 17:01:05 +00:00 (Migrated from github.com)
local function err_handler() return {} end

local ok, err = xpcall(function() error({}) end, err_handler)
print("table err", err.gmatch, type(err))

local ok, err = xpcall(function() error("oh no") end, err_handler)
print("string err", err.gmatch, type(err))
table err	nil	table
string err	function: gmatch	string

Looks like a string to me!

```lua local function err_handler() return {} end local ok, err = xpcall(function() error({}) end, err_handler) print("table err", err.gmatch, type(err)) local ok, err = xpcall(function() error("oh no") end, err_handler) print("string err", err.gmatch, type(err)) ``` ``` table err nil table string err function: gmatch string ``` Looks like a string to me!
amsitlab commented 2021-09-30 04:50:48 +00:00 (Migrated from github.com)

Its correct with code :

local function f (a,b)
  return a + b
end

local function err (x)
  print ("err called", x)
  return "oh no!"
end

local function pcallfun()
    return f(1,2)
end

status, err, ret = xpcall (pcallfun, err)

print (status)
print (err)
print (ret)

Result

┌[ .../usr/tmp ]─[ ]
└─$ java -cp /storage/emulated/0/Archives/Program/jar/luaj-jse-3.0.2.jar lua scratch2.lua
true
3
nil

┌[ .../usr/tmp ]─[ ]
└─$ lua5.2 scratch2.lua
true
3
nil
Its correct with code : ```lua local function f (a,b) return a + b end local function err (x) print ("err called", x) return "oh no!" end local function pcallfun() return f(1,2) end status, err, ret = xpcall (pcallfun, err) print (status) print (err) print (ret) ``` Result ``` ┌[ .../usr/tmp ]─[ ] └─$ java -cp /storage/emulated/0/Archives/Program/jar/luaj-jse-3.0.2.jar lua scratch2.lua true 3 nil ┌[ .../usr/tmp ]─[ ] └─$ lua5.2 scratch2.lua true 3 nil ```
technomancy commented 2021-09-30 06:00:18 +00:00 (Migrated from github.com)

I don't understand what your code is intended to demonstrate. There are no errors involved in that code, so it does not trigger the bug.

My code, which actually does call error, shows the bug because the return value is coerced in a type which is different from what it should be.

Perhaps this would be clearer:

local e = {}
local function err_handler() return e end

local ok, err = xpcall(function() error("oh no") end, err_handler)
print(err == e)

-- $ luaj /tmp/scratch.lua
-- false
-- $ lua5.2 /tmp/scratch.lua
-- true
I don't understand what your code is intended to demonstrate. There are no errors involved in that code, so it does not trigger the bug. My code, which actually does call `error`, shows the bug because the return value is coerced in a type which is different from what it should be. Perhaps this would be clearer: ```lua local e = {} local function err_handler() return e end local ok, err = xpcall(function() error("oh no") end, err_handler) print(err == e) -- $ luaj /tmp/scratch.lua -- false -- $ lua5.2 /tmp/scratch.lua -- true ```
amsitlab commented 2021-09-30 18:19:43 +00:00 (Migrated from github.com)

I don't understand what your code is intended to demonstrate. There are no errors involved in that code, so it does not trigger the bug.

My code, which actually does call error, shows the bug because the return value is coerced in a type which is different from what it should be.

Perhaps this would be clearer:

local e = {}
local function err_handler() return e end

local ok, err = xpcall(function() error("oh no") end, err_handler)
print(err == e)

-- $ luaj /tmp/scratch.lua
-- false
-- $ lua5.2 /tmp/scratch.lua
-- true

Maybe this

> I don't understand what your code is intended to demonstrate. There are no errors involved in that code, so it does not trigger the bug. > > My code, which actually does call `error`, shows the bug because the return value is coerced in a type which is different from what it should be. > > Perhaps this would be clearer: > > ```lua > local e = {} > local function err_handler() return e end > > local ok, err = xpcall(function() error("oh no") end, err_handler) > print(err == e) > > -- $ luaj /tmp/scratch.lua > -- false > -- $ lua5.2 /tmp/scratch.lua > -- true > ``` Maybe [this](https://github.com/luaj/luaj/blob/daf3da94e3cdba0ac6a289148d7e38bd53d3fe64/src/core/org/luaj/vm2/lib/BaseLib.java#L384)
SquidDev commented 2021-09-30 18:28:44 +00:00 (Migrated from github.com)

I think (afraid I'm more familiar with LuaJ 2's codebase) the problem occurs here:

daf3da94e3/src/core/org/luaj/vm2/LuaClosure.java (L570)

errorHook is the function responsible for calling the xpcall callback. It's being called with getMessage rather than getMessageObject, meaning the table is coerced to a string.

In my fork, I found it more convenient to manage the stack unwinding and error function handling within the body of xpcall itself, rather than in the implementation of LuaClosure. But I fear doing anything different is a bit of a breaking change.

I _think_ (afraid I'm more familiar with LuaJ 2's codebase) the problem occurs here: https://github.com/luaj/luaj/blob/daf3da94e3cdba0ac6a289148d7e38bd53d3fe64/src/core/org/luaj/vm2/LuaClosure.java#L570 `errorHook` is the function responsible for calling the xpcall callback. It's being called with `getMessage` rather than `getMessageObject`, meaning the table is coerced to a string. In my fork, I found it more convenient to manage the stack unwinding and error function handling within the body of `xpcall` itself, rather than in the implementation of `LuaClosure`. But I fear doing anything different is a bit of a breaking change.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: open-autonomous-connection/luaj#94