Skip to content

Commit

Permalink
refactor: ensure immutability to avoid side effects in HTML encoding
Browse files Browse the repository at this point in the history
Previously, the encoding was done inline using `map`, which could lead to confusion
due to the side effect of mutating the `lines` objects within `exampleCommitFixes`.
Since `map` should ideally be used to create a new array without side effects, this was not
the advised use of the method.
  • Loading branch information
cat2608 committed Dec 19, 2023
1 parent a1c763b commit 8af1606
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 7 deletions.
16 changes: 16 additions & 0 deletions src/snyk/snykCode/utils/htmlEncoder.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import he from 'he';
import { ExampleCommitFix } from '../../common/languageServer/types';

export const encodeExampleCommitFixes = (exampleCommitFixes: ExampleCommitFix[]): ExampleCommitFix[] => {
return exampleCommitFixes.map(exampleCommitFixes => {
return {
...exampleCommitFixes,
lines: exampleCommitFixes.lines.map(line => {
return {
...line,
line: he.encode(line.line),
};
}),
};
});
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import he from 'he';
import _ from 'lodash';
import * as vscode from 'vscode';
import {
Expand All @@ -22,6 +21,7 @@ import { IVSCodeWorkspace } from '../../../common/vscode/workspace';
import { WEBVIEW_PANEL_QUALITY_TITLE, WEBVIEW_PANEL_SECURITY_TITLE } from '../../constants/analysis';
import { messages as errorMessages } from '../../messages/error';
import { getAbsoluteMarkerFilePath } from '../../utils/analysisUtils';
import { encodeExampleCommitFixes } from '../../utils/htmlEncoder';
import { IssueUtils } from '../../utils/issueUtils';
import { ICodeSuggestionWebviewProvider } from '../interfaces';

Expand Down Expand Up @@ -113,12 +113,7 @@ export class CodeSuggestionWebviewProvider
this.registerListeners();
}

issue.additionalData.exampleCommitFixes.map(ecf => {
return ecf.lines.map(l => {
l.line = he.encode(l.line);
return l;
});
});
issue.additionalData.exampleCommitFixes = encodeExampleCommitFixes(issue.additionalData.exampleCommitFixes);

this.panel.webview.html = this.getHtmlForWebview(this.panel.webview);
this.panel.iconPath = vscode.Uri.joinPath(
Expand Down
70 changes: 70 additions & 0 deletions src/test/unit/snykCode/helpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { strictEqual } from 'assert';
import { ExampleCommitFix } from '../../../snyk/common/languageServer/types';
import { encodeExampleCommitFixes } from '../../../snyk/snykCode/utils/htmlEncoder';

const exampleInput: ExampleCommitFix[] = [
{
commitURL:
'https://github.com/hiddentao/fast-levenshtein/commit/8d67bde78c9e75b5e253b0e84d0cbf227ffb98f9?diff=split#diff-a58852f173bee316460a26934b8a3c4d79b1acb88af655346ae74adce78113f7L-1',
lines: [
{
line: '\t\t\t\t\tuploadUserPicture(req.user.uid, req.files.userPhoto.name, req.files.userPhoto.path, res);\n',
lineNumber: 101,
lineChange: 'removed',
},
{
line: " files.map(file => path.join(...[dashboardAppPath].concat(file.fileName.split('/'))))\n",
lineNumber: 145,
lineChange: 'added',
},
],
},
{
commitURL:
'https://github.com/guzzle/guzzle/commit/3d499a1b7ce589c3ef39d78bb1730d83d8a89c79?diff=split#diff-dea67d130560147fa1eeb821ab305ce5c0d44c5a3a0b16f3f11b4cb48008dcbaL-1',
lines: [
{
line: "\tvar template = routes['/' + req.params.path] || routes['/'];\n",
lineNumber: 43,
lineChange: 'added',
},
{
line: "\t\t\tconsole.log('Info: Attempting upload to: '+ uploadPath);\n",
lineNumber: 130,
lineChange: 'none',
},
],
},
{
commitURL: 'https://github.com/example/security-fix/commit/1234567890abcdef?diff=split#diff-securityL-1',
lines: [
{
line: `'that.responses = eval('('+ request.body + ')');\n'`, // Potential security vulnerability: eval with user input
lineNumber: 50,
lineChange: 'removed',
},
],
},
];

suite('encodeExampleCommitFixes', () => {
test('should encode lines', () => {
// The `he` library encodes characters into their hexadecimal numeric character reference.
const encodedResults = encodeExampleCommitFixes(exampleInput);

strictEqual(
encodedResults[0].lines[0].line,
'					uploadUserPicture(req.user.uid, req.files.userPhoto.name, req.files.userPhoto.path, res);\n',
);

strictEqual(
encodedResults[0].lines[1].line,
' files.map(file => path.join(...[dashboardAppPath].concat(file.fileName.split('/'))))\n',
);

strictEqual(
encodedResults[2].lines[0].line,
''that.responses = eval('('+ request.body + ')');\n' + ''',
);
});
});

0 comments on commit 8af1606

Please sign in to comment.