baso88 / SC_AngelScript

Sven Co-op AngelScript documentation, tutorials, sample code and tools
41 stars 4 forks source link

File::ReadLine doesn't handle CRLF line endings properly #43

Closed fnky closed 7 years ago

fnky commented 7 years ago

File::ReadLine reads until the given delimiter is reached, which is '\n' by default. On Windows, the OS will read CRLF as LF ('\n'), on Linux this is read as CRLF ("\r\n"), causing problems when lines are read in one at a time. Special handling is required when the delimiter is '\n' to ignore any '\r' preceding it.

fnky commented 7 years ago

The implementation looks like this:

/*
 * The delimiter is a string because characters are not supported
 */
void CASFile::ReadLine( CString& szOutLine, const CString& szDelim )
{
    if( !IsReading() )
        return;

    if( szDelim.IsEmpty() || !IsOpen() )
    {
      szOutLine = "";
      return;
    }

    const char cDelim = szDelim[ 0 ];
    std::ostringstream outStream;
    char character;
    bool fContinue = true;

    while( fContinue )
    {
        fContinue = g_pFileSystem->IsOk( m_hHandle ) && !g_pFileSystem->EndOfFile( m_hHandle );

        if( fContinue )
            fContinue = g_pFileSystem->Read( &character, 1, m_hHandle ) != 0;

        if( fContinue )
        {
            if( character == cDelim )
                fContinue = false;
        }

        if( fContinue )
            outStream << character;
    }

    szOutLine = outStream.str().c_str();
  }

There are 2 ways to fix it: removing carriage returns that are encountered before a newline if a newline delimiter was specified, or removing all carriage returns. I will be providing the fix for the former only.

At the end of the method, add this:

//If the delimiter is a newline, strip any carriage returns at the end of the string.
//Fixes #46
if( cDelim == '\n' && !szOutLine.IsEmpty() && szOutLine[ szOutLine.Length() - 1 ] == '\r' )
{
    szOutLine.Resize( szOutLine.Length() - 1 );
}

This will strip the carriage return if it occurs at the end of the line.

baso88 commented 7 years ago

The fix for this issue will be included in the next update (5.13). Closing.