Table redefinition does not produce an error, but instead a malformed document
ejpcmac opened this issue · 3 comments
Shorter than words, let’s start with an example:
# config.toml
[my_app."MyApp.Endpoint".url]
scheme = "https"
host = "my-app.com"
port = 443
# Table defined after a subtable
[my_app."MyApp.Endpoint"]
secret_key_base = "secret"
When you try to decode this files, it works, but not as one would expect:
iex> Toml.decode_file("config.toml")
{:ok,
%{
"my_app" => %{
"MyApp.Endpoint" => %{"secret_key_base" => "secret"},
"url" => %{"host" => "my-app.com", "port" => 443, "scheme" => "https"}
}
}}
This means the parser considers the file is the same as:
[my_app.url]
scheme = "https"
host = "my-app.com"
port = 443
# Table defined after a subtable
[my_app."MyApp.Endpoint"]
secret_key_base = "secret"
Which is strange. I’ve had some time wondering why my endpoint wasn’t starting, and hopefully came to this issue. I’ve quickly checked the TOML specifications: I have not seen any statement forbidding this.
Strictly speaking, the example is an invalid document, as you are redefining a table which is already defined (implicitly via the my_app."MyApp.Endpoint".url
key, since it by definition requires that my_app."MyApp.Endpoint"
be a table), which the spec calls out as a no-no.
So this should have raised an error, but didn't, likely because when we walk the keys to "reopen" a table to add a new key, we are not properly handling the case where a key is redefined, and a merge is being performed at the wrong level for some reason.
This is now fixed in master - I've decided that your original example should be valid, since you are not actually redefining a table, but extending it. It is currently a point of contention in the TOML issue tracker, but I think the symmetry of TOML documents representing a flat keyspace makes the most sense, in which case only when redefining a specific key should an error occur.
Pushed to Hex as 0.5.0
@bitwalker That was actually my point of view. Let’s see if the TOML specifications make it clearer in the future. Thank you for the patch!