Tag: hacking

Unbreak My Hea.. Firefox Ctrl Click Please!

When we launched Gamer.no over a year ago, we had to come up with a wallpaper advertising solution in a rush (everything were a rush back then as we built and launched a site from scratch (after disagreements between the previous owner and Gamer) in just under four days (or 96 hours)). While this solution has worked .. good enough .. it has always had a few irky bugs that I’ve never really had the right inspiration to uncover the cause of. Usually I’ve spent an hour and decided that the time wasn’t worth it at the moment and then moved onto something else, but today! Today is a glorious day!

The bug has been fixed!

The wallpaper element is placed around the main content div, which sadly also makes the wallpaper element receive any click elements that the main content div receives. This leads to the wallpaper getting clicked and the wallpaper ad window opening regardless of where people click – which will get very, very annoying very quick. So to battle this issue the original solution was to call .stopPropagation() on the evt object in a click handler for the main content div. This solved the issue and everyone rejoiced! However, all was not perfect in paradise.

Some time later we discovered that the .stopPropagation() fix borked ctrl-click a link in Firefox. Other browsers handled it just fine, but Firefox were obviously not happy. Not happy at all. Mad and going on a killing spree it shot down the proposed fixes from both myself and other people who had a brief look at the code. It wasn’t a big issue as we only run the wallpaper code for small intervals of time and people didn’t complain (maybe we were some of the few who had the issue).

Today I decided to have a look at the issue again, and finally I realized that we had been way to focused on our call to .stopPropagation(). Everyone had been planning how we could get .stopPropagation to do what we wanted it to do – after all – the issue was that stopPropagation didn’t behave when we ctrl-clicked in Firefox. But wait.

If you instead think of the original problem; the window.open gets triggered when people click the inner element instead of the outer, there may be alternative solutions to using stopPropagation. And yes, THAT was quite a simple fix. Instead of trying to stop the event from bubling up through the cloud.. let’s just set a status variable that tells the code handling the wallpaper click that THIS CLICK IS NOT FOR YOU BAD HANDLER GO AWAY LET OTHER GROWNUPS HANDLE THIS. So that I did.

$(document).ready(function () {
    innerClick = false;
    $('#wallpaper').click(function() {
        if (innerClick)
        {
            innerClick = false;
            return true;
        }
        
        window.open("..");
    });
    $('#content').click(function(evt) {
        innerClick = true;
    });
});

As soon as I actually spent some time on what we were trying to solve instead of what seemed like the cause of the issue .. everything went better than expected.

Going Hex Hunting in ZIP Files

It’s been a couple of weeks since I last had to go hex hunting, but fear not – we’re back in action! We discovered this issue after adding support for uploading ZIP files in the platform powering Gamer.no. All our test cases worked perfectly, but as soon as we released the feature our users discovered zip files from one particular site never worked. WinRAR and 7zip were both able to extract the files, so we just left the issue at that for the time being.

This evening I finally got around to trying to find out exactly what was going on. I started by updating my SVN trunk checkout of PHP6, adding the zip module to my configure and running make clean and make. The issue was still present in HEAD, so that meant getting my hands dirty and trying to find out exactly what we were up against.

The first thing on the table was to find out what the zip module was getting all antsy about when trying to open the file. A few well placed printfs (all hail the magic of printf debugging) told me that the line that this particular ZIP file did not pass was:

if ((comlen < cd->comment_len) || (cd->nentry != i)) {

The cd struct is the “end of central directory” struct, containing meta data about the archive and an optional comment about the ZIP file. The comlen and the cd->comment_len were both zero (as the file didn’t have a comment), so the trouble maker was the cd->nentry != i statement. The library that the PHP module uses reads i by itself and then reads the struct. nentry is the number of files in “this disk” of the archive (a ZIP archive can be divided across several physical disks (think back to 8″, 5.25″ and 3.5″ disks)), while i is the “total number of files in the archive”. The PHP library only supports single archives (not archives spanning several disks), so these two values should be identical. For some reason, they weren’t – meaning that the zip files generate by this particular site actually are invalid ZIP-files. WinRAR and 7zip just makes the best of the situation and does it very nicely.

Here’s the hex dump of the end of central directory section from one of the non-working files:

Hex dump of the end of central directory section from a ZIP file

The first four bytes are the signature of the section (50 4B 05 06, or PK\5\6), then we have 2 bytes which is the “number of this disk” (00 00 here) and 2 bytes with “number of the disk with the start of the central directory” (00 00 here again) (the library in PHP doesn’t support archives spanning multiple disks, so it just compares this section to 00 00 00 00). Then we have our magic numbers, both two bytes: “total number of entries in the central directory on this disk” and “total number of entries in the central directory”. These should logically be the same for single file archives, but they’re 6 and 3 (06 00 and 03 00) here (the correct number of files in the archive is 3).

The solution for us is to use the correct number (“total number of entries in the central directory”) for both the values. To do this we simply patch the two bytes of the binary zip file (.. we could do this with fopen, fseek, fwrite in place, but we’re lazy. This is not time sensitive code.) and rewrite it in place:

    /**
     * Attempt to repair the zip file by correcting wrong total number of files in archive
     */
    static public function repairZIPFile($file)
    {
        // lets read in the file
        $data = file_get_contents($file);

        // lets try to find the end of the central directory record (should be
        // the last thing in the file, so we search from the end)
        $startpos = strrpos($data, "PK\5\6");

        // if we found a header..
        if ($startpos)
        {
            // attempt to repair the file by copying the "total files" to the "files on this disk" field.
            // PHP's ZIP module doesn't handle multidisks anyway..
            $data[$startpos+8] = $data[$startpos+10];
            $data[$startpos+9] = $data[$startpos+11];

            file_put_contents($file, $data);
        }
    }

And Voilá – we’re a happy bunch again!