Quake 3 demos: Handle corrupted files more gracefully

[TomArrow]

If the game crashes while writing a demo, it can end up cut off at the end. Added a small safeguard against it so that that final corrupted message will simply be ignored.

[paxcut]

I am not exactly sure, but it looks like it is possible that files that were corrupted may appear as if they were healthy. From what I can see, in a healthy demo file all messages except the final demo message have non empty message data.

With the proposed changes, when a corrupted file is encountered, the last message will still exist and have both messageNum and len, but nothing else after that making it l look as if it was the final demo message.

In that case here is a possible way to skip the entire last message, including the message number and its length so the last message will have valid data indicating the existence of a problem.

Instead of adding the code:

bool corrupted = false;
if(len+$ > std::mem::size()){
    corrupted = true;
}
if(!corrupted  && (len != FINAL_DEMO_MESSAGE_LENGTH || messageNum != FINAL_DEMO_MESSAGE_NUMBER)) {

You can just add the code:

if(len+$ > std::mem::size()){
    continue;
}

leaving the rest of the original code as it was.Continue will effectively delete all the data read so far for the current array element and attempt to process another element again but in this case eof would have been reached so the array will just stop.

It is very possible that I am not interpreting things correctly and the proposed changes make no sense in which case I apologize.

[TomArrow]

Well, naturally there is many different ways a file can get corrupted. The situation I am trying to handle is that the final message has data, but the data is incomplete, aka the message is partially written. My first attempt was to have this:

if(len+$ <= std::mem::size()  && (len != FINAL_DEMO_MESSAGE_LENGTH || messageNum != FINAL_DEMO_MESSAGE_NUMBER)) {

But that led to an exception too because it just kept reading the message contents as if they were new message headers until it eventually crashed and burned. That's why I added the corrupted flag and am also checking it at the bottom alongside the EOF condition.

Maybe I'm misunderstanding your suggestion, but once this kind of corruption is encountered, there isn't much that can be done I think except abort parsing the rest of the demo.

A way to improve might be to still read the data of the final message but have it marked as incomplete/corrupted in the overview so that it doesn't leave an unparsed piece at the end and the user can see that corruption exists there. The current way just prevents it from throwing an exception.

[paxcut]

It is true that continue won't work if there are bytes left after the offset of the len variable of the corrupted message. As I explained above, continue will keep trying to add more elements to the array after discarding what was already read so it will try to use the truncated message data as if it was the beginning of a new message.

Under normal circumstances after the message number and the value of len there should be len bytes of data. If data corruption is detected then there will be only u8 bytesLeft [std::mem::size() - $]; bytes of data left instead which will always be smaller than len because corruption is detected if (len > std::mem::size() - $). That means that after those three items (the message number, the value of len and the bytesLeft bytes ) there will be nothing left because reading std::mem::size()-$ bytes starting at $ will put the cursor at std::mem::size()which is eof().

So in view of the fact that when message corruption occurs the data may be truncated and not always empty, the following amended code should work instead of the code proposed before.

if(len+$ > std::mem::size()){
    u8 bytesLeft[std::mem::size()-$];
    continue;
}

This will skip all the data left and terminate the array leaving only patterns that contain whole messages and the corrupted message will be completely skipped. The important thing to remember is that continue will move the cursor to the last position where data was successfully read thus skipping all of the bytes read for the current array element so it is important to read as much data as there is available so that it can be safely skipped in the patterns.

Of course this only addresses the case of the game crashing while writing the demo. Other forms of corruption would have to be addressed independently.