evidence icon indicating copy to clipboard operation
evidence copied to clipboard

Source Variable substitution only works for single occurrence

Open smilingthax opened this issue 10 months ago • 1 comments

Steps To Reproduce

(This can also be reproduced using a real sources/ file, but is harder to show).

Import subSourceVariables from https://github.com/evidence-dev/evidence/blob/next/packages/lib/plugin-connector/src/data-sources/sub-source-vars.js into nodejs:

process.env.EVIDENCE_VAR__var_a = 'abc';   // "hack", usually already set in the calling environment
process.env.EVIDENCE_VAR__var_b = 'def';

console.log(subSourceVariables('|${var_a}|${var_b}|'));

Environment

  • Node version (node -v): v20.5.1

Expected Result

'|abc|def|'

Actual Result

'|abc|${vardef'

Workarounds

Maybe: Use only a single variable?

Better: Fix implementation to not rely on (or: adjust) match indices that become invalid after the first replacement.

smilingthax avatar Apr 10 '24 14:04 smilingthax

I believe this whole (buggy) part:

	let output = queryString;
	// This regex is prefixed with a negative lookbehind to disqualify $${var} patterns
	const regex = RegExp(/(?<!\$)\$\{(.+?)\}/, 'g');

	let match;
	while ((match = regex.exec(queryString)) !== null) {
		const fullMatch = match[0]; // e.g. ${variable}
		const varName = match[1]; // e.g. variable
		const start = match.index;
		const end = match[0].length + start;
		if (...replacement exists...) {
			const value = [... obtain replacement...]
			const before = output.substring(0, start);
			const after = output.substring(end);
			output = `${before}${value}${after}`;

can simply be replaced with

	// This regex is prefixed with a negative lookbehind to disqualify $${var} patterns
	const regex = RegExp(/(?<!\$)\$\{(.+?)\}/, 'g');

	let output = queryString.replaceAll(regex, (fullMatch, varName) => {
		return [...obtain replacement or warn/throw];
	});

(in older JS standards, .replace(regex, (...) => ...) would also work; in newer JS, .replaceAll is a bit clearer.)

smilingthax avatar Apr 12 '24 11:04 smilingthax

closed by #2173

mcrascal avatar Jul 15 '24 20:07 mcrascal