test(row 333 follow-ups R2): refuse a dynamic import inside a template substitution, read division after ++ and -- (Vikunja #779)
The business-imports helper blanked a whole template literal, so a dynamic import inside a
${...} substitution was neither found nor refused and skipped the existence and containment
checks. The scanner now reads a substitution to its closing brace, refuses it with
"unsupported import inside a template substitution" when it holds a dynamic import, and drops
it otherwise, so a nested template and a brace inside a string no longer end it early.
A slash after a postfix ++ or -- was read as the start of a regular expression, so
`n++ / 2` threw "unterminated regular expression". After a ++ or -- pair that follows an
operand the slash is division; after one that follows an operator it still starts a regular
expression. The header names the rare form this still misreads.
Output on the real tree is byte-identical.
This commit is contained in:
1 parent
2ba22b4cbf
commit
d3b6ab11da
2 files changed
+122
-16
No files matched your search
@@ -8,11 +8,18 @@
|
||||
// it cannot read. Comments are blanked before matching, so an import in a comment is not one, and an
|
||||
// import form inside a string or a regular expression is not one either. A dynamic import written
|
||||
// with a space, `import ('./x.mjs')`, counts. These throw: a dynamic import whose argument is not a
|
||||
// plain string, a file with an unterminated string, comment, template or regular expression, a
|
||||
// target that is missing or not a regular file, and a target whose real path leaves the repository.
|
||||
// plain string, a dynamic import inside a template substitution (${...}), a file with an unterminated
|
||||
// string, comment, template or regular expression, a target that is missing or not a regular file,
|
||||
// and a target whose real path leaves the repository. A substitution with no dynamic import is
|
||||
// blanked with the rest of the template, as is `${import.meta.url}`; the text of a template and a
|
||||
// substitution that holds a template of its own are read to their ends.
|
||||
// The scanner knows line and block comments, quoted strings, template literals, regular expression
|
||||
// literals (by the usual guess from the previous token) and a hashbang. A template literal with a
|
||||
// backtick nested inside its ${} is outside that, and so is a regular expression it guesses wrong.
|
||||
// literals (by the usual guess from the previous token) and a hashbang. The guess is wrong for a
|
||||
// regular expression after `)` or a keyword, as in `return /x/.test(y)`. After `++` or `--` a slash
|
||||
// is division when the character before the pair ends an operand (`n++ / 2`) and starts a regular
|
||||
// expression when it does not (`x = ++/re/.lastIndex`). That misreads one rare form as division: a
|
||||
// prefix `++` or `--` applied to a regular expression's member right after `)`, `]`, a word or a
|
||||
// line break, as in `if (c) ++/re/.lastIndex`.
|
||||
import {readdirSync,readFileSync,realpathSync,statSync} from 'node:fs';
|
||||
import {dirname,join,relative,resolve,sep} from 'node:path';
|
||||
|
||||
@@ -20,16 +27,34 @@ const IMPORT=/(?:import|export)\s[^'"`;]*?\bfrom\s*['"](\.{1,2}\/[^'"]+)['"]|\bi
|
||||
const DYNAMIC=/(?<![.\w$])import\s*\(/g;
|
||||
const DYNAMIC_LITERAL=/^import\s*\(\s*(['"])[^'"\n]*\1\s*\)/;
|
||||
const REGEX_AFTER='(,=:[!&|?{};+-*%<>~^';
|
||||
const inert=(literals,index)=>literals.some(([from,to])=>index>=from&&index<to);
|
||||
const sources=(dir)=>readdirSync(dir,{withFileTypes:true}).flatMap((e)=>e.isDirectory()?sources(join(dir,e.name)):e.name.endsWith('.mjs')?[join(dir,e.name)]:[]);
|
||||
|
||||
// The text with every comment blanked and every template literal emptied, and the ranges of the
|
||||
// quoted strings and regular expressions left in it, as [from,to) offsets into the returned code.
|
||||
// Throws on a string, comment, template or regular expression that never ends.
|
||||
// Throws on a string, comment, template or regular expression that never ends, and on a dynamic
|
||||
// import inside a template substitution.
|
||||
function scan(text){
|
||||
let code='',i=0,previous='';
|
||||
const literals=[];
|
||||
let code='',i=0,previous='',before='';
|
||||
const literals=[],open=[];
|
||||
const fail=(what)=>{throw new Error(`unterminated ${what}`);};
|
||||
const keep=(end,last)=>{literals.push([code.length,code.length+end-i]);code+=text.slice(i,end);i=end;previous=last;};
|
||||
const mark=(token)=>{before=previous;previous=token;};
|
||||
const keep=(end,last)=>{literals.push([code.length,code.length+end-i]);code+=text.slice(i,end);i=end;mark(last);};
|
||||
// Template text from i to the closing backtick, or to the ${ that opens a substitution, which is then read as code.
|
||||
const template=()=>{
|
||||
for(;i<text.length;i++){
|
||||
if(text[i]==='\\')i++;
|
||||
else if(text[i]==='`'){i++;mark('`');return;}
|
||||
else if(text[i]==='$'&&text[i+1]==='{'){i+=2;open.push({braces:0,start:code.length});mark('{');return;}
|
||||
}
|
||||
fail('template');
|
||||
};
|
||||
// A substitution is read as code to its closing brace, refused if it holds a dynamic import, then dropped.
|
||||
const close=({start})=>{
|
||||
for(const m of code.slice(start).matchAll(DYNAMIC))if(!inert(literals,start+m.index))throw new Error('unsupported import inside a template substitution');
|
||||
code=code.slice(0,start);
|
||||
while(literals.length&&literals[literals.length-1][0]>=start)literals.pop();
|
||||
};
|
||||
if(text.startsWith('#!')){const end=text.indexOf('\n');i=end<0?text.length:end;}
|
||||
while(i<text.length){
|
||||
const c=text[i],next=text[i+1];
|
||||
@@ -41,12 +66,7 @@ function scan(text){
|
||||
if(j>=text.length)fail('string');
|
||||
keep(j+1,c);continue;
|
||||
}
|
||||
if(c==='`'){
|
||||
let j=i+1;
|
||||
for(;j<text.length&&text[j]!=='`';j++)if(text[j]==='\\')j++;
|
||||
if(j>=text.length)fail('template');
|
||||
code+='``';i=j+1;previous='`';continue;
|
||||
}
|
||||
if(c==='`'){code+='``';i++;template();continue;}
|
||||
if(c==='/'&&(previous===''||REGEX_AFTER.includes(previous))){
|
||||
let j=i+1,inClass=false;
|
||||
for(;j<text.length&&(inClass||text[j]!=='/');j++){
|
||||
@@ -56,9 +76,19 @@ function scan(text){
|
||||
if(j>=text.length)fail('regular expression');
|
||||
keep(j+1,'/');continue;
|
||||
}
|
||||
if(open.length&&(c==='{'||c==='}')){
|
||||
const top=open[open.length-1];
|
||||
if(c==='{')top.braces++;
|
||||
else if(top.braces===0){close(open.pop());i++;template();continue;}
|
||||
else top.braces--;
|
||||
}
|
||||
code+=c;i++;
|
||||
if(!/\s/.test(c))previous=c;
|
||||
if(!/\s/.test(c)){
|
||||
const postfix=(c==='+'||c==='-')&&previous===c&&text[i-2]===c&&before!==''&&!REGEX_AFTER.includes(before);
|
||||
mark(postfix?c+c:c);
|
||||
}
|
||||
}
|
||||
if(open.length)fail('template');
|
||||
return {code,literals};
|
||||
}
|
||||
|
||||
@@ -68,7 +98,7 @@ function specifiers(file,text){
|
||||
let scanned;
|
||||
try{scanned=scan(text);}
|
||||
catch(error){throw new Error(`${file} cannot be read for imports: ${error.message}`);}
|
||||
const {code,literals}=scanned,live=(m)=>!literals.some(([from,to])=>m.index>=from&&m.index<to);
|
||||
const {code,literals}=scanned,live=(m)=>!inert(literals,m.index);
|
||||
for(const m of code.matchAll(DYNAMIC)){
|
||||
if(live(m)&&!DYNAMIC_LITERAL.test(code.slice(m.index,m.index+512)))throw new Error(`${file} has a dynamic import that is not a plain string literal`);
|
||||
}
|
||||
|
||||
@@ -152,3 +152,79 @@ test('#779 a symlink that stays inside the repository, and a repository reached
|
||||
symlinkSync(repo,via);
|
||||
assert.deepEqual(businessImportedFiles(via),['packages/other/src/link.mjs']);
|
||||
});
|
||||
|
||||
const SUBSTITUTION=/packages\/business\/src\/a\.mjs cannot be read for imports: unsupported import inside a template substitution/;
|
||||
|
||||
test('#779 R2 a dynamic import inside a template substitution throws, whether the target exists, is missing or is outside', ()=>{
|
||||
const exists=withBusiness("const v=`${await import('../../other/src/real.mjs')}`;\n",{[`${EXT}real.mjs`]:EMPTY});
|
||||
assert.throws(()=>businessImportedFiles(exists),SUBSTITUTION);
|
||||
const missing=withBusiness("const v=`${await import('../../other/src/real.mjs')}`;\n");
|
||||
assert.throws(()=>businessImportedFiles(missing),SUBSTITUTION);
|
||||
const outside=tree({[`${BIZ}a.mjs`]:"const v=`${await import('../../../../outside.mjs')}`;\n",'outside.mjs':EMPTY});
|
||||
assert.throws(()=>businessImportedFiles(outside),SUBSTITUTION);
|
||||
});
|
||||
|
||||
test('#779 R2 a dynamic import inside a substitution throws however the substitution is written', ()=>{
|
||||
for(const source of [
|
||||
"const v=`a ${b} ${await import ('../../other/src/real.mjs')} c`;",
|
||||
"const v=`${await import(name)}`;",
|
||||
"const v=`${/* c */ import('../../other/src/real.mjs') /* d */}`;",
|
||||
"const v=`x ${`y ${await import('../../other/src/real.mjs')}`}`;",
|
||||
"const v=`${ {a:1}.a + '}' + (await import('../../other/src/real.mjs')) }`;",
|
||||
"const v=`${\n await import(\n '../../other/src/real.mjs'\n )\n}`;",
|
||||
]){
|
||||
const repo=withBusiness(`${source}\n`,{[`${EXT}real.mjs`]:EMPTY});
|
||||
assert.throws(()=>businessImportedFiles(repo),SUBSTITUTION,source);
|
||||
}
|
||||
const external=withBusiness("import '../../other/src/a.mjs';\n",{[`${EXT}a.mjs`]:"const v=`${await import('./b.mjs')}`;\n",[`${EXT}b.mjs`]:EMPTY});
|
||||
assert.throws(()=>businessImportedFiles(external),/packages\/other\/src\/a\.mjs cannot be read for imports: unsupported import inside a template substitution/);
|
||||
});
|
||||
|
||||
test('#779 R2 a substitution without a dynamic import is allowed, and the import around it is found', ()=>{
|
||||
const repo=withBusiness([
|
||||
"const a=`plain ${b} text ${c.d({e:1})} end`;",
|
||||
"const u=`${import.meta.url}/${x.import}/${loader.import('y')}`;",
|
||||
"const s=`${'}'}${\"import('../../other/src/missing1.mjs')\"}${/import('..\\/x')/.source}`;",
|
||||
"const t=`text import('../../other/src/missing2.mjs') ${a} import '../../other/src/missing3.mjs'`;",
|
||||
"const e=`\\${await import('../../other/src/missing4.mjs')}`;",
|
||||
"const n=`outer ${`inner ${a}`} tail`;",
|
||||
"import '../../other/src/real.mjs';",
|
||||
].join('\n'),{[`${EXT}real.mjs`]:EMPTY});
|
||||
assert.deepEqual(businessImportedFiles(repo),['packages/other/src/real.mjs']);
|
||||
});
|
||||
|
||||
test('#779 R2 a substitution or the template text after it that never ends throws', ()=>{
|
||||
for(const text of ['const a=`${b','const a=`${b} open','const a=`${`${b}']){
|
||||
const repo=withBusiness(text);
|
||||
assert.throws(()=>businessImportedFiles(repo),/packages\/business\/src\/a\.mjs cannot be read for imports: unterminated template/,text);
|
||||
}
|
||||
});
|
||||
|
||||
test('#779 R2 a slash after a postfix ++ or -- is division, not a regular expression', ()=>{
|
||||
for(const op of ['++','--']){
|
||||
const repo=withBusiness([
|
||||
'let n=2;',
|
||||
`const half=n${op} / 2;`,
|
||||
`const spaced=n ${op}/2;`,
|
||||
`const member=arr[0]${op} / 2;`,
|
||||
`const grouped=(n)${op} / 2 / 3;`,
|
||||
"import '../../other/src/real.mjs';",
|
||||
].join('\n'),{[`${EXT}real.mjs`]:EMPTY});
|
||||
assert.deepEqual(businessImportedFiles(repo),['packages/other/src/real.mjs'],op);
|
||||
}
|
||||
});
|
||||
|
||||
test('#779 R2 a regular expression after an operator, a bracket or a prefix ++ or -- is still a regular expression', ()=>{
|
||||
// Each body holds a quote, which would start an unterminated string if the slash were read as division.
|
||||
const repo=withBusiness([
|
||||
"const a = /it's/;",
|
||||
"if (/it's/.test(s)) {}",
|
||||
"const b = f(1, /it's \\/ x/);",
|
||||
"const c = a + /it's/.source;",
|
||||
"const d = a++ + /it's/.source;",
|
||||
"const e = ++/it's/.lastIndex;",
|
||||
"const f = 1 + --/it's/.lastIndex;",
|
||||
"import '../../other/src/real.mjs';",
|
||||
].join('\n'),{[`${EXT}real.mjs`]:EMPTY});
|
||||
assert.deepEqual(businessImportedFiles(repo),['packages/other/src/real.mjs']);
|
||||
});
|
||||
Reference in new issue
Block a user