Issue 5728 - Replace Services.vc.compare

lib/coreUtils.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 16 matching lines @@
 
   return descriptor;
 }
 exports.desc = desc;
 
 function extend(cls, properties)
 {
   return Object.create(cls.prototype, desc(properties));
 }
 exports.extend = extend;
+
+function compareVersion(v1, v2)
+{
+  let components1 = v1.split(".");
+  let components2 = v2.split(".");
+  let result = 0;
+
+  for (let i = 0; result == 0 && (i < components1.length ||
+                                  i < components2.length); i++)
+  {
+    let comp1 = components1[i];
+    let comp2 = components2[i];
+
+    result = (comp1 == "*" ? Number.MAX_VALUE : parseInt(comp1, 10) || 0) -

[Manish Jethani, 2017/09/19 07:53:14]

I'd just like to point out that parseInt doesn't just parse strings as NaN, it
can also yield Infinity. In the unlikely case where one of the values is a
number too large, this will give the wrong result. Number.MAX_VALUE - Infinity
is -Infinity, for example. If both values are too large, you'll have Infinity -
Infinity, which is NaN and not 0.

Of course this is unlikely but I just thought I'd point it out.

For what it's worth, the previous version of this function does not handle this
correctly either, nor does the native version in Gecko.

A simple way around this:

  if (comp1 == "*")
    result = comp2 == "*" ? 0 : Infinity;
  else if (comp2 == "*")
    result = -Infinity;
  else
    result = (parseInt(comp1, 10) || 0) - (parseInt(comp2, 10) || 0) || 0;

The last "|| 0" is needed because Infinity is true and Infinity - Infinity is
NaN. Alternatively that expression could be "parseInt(comp1, 10) | 0 -
parseInt(comp2, 10) | 0".

[Manish Jethani, 2017/09/19 08:21:40]

Bitwise OR-ing with 0 will mess up any values larger than 2 ^ 31 - 1, by the
way, so it's not a good idea either.

[Sebastian Noack, 2017/09/19 22:53:43]

How exactly might parseInt() return Infinity? It seems that too large numbers
are just rounded to a finite number, but never to Infinity, which makes sense
mathematically, since every possible finite number is closer to Number.MAX_VALUE
than to infinity. Neither does parseInt("Infinity") return Infinity, but NaN.

[Manish Jethani, 2017/09/20 01:15:38]

Number.MAX_VALUE is approximately 1.79E+308, so for example 1.79E+309 (179
followed by 307 zeroes) yields Infinity.

  parseInt("179" + Array(307).fill(0).join(""), 10)

[Sebastian Noack, 2017/09/20 01:31:25]

Interesting. It's probably not a too relevant scenario, but I think the way I
account for it now, seems simple enough to just do it.

+             (comp2 == "*" ? Number.MAX_VALUE : parseInt(comp2, 10) || 0);

[Manish Jethani, 2017/09/19 07:53:14]

Also of course this version differs from the previous one in that it doesn't
handle version numbers like "54.0b1", but I take it that is intentional.

Here are some actual Firefox version numbers for reference:

https://ftp.mozilla.org/pub/firefox/releases/

+  }
+
+  return result;
+}
+exports.compareVersion = compareVersion;