Skip to content

Commit

Permalink
fix: sanitize newlines and pipes from markdown table cells
Browse files Browse the repository at this point in the history
  • Loading branch information
bmish committed Oct 13, 2023
1 parent c0f2f03 commit bdc3664
Show file tree
Hide file tree
Showing 10 changed files with 199 additions and 16 deletions.
16 changes: 7 additions & 9 deletions lib/config-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
import { markdownTable } from 'markdown-table';
import type { ConfigsToRules, ConfigEmojis, Plugin } from './types.js';
import { ConfigFormat, configNameToDisplay } from './config-format.js';
import { sanitizeMarkdownTable } from './string.js';

function generateConfigListMarkdown(
plugin: Plugin,
Expand All @@ -26,28 +27,25 @@ function generateConfigListMarkdown(
}

return markdownTable(
[
sanitizeMarkdownTable([
listHeaderRow,
...Object.keys(configsToRules)
.filter((configName) => !ignoreConfig.includes(configName))
.sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase()))
.map((configName) => {
const description = // @ts-expect-error -- description is not an official config property.
plugin.configs?.[configName]?.description as string | undefined;
return [
configEmojis.find((obj) => obj.config === configName)?.emoji || '',
`\`${configNameToDisplay(
configName,
configFormat,
pluginPrefix
)}\``,
hasDescription
? // @ts-expect-error -- description is not an official config property.
(plugin.configs?.[configName]?.description as
| string
| undefined) || ''
: undefined,
].filter((col) => col !== undefined);
hasDescription ? description || '' : undefined,
].filter((col) => col !== undefined) as string[];
}),
],
]),
{ align: 'l' } // Left-align headers.
);
}
Expand Down
10 changes: 6 additions & 4 deletions lib/rule-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import type {
import { EMOJIS_TYPE } from './rule-type.js';
import { hasOptions } from './rule-options.js';
import { getLinkToRule } from './rule-link.js';
import { capitalizeOnlyFirstLetter } from './string.js';
import { capitalizeOnlyFirstLetter, sanitizeMarkdownTable } from './string.js';
import { noCase } from 'no-case';
import { getProperty } from 'dot-prop';
import { boolean, isBooleanable } from 'boolean';
Expand Down Expand Up @@ -178,7 +178,9 @@ function buildRuleRow(
[COLUMN_TYPE.REQUIRES_TYPE_CHECKING]: rule.meta?.docs?.requiresTypeChecking
? EMOJI_REQUIRES_TYPE_CHECKING
: '',
[COLUMN_TYPE.TYPE]: rule.meta?.type ? EMOJIS_TYPE[rule.meta?.type] : '',
[COLUMN_TYPE.TYPE]: rule.meta?.type
? EMOJIS_TYPE[rule.meta?.type] || ''
: '',
};

// List columns using the ordering and presence of columns specified in columnsEnabled.
Expand Down Expand Up @@ -222,7 +224,7 @@ function generateRulesListMarkdown(
});

return markdownTable(
[
sanitizeMarkdownTable([
listHeaderRow,
...ruleNamesAndRules.map(([name, rule]) =>
buildRuleRow(
Expand All @@ -240,7 +242,7 @@ function generateRulesListMarkdown(
urlRuleDoc
)
),
],
]),
{ align: 'l' } // Left-align headers.
);
}
Expand Down
6 changes: 3 additions & 3 deletions lib/rule-options-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
import { markdownTable } from 'markdown-table';
import type { RuleModule } from './types.js';
import { RuleOption, getAllNamedOptions } from './rule-options.js';
import { capitalizeOnlyFirstLetter } from './string.js';
import { capitalizeOnlyFirstLetter, sanitizeMarkdownTable } from './string.js';

export enum COLUMN_TYPE {
// Alphabetical order.
Expand Down Expand Up @@ -116,11 +116,11 @@ function generateRuleOptionsListMarkdown(rule: RuleModule): string {
// Recreate object using correct ordering and presence of columns.
return Object.keys(COLUMN_TYPE_DEFAULT_PRESENCE_AND_ORDERING)
.filter((type) => columnsToDisplay[type as COLUMN_TYPE])
.map((type) => ruleOptionColumnValues[type as COLUMN_TYPE]);
.map((type) => ruleOptionColumnValues[type as COLUMN_TYPE] || '');
});

return markdownTable(
[listHeaderRow, ...rows],
sanitizeMarkdownTable([listHeaderRow, ...rows]),
{ align: 'l' } // Left-align headers.
);
}
Expand Down
10 changes: 10 additions & 0 deletions lib/string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,13 @@ export function addTrailingPeriod(str: string) {
export function capitalizeOnlyFirstLetter(str: string) {
return str.charAt(0).toUpperCase() + str.slice(1).toLowerCase();
}

function sanitizeMarkdownTableCell(text: string): string {
return text.replace(/\|/gu, '\\|').replace(/\n/gu, '<br/>');

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.
}

export function sanitizeMarkdownTable(
text: readonly (readonly string[])[]
): readonly (readonly string[])[] {
return text.map((row) => row.map((col) => sanitizeMarkdownTableCell(col)));
}
19 changes: 19 additions & 0 deletions test/lib/generate/__snapshots__/configs-list-test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,25 @@ exports[`generate (configs list) basic generates the documentation 1`] = `
<!-- end auto-generated configs list -->"
`;

exports[`generate (configs list) when a config description needs to be escaped in table generates the documentation 1`] = `
"## Rules
<!-- begin auto-generated rules list -->
| Name | Description |
| :----------------------------- | :------------------ |
| [no-foo](docs/rules/no-foo.md) | Description no-foo. |
<!-- end auto-generated rules list -->
## Configs
<!-- begin auto-generated configs list -->
| | Name | Description |
| :- | :------------ | :---------- |
| ✅ | \`recommended\` | Foo\\|Bar |
<!-- end auto-generated configs list -->"
`;

exports[`generate (configs list) when a config exports a description generates the documentation 1`] = `
"## Rules
<!-- begin auto-generated rules list -->
Expand Down
12 changes: 12 additions & 0 deletions test/lib/generate/__snapshots__/rule-description-test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,15 @@ exports[`generate (rule descriptions) rule with long-enough description to requi
<!-- end auto-generated rule header -->
"
`;

exports[`generate (rule descriptions) with rule description that needs to be escaped in table generates the documentation 1`] = `
"## Rules
<!-- begin auto-generated rules list -->
| Name | Description |
| :----------------------------- | :---------- |
| [no-foo](docs/rules/no-foo.md) | Foo\\|Bar |
<!-- end auto-generated rules list -->
"
`;
14 changes: 14 additions & 0 deletions test/lib/generate/__snapshots__/rule-options-list-test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,19 @@ exports[`generate (rule options list) with no options generates the documentatio
<!-- end auto-generated rule options list -->"
`;

exports[`generate (rule options list) with string that needs to be escaped generates the documentation 1`] = `
"# test/no-foo
<!-- end auto-generated rule header -->
## Options
<!-- begin auto-generated rule options list -->
| Name | Description | Type |
| :---- | :------------------------------ | :------------- |
| \`foo\` | test<br/> desc | String\\|number |
<!-- end auto-generated rule options list -->"
`;
45 changes: 45 additions & 0 deletions test/lib/generate/configs-list-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,51 @@ describe('generate (configs list)', function () {
});
});

describe('when a config description needs to be escaped in table', function () {
beforeEach(function () {
mockFs({
'package.json': JSON.stringify({
name: 'eslint-plugin-test',
exports: 'index.js',
type: 'module',
}),

'index.js': `
export default {
rules: {
'no-foo': {
meta: { docs: { description: 'Description no-foo.' }, },
create(context) {}
},
},
configs: {
recommended: { description: 'Foo|Bar' },
}
};`,

'README.md': `## Rules
## Configs
<!-- begin auto-generated configs list -->
<!-- end auto-generated configs list -->`,

'docs/rules/no-foo.md': '',

// Needed for some of the test infrastructure to work.
node_modules: mockFs.load(PATH_NODE_MODULES),
});
});

afterEach(function () {
mockFs.restore();
jest.resetModules();
});

it('generates the documentation', async function () {
await generate('.');
expect(readFileSync('README.md', 'utf8')).toMatchSnapshot();
});
});

describe('when there are no configs', function () {
beforeEach(function () {
mockFs({
Expand Down
39 changes: 39 additions & 0 deletions test/lib/generate/rule-description-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,4 +208,43 @@ describe('generate (rule descriptions)', function () {
expect(readFileSync('docs/rules/no-bar.md', 'utf8')).toMatchSnapshot();
});
});

describe('with rule description that needs to be escaped in table', function () {
beforeEach(function () {
mockFs({
'package.json': JSON.stringify({
name: 'eslint-plugin-test',
exports: 'index.js',
type: 'module',
}),

'index.js': `
export default {
rules: {
'no-foo': {
meta: { docs: { description: 'Foo|Bar'} },
create(context) {},
},
},
};`,

'README.md': '## Rules\n',

'docs/rules/no-foo.md': '',

// Needed for some of the test infrastructure to work.
node_modules: mockFs.load(PATH_NODE_MODULES),
});
});

afterEach(function () {
mockFs.restore();
jest.resetModules();
});

it('generates the documentation', async function () {
await generate('.');
expect(readFileSync('README.md', 'utf8')).toMatchSnapshot();
});
});
});
44 changes: 44 additions & 0 deletions test/lib/generate/rule-options-list-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,48 @@ describe('generate (rule options list)', function () {
expect(readFileSync('docs/rules/no-foo.md', 'utf8')).toMatchSnapshot();
});
});

describe('with string that needs to be escaped', function () {
beforeEach(function () {
mockFs({
'package.json': JSON.stringify({
name: 'eslint-plugin-test',
exports: 'index.js',
type: 'module',
}),

'index.js': `
export default {
rules: {
'no-foo': {
meta: {
schema: [{ type: "object", properties: { foo: { description: \`test
desc\`, type: 'string|number' } } }]
},
create(context) {}
},
},
};`,

'README.md': '## Rules\n',

'docs/rules/no-foo.md': `## Options
<!-- begin auto-generated rule options list -->
<!-- end auto-generated rule options list -->`,

// Needed for some of the test infrastructure to work.
node_modules: mockFs.load(PATH_NODE_MODULES),
});
});

afterEach(function () {
mockFs.restore();
jest.resetModules();
});

it('generates the documentation', async function () {
await generate('.');
expect(readFileSync('docs/rules/no-foo.md', 'utf8')).toMatchSnapshot();
});
});
});

0 comments on commit bdc3664

Please sign in to comment.