Skip to content

table.update errors out in unpredictable, input-dependent ways #8694

@eternaleye

Description

@eternaleye

Brief summary of issue:

When table.update is called with the first table having a key with a non-nil, non-false, non-table value, and the second having the same key with a table value, table.update attempts to recursively call itself in an invalid way, causing a hard error when pairs is given a non-table.

Steps to reproduce the issue:

  1. table.update({ x = 1 }, { x = { y = 2 } })
  2. See it error out in pairs

Error output

bad argument #1 to 'pairs' (table expected, got number)

Extra information, such as the Mudlet version, operating system and ideas for how to solve:

This occurs because table.update uses the following code to perform the recursion:

for k, v in pairs(t2) do
  if type(v) == "table" then
    tbl[k] = table.update(tbl[k] or {}, v)
  else 
    tbl[k] = v
  end
end

This only validates that the value in t2[k] is a table with the if, and the or only validates against t1[k] being nil or false. If it's true, or a number, or a string, then the call will pass the first argument to pairs, which rightly rejects it.

Furthermore, this behavior does not match the documentation, which claims that this function (much like JavaScript's Object.assign) simply operates as a key-wise shadowing operation: the recursion is not only flawed, it is surprising and often unwanted. I discovered this not because of the error (which was actually quite difficult to track down, due to Mudlet not executing functions in protected mode with a handler that emits stack traces), but because of a different issue in my scripts where I relied on the documented behavior to completely replace a table, and this instead merged them.

In order to have a function that is actually usable, I defined the following for myself:

function table.assign(...)
  local result = {}

  for index, item in ipairs{ ... } do
    assert(rawequal(type(item), "table"))
    
    for key, val in pairs(item) do
      result[key] = val
    end
  end
  
  return result
end

This has the further advantage of working with as many arguments as I give it, which I use to work around #7989 by way of

local allmatches = table.assign(unpack(multimatches))

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions