Fix version format regexp (#1507)

* Fix version format regexp

* Add tests for isToolkitVersionString()

* Fix missing comma

* Add more test patterns

Closes #1439 
Closes #1172

src/schema/formats.js

@@ -1,48 +1,32 @@
 import { URL } from 'whatwg-url';
 
-const VALIDNUMRX = /^[0-9]{1,5}$/;
-
 // Firefox's version format is laxer than Chrome's, it accepts:
 // https://developer.mozilla.org/en-US/docs/Toolkit_version_format
 // We choose a slightly restricted version of that format (but still more
 // permissive than Chrome) to allow Beta addons, per:
 // https://developer.mozilla.org/en-US/Add-ons/AMO/Policy/Maintenance
-const TOOLKIT_VERSION_REGEX = /^(\d+\.?){1,3}\.(\d+([A-z]+(-?[\dA-z]+)?))$/;
+const VERSION_PART =
+  '(?:0|[1-9]\\d{0,3}|[1-5]\\d{4}|6(?:[0-4]\\d{3}|5(?:[0-4]\\d{2}|5(?:[0-2]\\d|3[0-5]))))';
+const BETA_PART = '(?:a(?:lpha)?|b(?:eta)?|pre|rc)\\d*';
+const VERSION_REGEXP =
+  new RegExp(`^${VERSION_PART}(?:\\.${VERSION_PART}){0,3}(?:${BETA_PART})?$`);
+const TOOLKIT_REGEXP =
+  new RegExp(`^${VERSION_PART}(?:\\.${VERSION_PART}){0,3}${BETA_PART}$`);
 
 export function isValidVersionString(version) {
   // We should be starting with a string.
   if (typeof version !== 'string') {
     return false;
   }
-  // If valid toolkit version string, return true early
-  if (TOOLKIT_VERSION_REGEX.test(version)) {
-    return true;
-  }
-  const parts = version.split('.');
-  if (parts.length > 4) {
-    return false;
-  }
-
-  for (let i = 0; i < parts.length; i++) {
-    let part = parts[i];
-    // Leading or multiple zeros not allowed.
-    if (part.startsWith('0') && part.length > 1) {
-      return false;
-    }
-    // Disallow things like 123e5 which parseInt will convert.
-    if (!VALIDNUMRX.test(part)) {
-      return false;
-    }
-    part = parseInt(part, 10);
-    if (Number.isNaN(part) || part < 0 || part > 65535) {
-      return false;
-    }
-  }
-  return true;
+  return VERSION_REGEXP.test(version);
 }
 
 export function isToolkitVersionString(version) {
-  return TOOLKIT_VERSION_REGEX.test(version) && isValidVersionString(version);
+  // We should be starting with a string.
+  if (typeof version !== 'string') {
+    return false;
+  }
+  return TOOLKIT_REGEXP.test(version);
 }
 
 export function isAbsoluteUrl(value) {

tests/schema/test.formats.js

@@ -3,13 +3,13 @@ import {
   isAnyUrl,
   isSecureUrl,
   isStrictRelativeUrl,
+  isToolkitVersionString,
   isValidVersionString,
 } from 'schema/formats';
 
 describe('formats', () => {
   describe('isValidVersionString', () => {
     const validVersionStrings = [
-      '0.1.12dev-cb31c51',
       '1.0',
       '1.0.0beta2',
       '2.10.2',
@@ -17,7 +17,6 @@ describe('formats', () => {
       '3.1.2.65535',
       '4.1pre1',
       '4.1.1pre2',
-      '4.1.1dev-abcdef1',
       '4.1.1.2pre3',
     ];
 
@@ -28,11 +27,20 @@ describe('formats', () => {
       '.',
       'a.b.c.d',
       '1.2.2.2.4',
+      '1.2.2.2.4a',
       '01',
+      '1.01',
+      '1.01a',
       '1.000000',
+      '1.000000a1',
       '2.99999',
       '3.65536',
+      '3.65536a1',
       '1.0.0-beta2',
+      '1.0.0+1',
+      '1.0.0-rc1.0+001',
+      '0.1.12dev-cb31c51',
+      '4.1.1dev-abcdef1',
     ];
 
     validVersionStrings.forEach((validVersionString) => {
@@ -48,6 +56,68 @@ describe('formats', () => {
     });
   });
 
+  describe('isToolkitVersionString', () => {
+    const validToolkitVersionStrings = [
+      '1a',
+      '1alpha',
+      '1b',
+      '1beta',
+      '1pre',
+      '1rc',
+      '1.0a1',
+      '1.0.0alpha01',
+      '1.0.0.0b10',
+      '1.0.0beta2',
+      '4.1pre1',
+      '4.1.1pre2',
+      '4.1.1.2pre3',
+    ];
+
+    const invalidToolkitVersionStrings = [
+      2,
+      '123e5',
+      '1.',
+      '.',
+      'a.b.c.d',
+      '1.2.2.2.4',
+      '1.2.2.2.4a',
+      '01',
+      '1.01',
+      '1.01a',
+      '1.000000',
+      '1.000000a1',
+      '2.99999',
+      '3.65536',
+      '3.65536a1',
+      '1.0.0-beta2',
+      '1.0.0+1',
+      '1.0.0-rc1.0+001',
+      '0.1.12dev-cb31c51',
+      '4.1.1dev-abcdef1',
+      '1.0',
+      '2.10.2',
+      '3.1.2.4567',
+      '3.1.2.65535',
+      '1abc',
+      '1.0.0.0.0a',
+      '1.0.0a-1',
+      '1.0.0a1.1',
+    ];
+
+    validToolkitVersionStrings.forEach((validToolkitVersionString) => {
+      it(`should find ${validToolkitVersionString} to be valid`, () => {
+        expect(isToolkitVersionString(validToolkitVersionString)).toEqual(true);
+      });
+    });
+
+    invalidToolkitVersionStrings.forEach((invalidToolkitVersionString) => {
+      it(`should find ${invalidToolkitVersionString} to be invalid`, () => {
+        expect(isToolkitVersionString(invalidToolkitVersionString))
+          .toEqual(false);
+      });
+    });
+  });
+
   describe('URL formats', () => {
     it('domain', () => {
       const value = 'https://example.com';