Your error only occurs in LuaStandalone, and also goes away if you comment out the line "p.colors = colors".

The problem is that LuaStandalone cannot return data structures to PHP that incorporate references. While it would almost be possible to do this,[1] PHP does a shallow clone when passing the array around unless "&" is used appropriately. So for something like "p.recurse = p", when it got back to PHP $p['recurse'] !== $p although $p['recurse']['recurse'] === $p['recurse']. And if PHP does $x = $p['recurse'], now $x['recurse'] !== $x.

The error doesn't occur on enwiki because LuaSandbox just silently replaces the recursive object with an empty "placeholder". Odd that it was done in two different ways there, that should probably be fixed.

[1]: I looked at it back in December, Lua→PHP is easy but there are unresolved issues for PHP→Lua.

Related URL: https://gerrit.wikimedia.org/r/63565 (Gerrit Change I99c80549aebbd2ae1b7fbf8ab11f8588b40855fd)

Thanks for the explanation. I didn't realize that enwiki was running LuaSandbox. (I'll download and build this later for my own machines.) For now, I follow your logic well enough to understand why this occurs / is difficult to fix. I also appreciate your check-in to make Standalone and Sandbox behave the same. It was quite confusing looking at two different behaviors.

One other question: I know you said this was silently broken in LuaSandbox, but shouldn't enwiki return incorrect data for the MTQ invocation? I tried the below on enwiki's sandbox, and MTQ is returning the same values as FRA. It almost appears as if the MTQ assignment works and the FRA object reference was cloned properly to it (i.e.: "colors.MTQ = FRA".)

<!-- as per the module, both FRA and MTQ are the same for type A -->
<pre>FRA:addTypesAsColor({"A"}, "background:#0079C1; color:#fff;")</pre>
{{#invoke:Infobox road/color|color|country=FRA|type=A}}<br/>
{{#invoke:Infobox road/color|color|country=MTQ|type=A}}<br/>

<!-- as per the module, both FRA and MTQ are the same for type N -->
<pre>FRA:addTypesAsColor({"N"}, "background:#006A4D; color:#fff;")</pre>
{{#invoke:Infobox road/color|color|country=FRA|type=N}}<br/>
{{#invoke:Infobox road/color|color|country=MTQ|type=N}}<br/>

This problem affects only PHP↔Lua. When Lua accesses FRA or MTQ, it works fine.

And it even works for something like this

local p = {}

function p.foo()

return 'ok'

end

p.bar = p.foo

return p

Although I expect PHP sees two different functions rather than two references to the same function, that doesn't really matter much. Off the top of my head, I can't think of a way where you could actually *see* this breakage on enwiki right now.

Okay. Thanks again for the explanation.

So if I understand correctly (and I very well may not), the Infobox_road/color Module will not break on enwiki today (because it is Sandbox / Lua-only). However, https://gerrit.wikimedia.org/r/#/c/63565/ will force it to break in the future? (with the goal that Sandbox and Standalone should behave the same?)

If so, I'll go to the talk page for Infobox_road/color and recommend a change based on this ticket.

If not, and this Module behaves the same on enwiki in the future as it does today, then I'm okay with the situation as is. I'll just need to make some adjustments on my side to set up a Sandbox-like environment.

(In reply to comment #5)

So if I understand correctly (and I very well may not), the
Infobox_road/color
Module will not break on enwiki today (because it is Sandbox / Lua-only).

Basically yes, since the manner in which it breaks is not accessible.

However, https://gerrit.wikimedia.org/r/#/c/63565/ will force it to break in
the future? (with the goal that Sandbox and Standalone should behave the
same?)

Yes, assuming that actually gets merged. In the future it would cause a script error rather than silently breaking, so other situations where the current breakage actually is accessible would generate proper errors.

If so, I'll go to the talk page for Infobox_road/color and recommend a change
based on this ticket.

Please do. As I mentioned earlier, the proper fix is to not place that "colors" subtable in the table of functions being returned to PHP for #invoke.

Should direct access to that colors table be needed by some other module loading this one with require(), you could split it to a separate module (which could probably be loaded with mw.loadData(), if it's going to be used from many #invoke calls in one page) or you could make an accessor function to return it as needed.

Please do. As I mentioned earlier, the proper fix is to not place that "colors"
subtable in the table of functions being returned to PHP for #invoke.

Ah! I now understand your earlier point. I thought that "p.colors = colors" was valid usage (it looks harmless to the untrained eye).

I've posted accordingly to the Talk page: http://en.wikipedia.org/wiki/Module_talk:Infobox_road/color

Thanks again!

As the original writer of the module, I can say that the offending code has been removed. Calls from other modules, in particular the to-be-written Module:Infobox road, should go through p._color.

Thank you,
Alexander

Change 63565 merged by jenkins-bot:
Invalid data should cause an error, not a silent placeholder

https://gerrit.wikimedia.org/r/63565

Change is merged, so marking this as fixed. Note that the luasandbox PHP extension is not part of the normal deployment process, so I'm not sure when this will be deployed to WMF wikis.