Erlang list parsing is incomplete

[tiborebner]

According to the Erlang external term format documentation the list term has a Tail that usually is a Nil (or any other type, for that matter). Current implementation of the list parser misses a Nil for the tail, therefore parsing nested lists incorrectly.

Here is a fix:

public ErlList ReadList()
{
    int arity = readListHead();
    if (arity == 0)
        return new ErlList();

    var list = new List<IErlObject>(arity);
    for (int i=0; i < arity; i++)
        list.Add(Read());

    // Read list tail
    if ((ErlExternalTag)m_Buffer[m_Position] == ErlExternalTag.Nil) {
        Read();
    } else {
        list.Add(Read());
    }

    return new ErlList(list, false);
}

Also, the ErlExternalTag.String case is superfluous in the readListHead() method in ErlInputStream.

[itadapter]

ok, will look with @saleyn

[saleyn]

It's a valid point. I checked, it is handled correctly for decoding strings but not nested lists.

Though your patch doesn't seem to be correct either since if next tag is not Nil, it's a mistake to add the following term to list, since we already added arity number of terms.

I'll fix it.

[tiborebner]

I interpreted the specification so that for a list a Tail must always be there whether it is a Nil or, in the case of an "improper list", another type. That's why I put the else branch there. I'm not sure, however, if I understand the spec correctly. My implementation didn't surface an error during testing, at least that's for sure. Below is an example test case:

[23,4,
 {[{<<"event">>,
    {[{<<"fleet_hash">>,<<"a6a22d14-8483-41fc-a46b-283b9777c2a2">>},
      {<<"type">>,<<"fleet_changed">>}]}}]}]
<<131,108,0,0,0,3,97,23,97,4,104,1,108,0,0,0,1,104,2,109,0,0,0,5,101,118,101,
  110,116,104,1,108,0,0,0,2,104,2,109,0,0,0,10,102,108,101,101,116,95,104,97,
  115,104,109,0,0,0,36,97,54,97,50,50,100,49,52,45,56,52,56,51,45,52,49,102,99,
  45,97,52,54,98,45,50,56,51,98,57,55,55,55,99,50,97,50,104,2,109,0,0,0,4,116,
  121,112,101,109,0,0,0,13,102,108,101,101,116,95,99,104,97,110,103,101,100,
  106,106,106>>

The input was generated from JSON with Jiffy.

[tiborebner]

Here is another test case:

[24,4,{[{<<"event">>,
    {[{<<"data">>,{[{<<"hash">>,<<"a6a22d14-8483-41fc-a46b-283b9777c2a2">>}]}},
      {<<"event_type">>,<<"fleet_changed">>},
      {<<"trip_hash">>,<<"a6a22d14-8483-41fc-a46b-283b9777c2a2">>}]}}]}]
<<131,108,0,0,0,3,97,24,97,4,104,1,108,0,0,0,1,104,2,109,0,0,0,5,101,118,101,
  110,116,104,1,108,0,0,0,3,104,2,109,0,0,0,4,100,97,116,97,104,1,108,0,0,0,1,
  104,2,109,0,0,0,4,104,97,115,104,109,0,0,0,36,97,54,97,50,50,100,49,52,45,56,
  52,56,51,45,52,49,102,99,45,97,52,54,98,45,50,56,51,98,57,55,55,55,99,50,97,
  50,106,104,2,109,0,0,0,10,101,118,101,110,116,95,116,121,112,101,109,0,0,0,
  13,102,108,101,101,116,95,99,104,97,110,103,101,100,104,2,109,0,0,0,9,116,
  114,105,112,95,104,97,115,104,109,0,0,0,36,97,54,97,50,50,100,49,52,45,56,52,
  56,51,45,52,49,102,99,45,97,52,54,98,45,50,56,51,98,57,55,55,55,99,50,97,50,
  106,106,106>>

This was where we run into problems originally.

[saleyn]

You have edge cases. It would likely fail on this one:

3> term_to_binary([[1, [2]],[3]]).
<<131,108,0,0,0,2,108,0,0,0,2,97,1,107,0,1,2,106,107,0,1,3,106>>

[tiborebner]

I ran the test on your case and it actually passed. This is a nested proper list which is handled correctly in all cases. An "improper" list may trigger an error, though. I'm not sure.

[saleyn]

I fixed this issue in the main repository. Will ask @itadapter to push it to github.

[itadapter]

thanks @saleyn I am prepping some other stuff and will push later tonight @tiborebner i'll let you know thanks guys!

[itadapter]

@tiborebner @saleyn pushed latest