`skipLine(...)` does not properly handle '\r\n' cases

[TomTheFurry]

The static void skipLine(Cursor* cursor) when encountering a line terminated with \r\n new lines style, it only advance the cursor up to the \n, as it is hardcoded to advance 1 character once it see the new line characters sequence (checked via isEndLine(const Cursor& cursor) which does check both cases of new lines style properly).

This issue in effect while does not cause it to stuck in the main tokenizeText(...) parsing loop, it does however fail to parse an element properly when there is a ;Comment here comment in a line between properties. This failure is caused by readTextElement(...) calling skipWhitespaces(cursor) on many different cases, all which then call skipLine(cursor) if it encounter a comment. This due to above bug cause the cursor to still be on a \n character after the call to skipWhitespaces(...), thus failing the parsing.

A simple fix is to have the skipLine(..) make sure to move the cursor up by 2 characters if it encounter the \r\n style new line.

Background info:

This group of issues are discovered when our team is porting OpenFBX library to run natively on C#. Our company is planning to use OpenFBX to expend model loading support for our game (and game engine), as we are currently using block bench parsing and loading only which provides limited features. While the modified C# port is currently closed source and for internal use only, it is undecided whether we will release the port in any format once it is to a stage we are happy with.

[nem0]

Hi, I can't reproduce the issue. I see skipLine correctly handling comments in \r\n mode. Do you have an fbx file I can try to reproduce this?

[TomTheFurry]

I amon the phone atm so can't give a sample right now, but basically, in here:

https://github.com/nem0/OpenFBX/blob/1c1e86380c5cc78180083d9aed30ff115cf55774/src/ofbx.cpp#LL779C5-L779C6

if (cursor->current < cursor->end) ++cursor->current;

While the 'next line' token is detected correctly on the loop before the linked line, which advances the cursor til it hit the 'next line' token, the linked line then, disregard whether it is a '\n' or '\r\n', and instead just advances 1 character. This makes the 'skipLine()' to fail to skip the actual line.

[nem0]

what happens in case of ;comment\r\n is:

1. cursor goes up to \r
2. isEndLine returns false for \r because it fails on *(cursor.current + 1) != '\n' (emphasis on !=)
3. cursor moves further to \n
4. isEndLine returns true now
5. cursor is move further by one character if (cursor->current < cursor->end) ++cursor->current; so the whole line is correctly skipped

I must be misunderstanding something.

[TomTheFurry]

Oh. That's my mistake then. Apologies for wasting your time here. I did not notice that is a 'not equal' sign instead of a 'equal' sign.

[nem0]

No problem.