bitwalker/toml-elixir

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!