luciano/diff2html
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor: Unify escaping

Browse source
This commit is contained in:
Rodrigo Fernandes 2019-11-26 09:44:09 +00:00
parent f8f5c10c57
commit 0f08c85938
No known key found for this signature in database
GPG key ID: 67157D2E3D4258B4
7 changed files with 188 additions and 200 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/__tests__/diff2html-tests.ts
View file

@ -299,7 +299,7 @@ describe("Diff2Html", () => {
' $a="<table><tr><td>\n' +
" $a=\"<table><tr><td>- 1.1.9: Fix around ubuntu's inability to cache promises. [#877](https://github.com/FredrikNoren/ungit/pull/878)\n" +
" - 1.1.8:\n" +
"@@ -11,7 +10,7 @@ $a=\"<table><tr><td>- 1.1.9: Fix around ubuntu's inability to cache promises. [#8\n" +
"@@ -11,7 +10,7 @@ $a=&quot;&lt;table&gt;&lt;tr&gt;&lt;td&gt;- 1.1.9: Fix around ubuntu&#x27;s inability to cache promises. [#8\n" +
" - 1.1.7:\n" +
" - Fix diff flickering issue and optimization [#865](https://github.com/FredrikNoren/ungit/pull/865)\n" +
" - Fix credential dialog issue [#864](https://github.com/FredrikNoren/ungit/pull/864)\n" +

59
src/__tests__/printer-utils-tests.ts
View file

@ -1,17 +1,50 @@
import * as renderUtils from "../render-utils";
import { escapeForHtml, getHtmlId, filenameDiff, diffHighlight } from "../render-utils";
import { DiffStyleType, LineMatchingType } from "../types";
describe("Utils", () => {
describe("escapeForHtml", () => {
it("should escape & with &amp;", () => {
const result = escapeForHtml("&");
expect(result).toEqual("&amp;");
});
it("should escape < with &lt;", () => {
const result = escapeForHtml("<");
expect(result).toEqual("&lt;");
});
it("should escape > with &gt;", () => {
const result = escapeForHtml(">");
expect(result).toEqual("&gt;");
});
it('should escape " with &quot;', () => {
const result = escapeForHtml('"');
expect(result).toEqual("&quot;");
});
it("should escape ' with &#x27;", () => {
const result = escapeForHtml("'");
expect(result).toEqual("&#x27;");
});
it("should escape / with &#x2F;", () => {
const result = escapeForHtml("/");
expect(result).toEqual("&#x2F;");
});
it("should escape a string containing HTML code", () => {
const result = escapeForHtml(`<a href="/search?q=diff2html">Search 'Diff2Html'</a>`);
expect(result).toEqual(
"&lt;a href=&quot;&#x2F;search?q=diff2html&quot;&gt;Search &#x27;Diff2Html&#x27;&lt;&#x2F;a&gt;"
);
});
});
describe("getHtmlId", () => {
it("should generate file unique id", () => {
const result = renderUtils.getHtmlId({
const result = getHtmlId({
oldName: "sample.js",
newName: "sample.js"
});
expect("d2h-960013").toEqual(result);
});
it("should generate file unique id for empty hashes", () => {
const result = renderUtils.getHtmlId({
const result = getHtmlId({
oldName: "sample.js",
newName: "sample.js"
});
@ -21,49 +54,49 @@ describe("Utils", () => {
describe("getDiffName", () => {
it("should generate the file name for a changed file", () => {
const result = renderUtils.filenameDiff({
const result = filenameDiff({
oldName: "sample.js",
newName: "sample.js"
});
expect("sample.js").toEqual(result);
});
it("should generate the file name for a changed file and full rename", () => {
const result = renderUtils.filenameDiff({
const result = filenameDiff({
oldName: "sample1.js",
newName: "sample2.js"
});
expect("sample1.js → sample2.js").toEqual(result);
});
it("should generate the file name for a changed file and prefix rename", () => {
const result = renderUtils.filenameDiff({
const result = filenameDiff({
oldName: "src/path/sample.js",
newName: "source/path/sample.js"
});
expect("{src → source}/path/sample.js").toEqual(result);
});
it("should generate the file name for a changed file and suffix rename", () => {
const result = renderUtils.filenameDiff({
const result = filenameDiff({
oldName: "src/path/sample1.js",
newName: "src/path/sample2.js"
});
expect("src/path/{sample1.js → sample2.js}").toEqual(result);
});
it("should generate the file name for a changed file and middle rename", () => {
const result = renderUtils.filenameDiff({
const result = filenameDiff({
oldName: "src/really/big/path/sample.js",
newName: "src/small/path/sample.js"
});
expect("src/{really/big → small}/path/sample.js").toEqual(result);
});
it("should generate the file name for a deleted file", () => {
const result = renderUtils.filenameDiff({
const result = filenameDiff({
oldName: "src/my/file.js",
newName: "/dev/null"
});
expect("src/my/file.js").toEqual(result);
});
it("should generate the file name for a new file", () => {
const result = renderUtils.filenameDiff({
const result = filenameDiff({
oldName: "/dev/null",
newName: "src/my/file.js"
});
@ -73,7 +106,7 @@ describe("Utils", () => {
describe("diffHighlight", () => {
it("should highlight two lines", () => {
const result = renderUtils.diffHighlight("-var myVar = 2;", "+var myVariable = 3;", false, {
const result = diffHighlight("-var myVar = 2;", "+var myVariable = 3;", false, {
matching: LineMatchingType.WORDS
});
@ -89,7 +122,7 @@ describe("Utils", () => {
});
});
it("should highlight two lines char by char", () => {
const result = renderUtils.diffHighlight("-var myVar = 2;", "+var myVariable = 3;", false, {
const result = diffHighlight("-var myVar = 2;", "+var myVariable = 3;", false, {
diffStyle: DiffStyleType.CHAR
});
@ -105,7 +138,7 @@ describe("Utils", () => {
}).toEqual(result);
});
it("should highlight combined diff lines", () => {
const result = renderUtils.diffHighlight(" -var myVar = 2;", " +var myVariable = 3;", true, {
const result = diffHighlight(" -var myVar = 2;", " +var myVariable = 3;", true, {
diffStyle: DiffStyleType.WORD,
matching: LineMatchingType.WORDS,
matchWordsThreshold: 1.0

57
src/__tests__/utils-tests.ts
View file

@ -1,4 +1,4 @@
import { escapeForRegExp, escapeForHtml, unifyPath, hashCode } from "../utils";
import { escapeForRegExp, unifyPath, hashCode } from "../utils";
describe("Utils", () => {
describe("escapeForRegExp", () => {
@ -12,53 +12,20 @@ describe("Utils", () => {
});
});
describe("escapeForHtml", () => {
it("should escape & with &amp;", () => {
const result = escapeForHtml("&");
expect(result).toEqual("&amp;");
});
it("should escape < with &lt;", () => {
const result = escapeForHtml("<");
expect(result).toEqual("&lt;");
});
it("should escape > with &gt;", () => {
const result = escapeForHtml(">");
expect(result).toEqual("&gt;");
});
it('should escape " with &quot;', () => {
const result = escapeForHtml('"');
expect(result).toEqual("&quot;");
});
it("should escape ' with &#x27;", () => {
const result = escapeForHtml("'");
expect(result).toEqual("&#x27;");
});
it("should escape / with &#x2F;", () => {
const result = escapeForHtml("/");
expect(result).toEqual("&#x2F;");
});
it("should escape a string containing HTML code", () => {
const result = escapeForHtml(`<a href="/search?q=diff2html">Search 'Diff2Html'</a>`);
expect(result).toEqual(
"&lt;a href=&quot;&#x2F;search?q=diff2html&quot;&gt;Search &#x27;Diff2Html&#x27;&lt;&#x2F;a&gt;"
);
describe("unifyPath", () => {
it("should unify windows style path", () => {
const result = unifyPath("\\Users\\Downloads\\diff.html");
expect(result).toEqual("/Users/Downloads/diff.html");
});
});
describe("unifyPath", () => {
it("should unify windows style path", () => {
const result = unifyPath("\\Users\\Downloads\\diff.html");
expect(result).toEqual("/Users/Downloads/diff.html");
});
describe("hashCode", () => {
it("should create consistent hash for a text piece", () => {
const string = "/home/diff2html/diff.html";
expect(hashCode(string)).toEqual(hashCode(string));
});
describe("hashCode", () => {
it("should create consistent hash for a text piece", () => {
const string = "/home/diff2html/diff.html";
expect(hashCode(string)).toEqual(hashCode(string));
});
it("should create different hash for different text pieces", () => {
expect(hashCode("/home/diff2html/diff1.html")).not.toEqual(hashCode("/home/diff2html/diff2.html"));
});
it("should create different hash for different text pieces", () => {
expect(hashCode("/home/diff2html/diff1.html")).not.toEqual(hashCode("/home/diff2html/diff2.html"));
});
});
});

166
src/line-by-line-renderer.ts
View file

@ -1,8 +1,7 @@
import * as utils from "./utils";
import HoganJsUtils from "./hoganjs-utils";
import * as Rematch from "./rematch";
import * as renderUtils from "./render-utils";
import { DiffFile, DiffBlock, DiffLine, LineType } from "./types";
import { DiffFile, DiffLine, LineType } from "./types";
export interface LineByLineRendererConfig extends renderUtils.RenderConfig {
renderNothingWhenEmpty?: boolean;
@ -72,53 +71,6 @@ export default class LineByLineRenderer {
});
}
// TODO: Make this private after improving tests
makeColumnLineNumberHtml(block: DiffBlock): string {
return this.hoganUtils.render(genericTemplatesPath, "column-line-number", {
CSSLineClass: renderUtils.CSSLineClass,
blockHeader: utils.escapeForHtml(block.header),
lineClass: "d2h-code-linenumber",
contentClass: "d2h-code-line"
});
}
// TODO: Make this private after improving tests
makeLineHtml(
isCombined: boolean,
type: renderUtils.CSSLineClass,
content: string,
oldNumber?: number,
newNumber?: number,
possiblePrefix?: string
): string {
const lineNumberTemplate = this.hoganUtils.render(baseTemplatesPath, "numbers", {
oldNumber: oldNumber || "",
newNumber: newNumber || ""
});
let lineWithoutPrefix = content;
let prefix = possiblePrefix;
if (!prefix) {
const lineWithPrefix = renderUtils.deconstructLine(content, isCombined);
prefix = lineWithPrefix.prefix;
lineWithoutPrefix = lineWithPrefix.content;
}
if (prefix === " ") {
prefix = "&nbsp;";
}
return this.hoganUtils.render(genericTemplatesPath, "line", {
type: type,
lineClass: "d2h-code-linenumber",
contentClass: "d2h-code-line",
prefix: prefix,
content: lineWithoutPrefix,
lineNumber: lineNumberTemplate
});
}
// TODO: Make this private after improving tests
generateEmptyDiff(): string {
return this.hoganUtils.render(genericTemplatesPath, "empty-diff", {
@ -127,47 +79,20 @@ export default class LineByLineRenderer {
});
}
// TODO: Make this private after improving tests
processLines(isCombined: boolean, oldLines: DiffLine[], newLines: DiffLine[]): string {
let lines = "";
for (let i = 0; i < oldLines.length; i++) {
const oldLine = oldLines[i];
const oldEscapedLine = utils.escapeForHtml(oldLine.content);
lines += this.makeLineHtml(
isCombined,
renderUtils.toCSSClass(oldLine.type),
oldEscapedLine,
oldLine.oldNumber,
oldLine.newNumber
);
}
for (let j = 0; j < newLines.length; j++) {
const newLine = newLines[j];
const newEscapedLine = utils.escapeForHtml(newLine.content);
lines += this.makeLineHtml(
isCombined,
renderUtils.toCSSClass(newLine.type),
newEscapedLine,
newLine.oldNumber,
newLine.newNumber
);
}
return lines;
}
// TODO: Make this private after improving tests
generateFileHtml(file: DiffFile): string {
const distance = Rematch.newDistanceFn(
(e: DiffLine) => renderUtils.deconstructLine(e.content, file.isCombined).content
const matcher = Rematch.newMatcherFn(
Rematch.newDistanceFn((e: DiffLine) => renderUtils.deconstructLine(e.content, file.isCombined).content)
);
const matcher = Rematch.newMatcherFn(distance);
return file.blocks
.map(block => {
let lines = this.makeColumnLineNumberHtml(block);
let lines = this.hoganUtils.render(genericTemplatesPath, "column-line-number", {
CSSLineClass: renderUtils.CSSLineClass,
blockHeader: block.header,
lineClass: "d2h-code-linenumber",
contentClass: "d2h-code-line"
});
let oldLines: DiffLine[] = [];
let newLines: DiffLine[] = [];
@ -242,8 +167,7 @@ export default class LineByLineRenderer {
for (let i = 0; i < block.lines.length; i++) {
const diffLine = block.lines[i];
const { prefix, content: line } = renderUtils.deconstructLine(diffLine.content, file.isCombined);
const escapedLine = utils.escapeForHtml(line);
const { prefix, content } = renderUtils.deconstructLine(diffLine.content, file.isCombined);
if (
diffLine.type !== LineType.INSERT &&
@ -256,7 +180,7 @@ export default class LineByLineRenderer {
lines += this.makeLineHtml(
file.isCombined,
renderUtils.toCSSClass(diffLine.type),
escapedLine,
content,
diffLine.oldNumber,
diffLine.newNumber,
prefix
@ -265,7 +189,7 @@ export default class LineByLineRenderer {
lines += this.makeLineHtml(
file.isCombined,
renderUtils.toCSSClass(diffLine.type),
escapedLine,
content,
diffLine.oldNumber,
diffLine.newNumber,
prefix
@ -286,4 +210,70 @@ export default class LineByLineRenderer {
})
.join("\n");
}
// TODO: Make this private after improving tests
makeLineHtml(
isCombined: boolean,
type: renderUtils.CSSLineClass,
content: string,
oldNumber?: number,
newNumber?: number,
possiblePrefix?: string
): string {
const lineNumberTemplate = this.hoganUtils.render(baseTemplatesPath, "numbers", {
oldNumber: oldNumber || "",
newNumber: newNumber || ""
});
let lineWithoutPrefix = content;
let prefix = possiblePrefix;
if (!prefix) {
const lineWithPrefix = renderUtils.deconstructLine(content, isCombined);
prefix = lineWithPrefix.prefix;
lineWithoutPrefix = lineWithPrefix.content;
}
if (prefix === " ") {
prefix = "&nbsp;";
}
return this.hoganUtils.render(genericTemplatesPath, "line", {
type: type,
lineClass: "d2h-code-linenumber",
contentClass: "d2h-code-line",
prefix: prefix,
content: lineWithoutPrefix,
lineNumber: lineNumberTemplate
});
}
// TODO: Make this private after improving tests
processLines(isCombined: boolean, oldLines: DiffLine[], newLines: DiffLine[]): string {
let lines = "";
for (let i = 0; i < oldLines.length; i++) {
const oldLine = oldLines[i];
lines += this.makeLineHtml(
isCombined,
renderUtils.toCSSClass(oldLine.type),
oldLine.content,
oldLine.oldNumber,
oldLine.newNumber
);
}
for (let j = 0; j < newLines.length; j++) {
const newLine = newLines[j];
lines += this.makeLineHtml(
isCombined,
renderUtils.toCSSClass(newLine.type),
newLine.content,
newLine.oldNumber,
newLine.newNumber
);
}
return lines;
}
}

28
src/render-utils.ts
View file

@ -1,6 +1,6 @@
import * as jsDiff from "diff";
import { unifyPath, escapeForHtml, hashCode } from "./utils";
import { unifyPath, hashCode } from "./utils";
import * as rematch from "./rematch";
import { LineMatchingType, DiffStyleType, LineType, DiffLineParts, DiffFile, DiffFileName } from "./types";
@ -75,6 +75,21 @@ function prefixLength(isCombined: boolean): number {
return isCombined ? 2 : 1;
}
/**
* Escapes all required characters for safe HTML rendering
*/
// TODO: Test this method inside deconstructLine since it should not be used anywhere else
export function escapeForHtml(str: string): string {
return str
.slice(0)
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;")
.replace(/'/g, "&#x27;")
.replace(/\//g, "&#x2F;");
}
/**
* Deconstructs diff @line by separating the content from the prefix type
*/
@ -82,7 +97,7 @@ export function deconstructLine(line: string, isCombined: boolean): DiffLinePart
const indexToSplit = prefixLength(isCombined);
return {
prefix: line.substring(0, indexToSplit),
content: line.substring(indexToSplit)
content: escapeForHtml(line.substring(indexToSplit))
};
}
@ -209,11 +224,11 @@ export function diffHighlight(
return {
oldLine: {
prefix: line1.prefix,
content: escapeForHtml(line1.content)
content: line1.content
},
newLine: {
prefix: line2.prefix,
content: escapeForHtml(line2.content)
content: line2.content
}
};
}
@ -248,11 +263,10 @@ export function diffHighlight(
const highlightedLine = diff.reduce((highlightedLine, part) => {
const elemType = part.added ? "ins" : part.removed ? "del" : null;
const addClass = changedWords.indexOf(part) > -1 ? ' class="d2h-change"' : "";
const escapedValue = escapeForHtml(part.value);
return elemType !== null
? `${highlightedLine}<${elemType}${addClass}>${escapedValue}</${elemType}>`
: `${highlightedLine}${escapedValue}`;
? `${highlightedLine}<${elemType}${addClass}>${part.value}</${elemType}>`
: `${highlightedLine}${part.value}`;
}, "");
return {

62
src/side-by-side-renderer.ts
View file

@ -1,4 +1,3 @@
import * as utils from "./utils";
import HoganJsUtils from "./hoganjs-utils";
import * as Rematch from "./rematch";
import * as renderUtils from "./render-utils";
@ -47,20 +46,10 @@ export default class SideBySideRenderer {
return this.hoganUtils.render(genericTemplatesPath, "wrapper", { content: diffsHtml });
}
// TODO: Make this private after improving tests
generateEmptyDiff(): FileHtml {
return {
right: "",
left:
this.hoganUtils.render(genericTemplatesPath, "empty-diff", {
contentClass: "d2h-code-side-line",
CSSLineClass: renderUtils.CSSLineClass
}) || ""
};
}
// TODO: Make this private after improving tests
makeFileDiffHtml(file: DiffFile, diffs: FileHtml): string {
if (this.config.renderNothingWhenEmpty && Array.isArray(file.blocks) && file.blocks.length === 0) return "";
const fileDiffTemplate = this.hoganUtils.template(baseTemplatesPath, "file-diff");
const filePathTemplate = this.hoganUtils.template(genericTemplatesPath, "file-path");
const fileIconTemplate = this.hoganUtils.template(iconsBaseTemplatesPath, "file");
@ -83,21 +72,21 @@ export default class SideBySideRenderer {
}
// TODO: Make this private after improving tests
makeSideHtml(blockHeader: string): string {
return this.hoganUtils.render(genericTemplatesPath, "column-line-number", {
CSSLineClass: renderUtils.CSSLineClass,
blockHeader: utils.escapeForHtml(blockHeader),
lineClass: "d2h-code-side-linenumber",
contentClass: "d2h-code-side-line"
});
generateEmptyDiff(): FileHtml {
return {
right: "",
left: this.hoganUtils.render(genericTemplatesPath, "empty-diff", {
contentClass: "d2h-code-side-line",
CSSLineClass: renderUtils.CSSLineClass
})
};
}
// TODO: Make this private after improving tests
generateSideBySideFileHtml(file: DiffFile): FileHtml {
const distance = Rematch.newDistanceFn(
(e: DiffLine) => renderUtils.deconstructLine(e.content, file.isCombined).content
const matcher = Rematch.newMatcherFn(
Rematch.newDistanceFn((e: DiffLine) => renderUtils.deconstructLine(e.content, file.isCombined).content)
);
const matcher = Rematch.newMatcherFn(distance);
const fileHtml = {
right: "",
@ -183,8 +172,7 @@ export default class SideBySideRenderer {
for (let i = 0; i < block.lines.length; i++) {
const diffLine = block.lines[i];
const { prefix, content: line } = renderUtils.deconstructLine(diffLine.content, file.isCombined);
const escapedLine = utils.escapeForHtml(line);
const { prefix, content } = renderUtils.deconstructLine(diffLine.content, file.isCombined);
if (
diffLine.type !== LineType.INSERT &&
@ -197,14 +185,14 @@ export default class SideBySideRenderer {
fileHtml.left += this.generateSingleLineHtml(
file.isCombined,
renderUtils.toCSSClass(diffLine.type),
escapedLine,
content,
diffLine.oldNumber,
prefix
);
fileHtml.right += this.generateSingleLineHtml(
file.isCombined,
renderUtils.toCSSClass(diffLine.type),
escapedLine,
content,
diffLine.newNumber,
prefix
);
@ -213,7 +201,7 @@ export default class SideBySideRenderer {
fileHtml.right += this.generateSingleLineHtml(
file.isCombined,
renderUtils.toCSSClass(diffLine.type),
escapedLine,
content,
diffLine.newNumber,
prefix
);
@ -233,6 +221,16 @@ export default class SideBySideRenderer {
return fileHtml;
}
// TODO: Make this private after improving tests
makeSideHtml(blockHeader: string): string {
return this.hoganUtils.render(genericTemplatesPath, "column-line-number", {
CSSLineClass: renderUtils.CSSLineClass,
blockHeader: blockHeader,
lineClass: "d2h-code-side-linenumber",
contentClass: "d2h-code-side-line"
});
}
// TODO: Make this private after improving tests
processLines(isCombined: boolean, oldLines: DiffLine[], newLines: DiffLine[]): FileHtml {
const fileHtml = {
@ -251,8 +249,8 @@ export default class SideBySideRenderer {
let newPrefix;
if (oldLine) {
const { prefix, content: line } = renderUtils.deconstructLine(oldLine.content, isCombined);
oldContent = utils.escapeForHtml(line);
const { prefix, content } = renderUtils.deconstructLine(oldLine.content, isCombined);
oldContent = content;
oldPrefix = prefix;
} else {
oldContent = "";
@ -260,8 +258,8 @@ export default class SideBySideRenderer {
}
if (newLine) {
const { prefix, content: line } = renderUtils.deconstructLine(newLine.content, isCombined);
newContent = utils.escapeForHtml(line);
const { prefix, content } = renderUtils.deconstructLine(newLine.content, isCombined);
newContent = content;
newPrefix = prefix;
} else {
newContent = "";

14
src/utils.ts
View file

@ -30,20 +30,6 @@ export function escapeForRegExp(str: string): string {
return str.replace(regex, "\\$&");
}
/**
* Escapes all required characters for safe HTML rendering
*/
export function escapeForHtml(str: string): string {
return str
.slice(0)
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;")
.replace(/'/g, "&#x27;")
.replace(/\//g, "&#x2F;");
}
/**
* Converts all '\' in @path to unix style '/'
*/