Don't JSONify arguments passed to BindableEvents

Bug Reports Engine Bugs
#1

There is no need for it as there is no server > client > server going on, it merely means if we want to pass a table, instead of the actual table being passed, it passes a duplicate, meaning any changes made to the table are not made to the actual table being passed to the event and we cannot pass cyclic / whatever roblox sees as cyclic tables.

Put this in the bug report section as this is preventing me from having commands added to my admins command table via an event that is called when it updates and in my opinion is a bug as it just means we have the overhead of obj > json > unjson

1 Like
Bindables should pass through parameters directly rather than serializing/deserializing
#2

The Bindable stuff also works between different contexts (e.g. regular scripts and corescripts), which leads to security issues if it doesn’t get JSONified.

1 Like
#3

I can’t see how that would cause security issues, if you could explain.

Also, where do regular scripts have access to a bindable event used by core scripts?

#4

If arguments weren’t serialized/deserialized when Bindables are invoked across security levels you could do
something like this:

bindableListenedToByCoreScript:Invoke(setmetatable({}, {
    __index = function()
        --This function will be run at CoreScript security level
        --when the corescript tries to index the table.
    end
}))
5 Likes
#5

Surely bindable events could just set __metatable to whatever if the security levels are different between the calling and receiving scripts

#6

Using __metatable won’t change anything. You could make bindables remove the metatable, but that’s even weirder, as that isn’t supposed to happen unless you call setmetatable (and it doesn’t have a __metatable field).

Maybe it should just use a deep copy, which keeps (fixed) cyclic references, non-JSON-able values, …

1 Like
#7

Ideally bindable functions and events would support complex table structures (e.g. nested tables, non-numeric keys) but supporting that would require a huge refactor of our codebase and we’re not sure it’s worth it at the present time.

2 Likes