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

session fails to decode correct mpack rpc string #17

Open
wmoors opened this issue Jul 8, 2019 · 4 comments
Open

session fails to decode correct mpack rpc string #17

wmoors opened this issue Jul 8, 2019 · 4 comments

Comments

@wmoors
Copy link

wmoors commented Jul 8, 2019

It seems the session:receive fails continuously to decode a correct msgpack rpc string once it first correctly fails on non rpc mpack data.
I can't seem to find a way to fix it except for creating a new session.

Here's a small script to demonstrate.

local mpack = require('mpack')

-- these data were built on the msgpack website with their "try" page
-- JSON: [0,8,"add",[1,2]]
corr_rpc_hex = [[ 94 00 08 a3 61 64 64 92 01 02 ]]

-- JSON: {"this" : "that", "that" : "this", "numbers" : [1,2,3,4], "sub" : { "a" : 1, "b" : 2} }
corr_mpack_hex = [[ 84 a4 74 68 69 73 a4 74 68 61 74 a4 74 68 61 74
                    a4 74 68 69 73 a7 6e 75 6d 62 65 72 73 94 01 02
                    03 04 a3 73 75 62 82 a1 61 01 a1 62 02 ]]

--------------------------------------------------------------------------------
function string.remove_spaces(str)
  return str:gsub("%s+", "")
end

--------------------------------------------------------------------------------
function string.fromhex(str)
    return (str:gsub('..', function (cc)
        return string.char(tonumber(cc, 16))
    end))
end

--------------------------------------------------------------------------------
-- tests

local rpc_data = corr_rpc_hex:remove_spaces():fromhex()
--print(rpc_data)

local mpack_data = corr_mpack_hex:remove_spaces():fromhex()
--print (mpack_data)

local session = mpack.Session({unpack = mpack.Unpacker()})

-- correctly decodes rpc
ok, rpctype, id, method, args, pos = pcall(session.receive, session, rpc_data)
print(ok, rpctype, id, method, args, pos)

-- correctly detects error
--[-[
ok, rpctype, id, method, args, pos = pcall(session.receive, session, mpack_data)
print(ok, rpctype, id, method, args, pos)
--]]

-- fails to decode rpc
ok, rpctype, id, method, args, pos = pcall(session.receive, session, rpc_data)
print(ok, rpctype, id, method, args, pos)

-- correctly decodes again
local session = mpack.Session({unpack = mpack.Unpacker()})

ok, rpctype, id, method, args, pos = pcall(session.receive, session, rpc_data)
print(ok, rpctype, id, method, args, pos)
@tarruda
Copy link
Contributor

tarruda commented Jul 8, 2019

@tarruda
Copy link
Contributor

tarruda commented Jul 8, 2019

Let me elaborate a little bit:

Technically we could allow the session to be recovered by resetting the session struct to its initial values, but the only sane thing to do is discard the connection.

Think of it as a failed assertion, if at a certain point in the stream we receive an invalid byte, it is pretty reasonable to imagine that the other side of the connection has a broken implementation of the protocol, so there's no reason to assume that all the bytes following the invalid one will be valid.

@wmoors
Copy link
Author

wmoors commented Jul 8, 2019

Ok. Thx for the fast reply!

I am not well versed enough in the inner workings of the lua C api to figure out what exactly is going on.
I understand that it is of no use to further parse the data in case of an error, what I am a bit confused about is that it seems that the session contains a state that gets messed up when the data is incorrect.

I wanted to use the session:receive as a check to distinguish between normal msgpack messages and RPC's. Creating an extra check function seems to be the only workaround.

@tarruda
Copy link
Contributor

tarruda commented Jul 8, 2019

I understand that it is of no use to further parse the data in case of an error, what I am a bit confused about is that it seems that the session contains a state that gets messed up when the data is incorrect.

Yes, the "message type" field is not reset when there's an error. We could eventually add a "reset" method that allows reusing the session after a failed parse, but for now the workaround is to create a new session

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants