CodeSOD: Plugin Acrobatics

Once upon a time, web browsers weren’t the one-stop-shop for all kinds of possible content that they are today. Aside from the most basic media types, your browser depended on content plugins to display different media types. Yes, there was an era where, if you wanted to watch a video in a web browser, you may need to have QuickTime or… (shudder) Real Player installed.

As a web developer, you’d need to write code to check which plugins were installed. If they don’t have Adobe Acrobat Reader installed, there’s no point in serving them up a PDF file- you’ll need instead to give them an install link.

Which brings us to Ido‘s submission. This code is intended to find the Acrobat Reader plugin version.

acrobatVersion: function GetAcrobatVersion() {
	// Check acrobat is Enabled or not and its version
	acrobatVersion = 0;
	if (navigator.plugins && navigator.plugins.length) {
		for (intLoop = 0; intLoop <= 15; intLoop++) {
			if (navigator.plugins[intLoop] != -1) {
				acrobatVersion = parseFloat(navigator.plugins[intLoop].version);
				isAcrobatInstalled = true;
				break;
			}
		}
	}
	else {...}
}

So, we start by checking for the navigator.plugins array. This is a wildly outdated thing to do, as the MDN is quite emphatic about, but I’m not going to to get hung up on that- this code is likely old.

But what I do want to pay attention to is that they check navigator.plugins.length. Then they loop across the set of plugins using a for loop. And don’t use the length! They bound the loop at 15, arbitrarily. Why? No idea- I suspect it’s for the same reason they named the variable intLoop and not i like a normal human.

Then they check to ensure that the entry at plugins[intLoop] is not equal to -1. I’m not sure what the expected behavior was here- if you’re accessing an array out of bounds in JavaScript, I’d expect it to return undefined. Perhaps some antique version of Internet Explorer did something differently? Sadly plausible.

Okay, we’ve found something we believe to be a plugin, because it’s not -1, we’ll grab the version property off of it and… parseFloat. On a version number. Which ignores the fact that 1.1 and 1.10 are different versions. Version numbers, like phone numbers, are not actually numbers. We don’t do arithmetic on them, treat them like text.

That done, we can say isAcrobatInstalled is true- despite the fact that we didn’t check to see if this plugin was actually an Acrobat plugin. It could have been Flash. Or QuickTime.

Then we break out of the loop. A loop that, I strongly suspect, would only ever have one iteration, because undefined != -1.

So there we have it: code that doesn’t do what it intends to, and even if it did, is doing it the absolute wrong way, and is also epically deprecated.

Source: Read More