Permalink
Browse files

Fix wrong BOM detection

This fixes a bug introduced in 27c6579.
In that commit we are checking if a string starts with a Unicode BOM and
remove the BOM if necessary. Apparently though Qt's startsWith()
function doesn't work as exected here, so it's changed to a manual
comparison in this commit which seems to work fine.

The first person who can explain that behaviour to me gets a free beer
if we meet in person.

See issue #1279.
See issue #1282.
  • Loading branch information...
MKleusberg committed Jan 2, 2018
1 parent 117af5a commit c9c848e99555f816aa183e7dad426e3151e0f51d
Showing with 3 additions and 3 deletions.
  1. +3 −3 src/Data.cpp
@@ -34,16 +34,16 @@ bool startsWithBom(const QByteArray& data)

QByteArray removeBom(QByteArray& data)
{
if(data.startsWith("\xEF\xBB\xBF"))
if(data.left(3) == QByteArray("\xEF\xBB\xBF"))
{
QByteArray bom = data.left(3);
data.remove(0, 3);
return bom;
} else if(data.startsWith("\xFE\xFF") || data.startsWith("\xFF\xFE")) {
} else if(data.left(2) == QByteArray("\xFE\xFF") || data.left(2) == QByteArray("\xFF\xFE")) {
QByteArray bom = data.left(2);
data.remove(0, 2);
return bom;
} else if(data.startsWith("\x00\x00\xFE\xFF") || data.startsWith("\xFF\xFE\x00\x00")) {

This comment has been minimized.

@mgrojo

mgrojo Jan 2, 2018

Contributor

This was always true, since "\x00\x00\xFE\xFF" is actually "" for startsWith, since the first character is in fact the null terminator. Maybe that C design aspect was a bit of a fail 😄

This wasn't easy to catch, but I was intrigued enough to investigate 😉

} else if(data.left(4) == QByteArray("\x00\x00\xFE\xFF") || data.left(4) == QByteArray("\xFF\xFE\x00\x00")) {

This comment has been minimized.

@mgrojo

mgrojo Jan 2, 2018

Contributor

And now these both checks are always false, since QByteArray("\x00\x00\xFE\xFF") has actually length 0 and QByteArray("\xFF\xFE\x00\x00") has length 2. I've changed these to constants correctly initialised so it should now not happen.

QByteArray bom = data.left(4);
data.remove(0, 4);
return bom;

2 comments on commit c9c848e

@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo replied Jan 2, 2018

If we ever met in person it will be a pleasure to take those 🍻 with you 😄

@MKleusberg

This comment has been minimized.

Copy link
Member Author

MKleusberg replied Jan 3, 2018

Thanks for the detailed explanation! It's kind of obvious now but I honestly didn't see it while looking at the code.

And I'll definitely let you know when I come to Spain one day 😉 🍻

Please sign in to comment.