guard against locale-specific sorting

Node will use the current environment's locale sorting for
String.localeCompare.  Thankfully, String.localeCompare can take a
second argument to specify the locale.  Up until Node 13, any valid
argument provided to the function _may_ be respected if Node was
compiled with Intl support, or would always be treated as `'en'`
otherwise.

The solution is to always set the locale to `'en'`.  An alternative
solution would be to compare strings based on byte order, but this does
not provide ideal ergonomis.

To verify this, the locale is set in the test environment to `'sk'`,
which has a significantly different alphabet, including sorting words
starting with `'ch'` _after_ words starting with `'d'`.

Re: npm/cli#2829
Fix: #276

bin/license.js

@@ -22,7 +22,7 @@ a.loadVirtual().then(tree => {
       set.push([tree.inventory.query('license', license).size, license])
 
     for (const [count, license] of set.sort((a, b) =>
-      a[1] && b[1] ? b[0] - a[0] || a[1].localeCompare(b[1])
+      a[1] && b[1] ? b[0] - a[0] || a[1].localeCompare(b[1], 'en')
       : a[1] ? -1
       : b[1] ? 1
       : 0))

lib/add-rm-pkg-deps.js

@@ -75,7 +75,7 @@ const addSingle = ({pkg, spec, saveBundle, saveType, log}) => {
     // keep it sorted, keep it unique
     const bd = new Set(pkg.bundleDependencies || [])
     bd.add(spec.name)
-    pkg.bundleDependencies = [...bd].sort((a, b) => a.localeCompare(b))
+    pkg.bundleDependencies = [...bd].sort((a, b) => a.localeCompare(b, 'en'))
   }
 }
 

lib/arborist/build-ideal-tree.js

@@ -764,7 +764,7 @@ This is a one-time fix-up, please be patient...
     // sort physically shallower deps up to the front of the queue,
     // because they'll affect things deeper in, then alphabetical
     this[_depsQueue].sort((a, b) =>
-      (a.depth - b.depth) || a.path.localeCompare(b.path))
+      (a.depth - b.depth) || a.path.localeCompare(b.path, 'en'))
 
     const node = this[_depsQueue].shift()
     const bd = node.package.bundleDependencies
@@ -902,7 +902,7 @@ This is a one-time fix-up, please be patient...
     }
 
     const placed = tasks
-      .sort((a, b) => a.edge.name.localeCompare(b.edge.name))
+      .sort((a, b) => a.edge.name.localeCompare(b.edge.name, 'en'))
       .map(({ edge, dep }) => this[_placeDep](dep, node, edge))
 
     const promises = []
@@ -1147,7 +1147,7 @@ This is a one-time fix-up, please be patient...
       // we typically only install non-optional peers, but we have to
       // factor them into the peerSet so that we can avoid conflicts
       .filter(e => e.peer && !(e.valid && e.to))
-      .sort(({name: a}, {name: b}) => a.localeCompare(b))
+      .sort(({name: a}, {name: b}) => a.localeCompare(b, 'en'))
 
     for (const edge of peerEdges) {
       // already placed this one, and we're happy with it.

lib/arborist/load-virtual.js

@@ -159,12 +159,12 @@ module.exports = cls => class VirtualLoader extends cls {
       ...depsToEdges('peerOptional', peerOptional),
       ...lockWS,
     ].sort(([atype, aname], [btype, bname]) =>
-      atype.localeCompare(btype) || aname.localeCompare(bname))
+      atype.localeCompare(btype, 'en') || aname.localeCompare(bname, 'en'))
 
     const rootEdges = [...root.edgesOut.values()]
       .map(e => [e.type, e.name, e.spec])
       .sort(([atype, aname], [btype, bname]) =>
-        atype.localeCompare(btype) || aname.localeCompare(bname))
+        atype.localeCompare(btype, 'en') || aname.localeCompare(bname, 'en'))
 
     if (rootEdges.length !== lockEdges.length) {
       // something added or removed

lib/arborist/rebuild.js

@@ -14,7 +14,7 @@ const {
 } = require('@npmcli/node-gyp')
 
 const boolEnv = b => b ? '1' : ''
-const sortNodes = (a, b) => (a.depth - b.depth) || a.path.localeCompare(b.path)
+const sortNodes = (a, b) => (a.depth - b.depth) || a.path.localeCompare(b.path, 'en')
 
 const _build = Symbol('build')
 const _resetQueues = Symbol('resetQueues')

lib/audit-report.js

@@ -78,7 +78,7 @@ class AuditReport extends Map {
     }
 
     obj.vulnerabilities = vulnerabilities
-      .sort(([a], [b]) => a.localeCompare(b))
+      .sort(([a], [b]) => a.localeCompare(b, 'en'))
       .reduce((set, [name, vuln]) => {
         set[name] = vuln
         return set

lib/printable.js

@@ -46,14 +46,14 @@ class ArboristNode {
     // edgesOut sorted by name
     if (tree.edgesOut.size) {
       this.edgesOut = new Map([...tree.edgesOut.entries()]
-        .sort(([a], [b]) => a.localeCompare(b))
+        .sort(([a], [b]) => a.localeCompare(b, 'en'))
         .map(([name, edge]) => [name, new EdgeOut(edge)]))
     }
 
     // edgesIn sorted by location
     if (tree.edgesIn.size) {
       this.edgesIn = new Set([...tree.edgesIn]
-        .sort((a, b) => a.from.location.localeCompare(b.from.location))
+        .sort((a, b) => a.from.location.localeCompare(b.from.location, 'en'))
         .map(edge => new EdgeIn(edge)))
     }
 
@@ -65,14 +65,14 @@ class ArboristNode {
     // fsChildren sorted by path
     if (tree.fsChildren.size) {
       this.fsChildren = new Set([...tree.fsChildren]
-        .sort(({path: a}, {path: b}) => a.localeCompare(b))
+        .sort(({path: a}, {path: b}) => a.localeCompare(b, 'en'))
         .map(tree => printableTree(tree, path)))
     }
 
     // children sorted by name
     if (tree.children.size) {
       this.children = new Map([...tree.children.entries()]
-        .sort(([a], [b]) => a.localeCompare(b))
+        .sort(([a], [b]) => a.localeCompare(b, 'en'))
         .map(([name, tree]) => [name, printableTree(tree, path)]))
     }
   }

lib/shrinkwrap.js

@@ -844,7 +844,7 @@ class Shrinkwrap {
       /* istanbul ignore next - sort calling order is indeterminate */
       return aloc.length > bloc.length ? 1
         : bloc.length > aloc.length ? -1
-        : aloc[aloc.length - 1].localeCompare(bloc[bloc.length - 1])
+        : aloc[aloc.length - 1].localeCompare(bloc[bloc.length - 1], 'en')
     })[0]
 
     const res = consistentResolve(node.resolved, this.path, this.path, true)

lib/update-root-package-json.js

@@ -18,7 +18,7 @@ const orderDeps = (pkg) => {
   for (const type of depTypes) {
     if (pkg && pkg[type]) {
       pkg[type] = Object.keys(pkg[type])
-        .sort((a, b) => a.localeCompare(b))
+        .sort((a, b) => a.localeCompare(b, 'en'))
         .reduce((res, key) => {
           res[key] = pkg[type][key]
           return res

lib/vuln.js

@@ -106,12 +106,12 @@ class Vuln {
         vulnerableVersions: undefined,
         id: undefined,
       }).sort((a, b) =>
-        String(a.source || a).localeCompare(String(b.source || b))),
+        String(a.source || a).localeCompare(String(b.source || b, 'en'))),
       effects: [...this.effects].map(v => v.name)
-        .sort(/* istanbul ignore next */(a, b) => a.localeCompare(b)),
+        .sort(/* istanbul ignore next */(a, b) => a.localeCompare(b, 'en')),
       range: this.simpleRange,
       nodes: [...this.nodes].map(n => n.location)
-        .sort(/* istanbul ignore next */(a, b) => a.localeCompare(b)),
+        .sort(/* istanbul ignore next */(a, b) => a.localeCompare(b, 'en')),
       fixAvailable: this[_fixAvailable],
     }
   }

lib/yarn-lock.js

@@ -34,7 +34,7 @@ const {breadth} = require('treeverse')
 
 // sort a key/value object into a string of JSON stringified keys and vals
 const sortKV = obj => Object.keys(obj)
-  .sort((a, b) => a.localeCompare(b))
+  .sort((a, b) => a.localeCompare(b, 'en'))
   .map(k => `    ${JSON.stringify(k)} ${JSON.stringify(obj[k])}`)
   .join('\n')
 
@@ -165,7 +165,7 @@ class YarnLock {
   toString () {
     return prefix + [...new Set([...this.entries.values()])]
       .map(e => e.toString())
-      .sort((a, b) => a.localeCompare(b)).join('\n\n') + '\n'
+      .sort((a, b) => a.localeCompare(b, 'en')).join('\n\n') + '\n'
   }
 
   fromTree (tree) {
@@ -175,15 +175,15 @@ class YarnLock {
       tree,
       visit: node => this.addEntryFromNode(node),
       getChildren: node => [...node.children.values(), ...node.fsChildren]
-        .sort((a, b) => a.depth - b.depth || a.name.localeCompare(b.name)),
+        .sort((a, b) => a.depth - b.depth || a.name.localeCompare(b.name, 'en')),
     })
     return this
   }
 
   addEntryFromNode (node) {
     const specs = [...node.edgesIn]
       .map(e => `${node.name}@${e.spec}`)
-      .sort((a, b) => a.localeCompare(b))
+      .sort((a, b) => a.localeCompare(b, 'en'))
 
     // Note:
     // yarn will do excessive duplication in a case like this:
@@ -309,7 +309,7 @@ class YarnLockEntry {
   toString () {
     // sort objects to the bottom, then alphabetical
     return ([...this[_specs]]
-      .sort((a, b) => a.localeCompare(b))
+      .sort((a, b) => a.localeCompare(b, 'en'))
       .map(JSON.stringify).join(', ') +
       ':\n' +
       Object.getOwnPropertyNames(this)
@@ -318,7 +318,7 @@ class YarnLockEntry {
           (a, b) =>
           /* istanbul ignore next - sort call order is unpredictable */
             (typeof this[a] === 'object') === (typeof this[b] === 'object')
-              ? a.localeCompare(b)
+              ? a.localeCompare(b, 'en')
               : typeof this[a] === 'object' ? 1 : -1)
         .map(prop =>
           typeof this[prop] !== 'object'

package.json

@@ -73,11 +73,13 @@
   "bin": {
     "arborist": "bin/index.js"
   },
+  "//": "sk test-env locale to catch locale-specific sorting",
   "tap": {
     "after": "test/fixtures/cleanup.js",
     "coverage-map": "map.js",
     "test-env": [
-      "NODE_OPTIONS=--no-warnings"
+      "NODE_OPTIONS=--no-warnings",
+      "LC_ALL=sk"
     ],
     "node-arg": [
       "--no-warnings",

test/arborist/rebuild.js

@@ -181,7 +181,7 @@ t.test('verify dep flags in script environments', async t => {
   // don't include path or env, because that's going to be platform-specific
   const saved = [...arb.scriptsRun]
     .sort(({path: patha, event: eventa}, {path: pathb, event: eventb}) =>
-      patha.localeCompare(pathb) || eventa.localeCompare(eventb))
+      patha.localeCompare(pathb, 'en') || eventa.localeCompare(eventb, 'en'))
     .map(({pkg, event, cmd, code, signal, stdout, stderr}) =>
       ({pkg, event, cmd, code, signal, stdout, stderr}))
   t.matchSnapshot(saved, 'saved script results')
@@ -191,7 +191,7 @@ t.test('verify dep flags in script environments', async t => {
     t.strictSame(flags, fs.readFileSync(file, 'utf8').split('\n'), pkg)
   }
   t.strictSame(checkLogs().sort((a, b) =>
-    a[2].localeCompare(b[2]) || (typeof a[4] === 'string' ? -1 : 1)), [
+    a[2].localeCompare(b[2], 'en') || (typeof a[4] === 'string' ? -1 : 1)), [
     ['info', 'run', 'devdep@1.0.0', 'postinstall', 'node_modules/devdep', 'node ../../env.js'],
     ['info', 'run', 'devdep@1.0.0', 'postinstall', {code: 0, signal: null}],
     ['info', 'run', 'devopt@1.0.0', 'postinstall', 'node_modules/devopt', 'node ../../env.js'],

test/arborist/reify.js

@@ -204,7 +204,7 @@ t.test('omit peer deps', t => {
     .then(() => {
       process.removeListener('time', onTime)
       process.removeListener('timeEnd', onTimeEnd)
-      finishedTimers.sort((a, b) => a.localeCompare(b))
+      finishedTimers.sort((a, b) => a.localeCompare(b, 'en'))
       t.matchSnapshot(finishedTimers, 'finished timers')
       t.strictSame(timers, {}, 'should have no timers in progress now')
     })

test/audit-report.js

@@ -23,14 +23,14 @@ const newArb = (path, opts = {}) => new Arborist({path, registry, cache, ...opts
 
 const sortReport = report => {
   const entries = Object.entries(report.vulnerabilities)
-  const vulns = entries.sort(([a], [b]) => a.localeCompare(b))
+  const vulns = entries.sort(([a], [b]) => a.localeCompare(b, 'en'))
     .map(([name, vuln]) => [
       name,
       {
         ...vuln,
         via: (vuln.via || []).sort((a, b) =>
-          String(a.source || a).localeCompare(String(b.source || b))),
-        effects: (vuln.effects || []).sort((a, b) => a.localeCompare(b)),
+          String(a.source || a).localeCompare(String(b.source || b, 'en'))),
+        effects: (vuln.effects || []).sort((a, b) => a.localeCompare(b, 'en')),
       },
     ])
   report.vulnerabilities = vulns.reduce((set, [k, v]) => {

test/diff.js

@@ -28,7 +28,7 @@ const formatDiff = obj =>
     removed: obj.removed.map(d => normalizePath(d.path)),
     children: [...obj.children]
       .map(formatDiff)
-      .sort((a, b) => path(a).localeCompare(path(b))),
+      .sort((a, b) => path(a).localeCompare(path(b, 'en'))),
   })
 
 t.formatSnapshot = obj => format(formatDiff(obj), { sort: false })

test/fixtures/index.js

@@ -167,7 +167,7 @@ const setup = () => {
       `### BEGIN IGNORED SYMLINKS ###
 ### this list is generated automatically, do not edit directly
 ### update it by running \`node test/fixtures/index.js\`
-${links.sort((a,b) => a.localeCompare(b)).join('\n')}
+${links.sort((a,b) => a.localeCompare(b, 'en')).join('\n')}
 ### END IGNORED SYMLINKS ###`)
     writeFileSync(gifile, gitignore)
   }

test/gather-dep-set.js

@@ -81,7 +81,7 @@ const tree = new Node({
 const normalizePath = path => path.replace(/[A-Z]:/, '').replace(/\\/g, '/')
 
 const printSet = set => [...set]
-  .sort((a, b) => a.name.localeCompare(b.name))
+  .sort((a, b) => a.name.localeCompare(b.name, 'en'))
   .map(n => n.location)
 
 const cwd = normalizePath(process.cwd())

test/inventory.js

@@ -16,7 +16,7 @@ t.test('basic operations', t => {
     i.get('y'),
   ], 'filter returns an iterable of all matching nodes')
 
-  t.same([...i.query('license')].sort((a, b) => String(a).localeCompare(String(b))),
+  t.same([...i.query('license')].sort((a, b) => String(a).localeCompare(String(b, 'en'))),
     ['ISC', 'MIT', undefined])
   t.same([...i.query('license', 'MIT')], [
     { location: 'x', name: 'x', package: { license: 'MIT', funding: 'foo' }},
@@ -28,7 +28,7 @@ t.test('basic operations', t => {
     { location: 'x', name: 'x', package: { license: 'MIT', funding: 'foo'}},
     { location: 'y', name: 'x', package: { license: 'ISC', funding: { url: 'foo' } }},
   ], 'can query by name')
-  t.same([...i.query('funding')].sort((a, b) => String(a).localeCompare(String(b))),
+  t.same([...i.query('funding')].sort((a, b) => String(a).localeCompare(String(b, 'en'))),
     ['bar', 'foo', undefined])
   t.same([...i.query('funding', 'foo')], [
     { location: 'x', name: 'x', package: { license: 'MIT', funding: 'foo' } },