Skip to content

Commit

Permalink
fix!(core/plugin): enforce sanity of params and phrase (#64)
Browse files Browse the repository at this point in the history 
* fix!(core/plugin): enforce sanity of params and phrase

* fix!(core/plugin): simplify the regex

It used be more complex and didn't cover the case where we used a param of
`p-256`, because it expected it to be preceded by a letter.

* test(core/plugin): add another case

Simply throws tests and add another case that tests for 'p-256'-like params and
phrases.
  • Loading branch information
@protodeniz
protodeniz authored Dec 12, 2023
1 parent 143b174 commit 9a0ed9c
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 105 deletions.
26 changes: 20 additions & 6 deletions pkg/core/src/plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,20 +115,21 @@ export class Plugin {
throw new Error('unreachable');
}

// TODO: allow dashes and underscores only in between words
if (phrase.split(' ').some((x) => !x.match(/^[0-9a-z_-]+$/)))
if (phrase.split(' ').some((x) => !isSane(x)))
throw new Error(
'phrase must composed of alpha-numerical, underscore, and dash values split by a single space',
'phrase must be composed of alpha-numerical, underscore, and dash values split by a single space',
);

const key: PluginMapKey = { phrase: phrase };
if (openconnect) key.openconnect = openconnect;
if (params) {
// TODO: allow dashes and underscores and only in between words
if (params.some((x) => !x.match(/^[0-9a-z_-]+$/)))
// TODO: allow spaces in params
const found = params.find((x) => !isSane(x));
if (found !== undefined)
throw new Error(
'each params must composed of alpha-numerical values, optionally split by dashes or underscores',
`the following parameter must be composed of alpha-numerical values, optionally split by dashes or underscores: ${found}`,
);

const duplicates = [
...params.reduce((acc, cur) => {
const found = acc.get(cur);
Expand All @@ -149,6 +150,19 @@ export class Plugin {
}
}

/**
* @internal
* Check the sanity of a Phrase or Param for the following:
*
* 1. Must start with a letter.
* 2. Dashes and underscores are allowed only inbetween words once.
* 3. Must be composed solely of letter, digits, dashes, and underscores.
*
* @param str The input to be checked for sanity.
* @returns whether it is sane or not.
*/
export const isSane = (str: string) => /^[a-z][a-z0-9]*([_-][a-z0-9]+)*$/.test(str);

// Todo: Maybe we should adapt some sort of monad library.

/**
Expand Down
193 changes: 94 additions & 99 deletions pkg/core/test/plugin.ts
View file
Original file line number Diff line number Diff line change
@@ -1,121 +1,116 @@
import test from 'ava';
import { DuplicatePluginError, Plugin, type PluginExecutor } from '@slangroom/core';
import { DuplicatePluginError, Plugin, isSane, type PluginExecutor } from '@slangroom/core';

const insanes = [
'',
' ',
' ',
'\n',
'NoN Lower cAse',
'double spacing',
'tabs in between',
'new\nlines',
' leading spaces',
'trailing spaces ',
' leading and trailing spaces ',
' leading tabs',
'trailing tabs ',
' leading and trailing tabs ',
"some 'identifiers' in between",
'not- good',
'not -bad',
'not-good-',
'not-bad-',
'not_ good',
'not _bad',
'not_good_',
'not_bad_',
];

test('isSane() works', (t) => {
insanes.forEach((x) => t.false(isSane(x)));
});

test('Plugin.new() phrase alpha-numerical checks work', (t) => {
[
'',
' ',
' ',
'\n',
'NoN Lower cAse',
'double spacing',
'tabs in between',
'new\nlines',
' leading spaces',
'trailing spaces ',
' leading and trailing spaces ',
' leading tabs',
'trailing tabs ',
' leading and trailing tabs ',
"some 'identifiers' in between",
].forEach((x) => {
const err = t.throws(() => new Plugin().new(x, (ctx) => ctx.pass(null)), {
insanes.forEach((x) =>
t.throws(() => new Plugin().new(x, (ctx) => ctx.pass(null)), {
instanceOf: Error,
}) as Error;
t.is(
err.message,
'phrase must composed of alpha-numerical, underscore, and dash values split by a single space',
);
});

['foo', 'foo bar', 'foo_bar', 'foo-bar', 'foo bar baz', 'foo_bar_baz', 'foo-bar-baz'].forEach(
(x) => {
t.notThrows(() => new Plugin().new(x, (ctx) => ctx.pass(null)));
},
message:
'phrase must be composed of alpha-numerical, underscore, and dash values split by a single space',
}),
);

[
'foo',
'foo bar',
'foo_bar',
'foo-bar',
'foo bar baz',
'foo_bar_baz',
'foo-bar-baz',
'p-p',
'p-256',
].forEach((x) => t.notThrows(() => new Plugin().new(x, (ctx) => ctx.pass(null)), x));
});

test('Plugin.new() params alpha-numerical checks work', (t) => {
[
'',
' ',
' ',
'\n',
'NoN Lower cAse',
'double spacing',
'tabs in between',
'new\nlines',
' leading spaces',
'trailing spaces ',
' leading and trailing spaces ',
' leading tabs',
'trailing tabs ',
' leading and trailing tabs ',
"some 'identifiers' in between",
'punctuation!',
].forEach((x) => {
const err = t.throws(
insanes.forEach((x) =>
t.throws(
() => new Plugin().new([x], 'doesnt matter', (ctx) => ctx.pass(null)),
{ instanceOf: Error },
) as Error;
t.is(
err.message,
'each params must composed of alpha-numerical values, optionally split by dashes or underscores',
);
});
{
instanceOf: Error,
message: `the following parameter must be composed of alpha-numerical values, optionally split by dashes or underscores: ${x}`,
},
x,
),
);

['foo', 'foo_bar', 'foo-bar', 'foo_bar_baz', 'foo-bar-baz'].forEach((x) => {
t.notThrows(() => new Plugin().new(x, (ctx) => ctx.pass(null)));
['foo', 'foo_bar', 'foo-bar', 'foo_bar_baz', 'foo-bar-baz', 'p-p', 'p-256'].forEach((x) => {
t.notThrows(() => new Plugin().new(x, (ctx) => ctx.pass(null)), x);
});
});

test('Plugin.new() duplicates detected', (t) => {
const f: PluginExecutor = (ctx) => ctx.pass(null);
t.is(
(
t.throws(
() => {
const p = new Plugin();
p.new('a', f);
p.new('a', f);
},
{ instanceOf: DuplicatePluginError },
) as DuplicatePluginError
).message,
`duplicated plugin with key: openconnect= params= phrase="a"`,
t.throws(
() => {
const p = new Plugin();
p.new('a', f);
p.new('a', f);
},
{
instanceOf: DuplicatePluginError,
message: `duplicated plugin with key: openconnect= params= phrase="a"`,
},
);

t.is(
(
t.throws(
() => {
const p = new Plugin();
p.new('a', f);
p.new('open', 'a', f);
p.new('open', 'a', f);
},
{ instanceOf: DuplicatePluginError },
) as DuplicatePluginError
).message,
`duplicated plugin with key: openconnect=open params= phrase="a"`,
t.throws(
() => {
const p = new Plugin();
p.new('a', f);
p.new('open', 'a', f);
p.new('open', 'a', f);
},
{
instanceOf: DuplicatePluginError,
message: `duplicated plugin with key: openconnect=open params= phrase="a"`,
},
);

t.is(
(
t.throws(
() => {
const p = new Plugin();
p.new('a', f);
p.new('open', 'a', f);
p.new('connect', 'a', f);
p.new('open', ['foo'], 'a', f);
p.new('connect', ['foo'], 'a', f);
p.new('connect', ['foo'], 'a', f);
},
{ instanceOf: DuplicatePluginError },
) as DuplicatePluginError
).message,
`duplicated plugin with key: openconnect=connect params=foo phrase="a"`,
t.throws(
() => {
const p = new Plugin();
p.new('a', f);
p.new('open', 'a', f);
p.new('connect', 'a', f);
p.new('open', ['foo'], 'a', f);
p.new('connect', ['foo'], 'a', f);
p.new('connect', ['foo'], 'a', f);
},
{
instanceOf: DuplicatePluginError,
message: `duplicated plugin with key: openconnect=connect params=foo phrase="a"`,
},
);
});

Expand Down

0 comments on commit 9a0ed9c

Please sign in to comment.