Skip to content

Commit

Permalink
Validate separator in versions
Browse files Browse the repository at this point in the history
  • Loading branch information
ehmicky committed Jan 9, 2022
1 parent 4ad6030 commit 1f9173b
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 5 deletions.
29 changes: 25 additions & 4 deletions src/top/system/versions/compute.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import pProps from 'p-props'
import { PluginError } from '../../../error/main.js'
import { spawnProcess } from '../../../utils/spawn.js'

import { VERSIONS_VALUE_SEPARATOR } from './merge.js'

// Get runtime versions for this runner, returned as `versions` from
// `runner.launch()`. This is an object where:
// - the key is runtime name (e.g. 'Node')
Expand All @@ -22,8 +24,8 @@ export const computeRunnerVersions = async function ({
cwd,
}) {
const dedupedVersions = mapObj(versions, dedupeVersion)
const dedupedVersionsA = await pProps(dedupedVersions, (version) =>
computeRunnerVersion({ version, id, spawnOptions, cwd }),
const dedupedVersionsA = await pProps(dedupedVersions, ({ name, version }) =>
computeRunnerVersion({ name, version, id, spawnOptions, cwd }),
)
const versionsA = mapObj(versions, (name, version) => [
name,
Expand All @@ -41,19 +43,26 @@ export const computeRunnerVersions = async function ({
// parallelization of retrieving task paths, which is much slower
// - Runners have different `id` to report in the error message
const dedupeVersion = function (name, version) {
return [serializeVersion(version), version]
return [serializeVersion(version), { name, version }]
}

const serializeVersion = function (version) {
return Array.isArray(version) ? version.join(' ') : version
}

const computeRunnerVersion = async function ({
name,
version,
id,
spawnOptions,
cwd,
}) {
const versionA = await getRunnerVersion({ version, id, spawnOptions, cwd })
validateVersion(versionA, name, id)
return versionA
}

const getRunnerVersion = async function ({ version, id, spawnOptions, cwd }) {
if (typeof version === 'string') {
return version
}
Expand All @@ -67,9 +76,21 @@ const computeRunnerVersion = async function ({
return stdout
} catch (error) {
throw new PluginError(
`Could not start runner "${id}"
`Could not start runner "${id}".
Retrieving runner versions failed: ${version.join(' ')}
${error.message}`,
)
}
}

// When merging runners and results with different values of the same version
// property, we concatenate them. This becomes ambiguous if the version value
// contains the same separator.
const validateVersion = function (version, name, id) {
if (version.includes(VERSIONS_VALUE_SEPARATOR)) {
throw new PluginError(
`Could not start runner "${id}".
Computing runner's "${name}" version failed because it cannot contain "${VERSIONS_VALUE_SEPARATOR}": "${version}"`,
)
}
}
2 changes: 1 addition & 1 deletion src/top/system/versions/merge.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@ const mergeVersionsValue = function (values, previousValue) {
[...valuesArray, previousValue].sort().join(VERSIONS_VALUE_SEPARATOR)
}

const VERSIONS_VALUE_SEPARATOR = ', '
export const VERSIONS_VALUE_SEPARATOR = ', '

0 comments on commit 1f9173b

Please sign in to comment.