QDataStream >> QList causes the program to crash when I try to open an invalid file

Donald Duck

I have a program that saves a QList as a file and then opens it like this:

void saveFile(const QFile &file, const QList<Something> &myList){
    QDataStream dataStream(&file);
    dataStream << myList;
}

void openFile(const QFile &file, QList<Something> &myList){
    QDataStream dataStream(&file);
    dataStream >> myList;
    if(dataStream.status() != QDataStream::Ok){
        QMessageBox::critical(nullptr, "", "This file is not valid.");
    }
}

This works great as long as I only use the openFile function on a file created with the saveFile function. The problem is that when I open a file that has nothing to do with my program, it crashes.

Since the file is selected by the user using a QFileDialog, I can't guarantee that they select a valid file. If the user opens an invalid file I would like to show them an error message (which is why I do if(dataStream.status() != QDataStream::Ok)), and I definitely don't want the program to crash.

Is there a way to check if the file contains a valid QList of the type I expect without the program crashing?

Pl45m4

@Donald-Duck

How does your QList look like (What kind of type is Something) ? What data is expected to be in that list?

Does QTextStream work for you (assuming you want to read-in string / text)?
Like used here:

https://doc.qt.io/qt-5/qfile.html#using-streams-to-read-files

Christian Ehrlicher

Use a debugger and see where exactly it crashes. I don't think it crashes inside Qt but inside your Something QDataStream operator.

Donald Duck

@Pl45m4 said in QDataStream >> QList causes the program to crash when I try to open an invalid file:

How does your QList look like (What kind of type is Something) ? What data is expected to be in that list?

Something is a struct containing a QString and two ints, with an overloaded operator that looks like this:

QDataStream &operator<<(QDataStream &dataStream, Something foo){
    dataStream << foo.string << foo.int1 << foo.int2;
}

and the same for >>.

Does QTextStream work for you (assuming you want to read-in string / text)?
Like used here:

https://doc.qt.io/qt-5/qfile.html#using-streams-to-read-files

I don't want it to be a text file, so no.

@Christian-Ehrlicher said in QDataStream >> QList causes the program to crash when I try to open an invalid file:

Use a debugger and see where exactly it crashes. I don't think it crashes inside Qt but inside your Something QDataStream operator.

I did that and it crashes inside Qt, on line 833 of qlist.h (the line that says QListData::Data *x = p.detach(alloc);), I'm using Qt 5.15.2. The part of my code that calls that is dataStream >> myList.

Pl45m4

@Donald-Duck said in QDataStream >> QList causes the program to crash when I try to open an invalid file:

Something is a struct containing a QString and two ints, with an overloaded operator that looks like this:
QDataStream &operator<<(QDataStream &dataStream, Something foo){
dataStream << foo.string << foo.int1 << foo.int2;
}

What do you think will happen, when an invalid file doesn't contain a string and two ints? Of course it cant put that data in your Something struct.

One way could be, that you check the result of what you've read, before pushing it to your list.

I don't want it to be a text file, so no.

Just because you write text (strings, ints, etc) it doesn't mean, that your file has to be a *.txt file

Christian Ehrlicher

@Donald-Duck said in QDataStream >> QList causes the program to crash when I try to open an invalid file:

QDataStream &operator<<(QDataStream &dataStream, Something foo){

This is the serialization operator, not the one which is used for deserialization.

dataStream >> myList

What is myList? Since QDataStream stores the size of the QList first it may read high number and tries to allocate the memory then. So you will most likely get an out of memory exception which can be catched with a try/catch.

To make sure you read only your files I suggest adding a magic pattern in the first bytes which you can check to be sure that it's really your file. And also add a checksum at the end so you can check this too later on.