fix: add identifier validation to inc()

classes/semver.js

@@ -10,7 +10,7 @@ class SemVer {
 
     if (version instanceof SemVer) {
       if (version.loose === !!options.loose &&
-          version.includePrerelease === !!options.includePrerelease) {
+        version.includePrerelease === !!options.includePrerelease) {
         return version
       } else {
         version = version.version
@@ -176,35 +176,37 @@ class SemVer {
   // preminor will bump the version up to the next minor release, and immediately
   // down to pre-release. premajor and prepatch work the same way.
   inc (release, identifier, identifierBase) {
+    this.#checkIdentifiers(release, identifier, identifierBase)
+
     switch (release) {
       case 'premajor':
         this.prerelease.length = 0
         this.patch = 0
         this.minor = 0
         this.major++
-        this.inc('pre', identifier, identifierBase)
+        this.#pre(identifier, identifierBase)
         break
       case 'preminor':
         this.prerelease.length = 0
         this.patch = 0
         this.minor++
-        this.inc('pre', identifier, identifierBase)
+        this.#pre(identifier, identifierBase)
         break
       case 'prepatch':
         // If this is already a prerelease, it will bump to the next version
         // drop any prereleases that might already exist, since they are not
         // relevant at this point.
         this.prerelease.length = 0
         this.inc('patch', identifier, identifierBase)
-        this.inc('pre', identifier, identifierBase)
+        this.#pre(identifier, identifierBase)
         break
       // If the input is a non-prerelease version, this acts the same as
       // prepatch.
       case 'prerelease':
         if (this.prerelease.length === 0) {
           this.inc('patch', identifier, identifierBase)
         }
-        this.inc('pre', identifier, identifierBase)
+        this.#pre(identifier, identifierBase)
         break
 
       case 'major':
@@ -244,50 +246,7 @@ class SemVer {
         }
         this.prerelease = []
         break
-      // This probably shouldn't be used publicly.
-      // 1.0.0 'pre' would become 1.0.0-0 which is the wrong direction.
-      case 'pre': {
-        const base = Number(identifierBase) ? 1 : 0
-
-        if (!identifier && identifierBase === false) {
-          throw new Error('invalid increment argument: identifier is empty')
-        }
 
-        if (this.prerelease.length === 0) {
-          this.prerelease = [base]
-        } else {
-          let i = this.prerelease.length
-          while (--i >= 0) {
-            if (typeof this.prerelease[i] === 'number') {
-              this.prerelease[i]++
-              i = -2
-            }
-          }
-          if (i === -1) {
-            // didn't increment anything
-            if (identifier === this.prerelease.join('.') && identifierBase === false) {
-              throw new Error('invalid increment argument: identifier already exists')
-            }
-            this.prerelease.push(base)
-          }
-        }
-        if (identifier) {
-          // 1.2.0-beta.1 bumps to 1.2.0-beta.2,
-          // 1.2.0-beta.fooblz or 1.2.0-beta bumps to 1.2.0-beta.0
-          let prerelease = [identifier, base]
-          if (identifierBase === false) {
-            prerelease = [identifier]
-          }
-          if (compareIdentifiers(this.prerelease[0], identifier) === 0) {
-            if (isNaN(this.prerelease[1])) {
-              this.prerelease = prerelease
-            }
-          } else {
-            this.prerelease = prerelease
-          }
-        }
-        break
-      }
       default:
         throw new Error(`invalid increment argument: ${release}`)
     }
@@ -297,6 +256,62 @@ class SemVer {
     }
     return this
   }
+
+  #checkIdentifiers (release, identifier, identifierBase) {
+    if (release.startsWith('pre')) {
+      if (!identifier && identifierBase === false) {
+        throw new Error('invalid increment argument: identifier is empty')
+      }
+
+      // Avoid an invalid semver results
+      if (identifier) {
+        const match = `-${identifier}`.match(this.options.loose ? re[t.PRERELEASELOOSE] : re[t.PRERELEASE])
+        if (!match || match[1] !== identifier) {
+          throw new Error(`invalid identifier: ${identifier}`)
+        }
+      }
+    }
+  }
+
+  // Keep 'pre' internal to avoid confusing results
+  // inc('1.0.0', 'pre') would become 1.0.0-0 which is the wrong direction.
+  #pre (identifier, identifierBase) {
+    const base = Number(identifierBase) ? 1 : 0
+
+    if (this.prerelease.length === 0) {
+      this.prerelease = [base]
+    } else {
+      let i = this.prerelease.length
+      while (--i >= 0) {
+        if (typeof this.prerelease[i] === 'number') {
+          this.prerelease[i]++
+          i = -2
+        }
+      }
+      if (i === -1) {
+        // didn't increment anything
+        if (identifier === this.prerelease.join('.') && identifierBase === false) {
+          throw new Error('invalid increment argument: identifier already exists')
+        }
+        this.prerelease.push(base)
+      }
+    }
+    if (identifier) {
+      // 1.2.0-beta.1 bumps to 1.2.0-beta.2,
+      // 1.2.0-beta.fooblz or 1.2.0-beta bumps to 1.2.0-beta.0
+      let prerelease = [identifier, base]
+      if (identifierBase === false) {
+        prerelease = [identifier]
+      }
+      if (compareIdentifiers(this.prerelease[0], identifier) === 0) {
+        if (isNaN(this.prerelease[1])) {
+          this.prerelease = prerelease
+        }
+      } else {
+        this.prerelease = prerelease
+      }
+    }
+  }
 }
 
 module.exports = SemVer

test/classes/semver.js

@@ -106,6 +106,39 @@ test('incrementing', t => {
   }))
 })
 
+test('invalid increments', (t) => {
+  // pre was used on internally and is not valid anymore
+  t.throws(
+    () => new SemVer('1.2.3').inc('pre'),
+    Error('invalid increment argument: pre')
+  )
+  t.throws(
+    () => new SemVer('1.2.3').inc('prerelease', '', false),
+    Error('invalid increment argument: identifier is empty')
+  )
+  t.throws(
+    () => new SemVer('1.2.3-dev').inc('prerelease', 'dev', false),
+    Error('invalid increment argument: identifier already exists')
+  )
+  t.throws(
+    () => new SemVer('1.2.3').inc('prerelease', 'invalid/preid'),
+    Error('invalid identifier: invalid/preid')
+  )
+
+  t.end()
+})
+
+test('increment side-effects', (t) => {
+  const v = new SemVer('1.0.0')
+  try {
+    v.inc('prerelease', 'hot/mess')
+  } catch (er) {
+    // ignore but check that the version has not changed
+  }
+  t.equal(v.toString(), '1.0.0')
+  t.end()
+})
+
 test('compare main vs pre', (t) => {
   const s = new SemVer('1.2.3')
   t.equal(s.compareMain('2.3.4'), -1)

test/fixtures/increments.js

@@ -124,4 +124,7 @@ module.exports = [
   ['1.2.0-dev', 'prepatch', '1.2.1-dev', false, 'dev', false],
   ['1.2.0', 'prerelease', null, false, '', false],
   ['1.0.0-rc.1+build.4', 'prerelease', '1.0.0-rc.2', 'rc', false],
+  ['1.2.0', 'prerelease', null, false, 'invalid/preid'],
+  ['1.2.0', 'prerelease', null, false, 'invalid+build'],
+  ['1.2.0beta', 'prerelease', null, { loose: true }, 'invalid/preid'],
 ]