Problem with CRCs and the updater

[Guest takinator]

Serpentine roped me into converting the tdm_updater from Perl to Python in an attempt to sort out some of the issues that people are having with the updater when packaged as a standalone executable.

 

I've basically been converting line for line and adjusting where the Python libraries differ. However, I've encountered a problem with the way the CRCs are calculated for files, particularly those in zip files like tdm_update_win.zip.

 

When I converted the 'get_crc_for_file' function, I ended up with this:

 

overallcrc = 0
data = fh2.read(32768)
while data:
      overallcrc ^= crc32(data)
      data = fh2.read(32768)

 

However, this would produce incorrect results for a number of files (negative values, which is clearly incorrect). The docs pointed at a better way of doing this, which fixed the problem, as shown below:

overallcrc = 0
data = fh2.read(32768)
while data:
      overallcrc = crc32(data, overallcrc) & 0xffffffff
      data = fh2.read(32768)

 

But I was still having issues where my Python version was throwing up the updater executables as checksum mismatches. I installed HashTab (http://beeblebrox.org/) to compare my generated CRC32s to what HashTab says and what crc_info.txt says as well as the CRC32 shown by WinRAR for the archive. According to crc_info.txt:

 

[File tdm_update_win.zip]

crc = 7ca4ce95

size = 2763886

 

crc = 9850fccf

size = 4995906

 

Running my Python version:

python tdm_update.py --dry-run --verbose

[snip]

tdm_update.exe Info: tdm_update.exe is inside tdm_update_win.zip

CRC for local file: 7ca4ce95, CRC on server is 9850fccf

checksum mismatch, getting tdm_update_win.zip.

 

HashTab and WinRAR both report the CRC32 for tdm_update.exe as: 7CA4CE95

 

So I looked at the Perl version of 'get_crc_for_file' and converted it from:

my $overallcrc = 0;
while (($n = read $fh2, $data, 32768) != 0)
{
      $overallcrc ^= Archive::Zip::computeCRC32($data);
}

 

To:

my $overallcrc = 0;
while (($n = read $fh2, $data, 32768) != 0)
{
      $overallcrc = Archive::Zip::computeCRC32($data, $overallcrc);
}

 

And that code now produces 7ca4ce95 as the CRC32 for tdm_update.exe instead of 9850fccf. You can actually see that there is a problem with the existing code by looking at the crc_info.txt file. The CRC for a zip file is calculated by XORing the CRCs for each file in the archive. And since the the updater zip only has 1 file and the CRC used in this case is taken from the zip archive and not calculated, the File entry should have the same CRC as the Member entry. This needs to be fixed for the current tdm_update.pl as well as the deployment scripts such as calculate_crcs.pl.

 

 

[greebo]

The wrong CRC calculation is probably my fault - I had to calculate the cumulative CRC myself and got it to work in a hurry, which explains the mess.

 

It'd be a delicate step to ship an updated CRC calculation algorithm, as the updater binary is already rolled out to all client machines. Updating the CRC on the server would make the existing tdm_update.exe believe all files got changed and need a redownload. However, as the last update 1.02 also included a change which allows to download a new updater first before downloading anything else, the update process would look like this:

 

- Run tdm_update.exe (1.02 version)

- Updater gets new CRCs from server, presumably all are mismatching (including the updater itself)

- Updater notices that a new updater is available and initiates a self-update.

- New updater is downloaded and started - the fixed CRC algorithm is now active already.

- Proceed as usual.

 

It has to be tested whether this causes any problems, maybe I missed a thing in my mind. The only important thing when considering such a change is that clients do not want to download 1.6 GB all over again.

 

(Welcome to the forums, btw.)

[Guest takinator]

I don't think we have to worry about it downloading all the .pk4s again. From what I could tell, only binaries such as the launcher and updater were being affected by the old CRC algorithm. So when get_crc_for_file in tdm_update.pl is modified and the new updater binaries are packed and zipped, the CRC for those binaries should change. So regardless of the updater version used by the client, the new updater will be downloaded. Then the updated calculate_crcs.pl is run to generate a new crc_info.txt. Only the updater and launcher entries in crc_info.txt should have changed, which means that only those zip files will be downloaded, making everything happy.

[greebo]

I agree, it's definitely possible to pull a seamless transition, if we decide to use the ported updater code.

 

Serpentine mentioned you had some sort of GUI planned for the updater? Any news on that?

[New Horizon]

I can help test when the time comes. I have 1.02 installed here for testing.

[Serpentine]

I agree, it's definitely possible to pull a seamless transition, if we decide to use the ported updater code.

 

Serpentine mentioned you had some sort of GUI planned for the updater? Any news on that?

 

I spoke a bit too soon! but it'll be looked sometime soon, think I've found the most reasonably cross platform method. For now the console updater seems to be working and correct (pretty much a straight port of the perl by taki here).

 

I also greebo to establish what his "undisclosed feature" will be doing so that we can piggyback on the ideas and hopefully have a nice maintainable system... but more of that another time

[greebo]

Can I see or have the sources of the Python updater, perhaps?

[taaaki]

Yeah, sorry about the lack of progress on this. I've been swamped with uni work and will probably only get time to implement all my extensions such as GUI and multiple thread downloading in December. I just want to integrate my improved zipfile handling (the Python Zipfile module is prone to directory traversal) and clean up some of my experiments.  I'll finish that off this weekend and upload it somewhere. I can't guarantee that I've accurately translated all of the Perl die statements (ie. the updater may exit when it shouldn't etc.).

[Bikerdude]

its good we have another coder of sorts helping out, as poor old Greebs is under the cosh atm..