Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Check script tags for inline JS #48

Merged
merged 34 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
5d59158
feat: Add html linter
d3xter666 Mar 22, 2024
2b19bf6
feat: Add html parser
d3xter666 Mar 22, 2024
61c869c
feat: Implement html script linter
d3xter666 Mar 25, 2024
5a9700a
refactor: Rename files
d3xter666 Mar 25, 2024
ccc974e
fix: Eslint issues
d3xter666 Mar 25, 2024
fec7d7f
fix: Add new files to the tests
d3xter666 Mar 25, 2024
ecb1350
refactor: Reword messageDetails
d3xter666 Mar 26, 2024
ae2f229
feat: Add reporting to the script linter
d3xter666 Mar 26, 2024
7c5eca9
feat: Add test cases
d3xter666 Mar 26, 2024
7a3500d
fix: Eslint issues
d3xter666 Mar 26, 2024
5f2c6ba
feat: Add messageDetails
d3xter666 Mar 26, 2024
cb755df
fix: --file-paths with non-js resources
d3xter666 Mar 26, 2024
15ae266
fix: Eslint
d3xter666 Mar 26, 2024
2604e63
fix: Tests
d3xter666 Mar 26, 2024
6e58189
feat: Add test cases
d3xter666 Mar 27, 2024
dcfa43b
fix: Eslint
d3xter666 Mar 27, 2024
efdbd26
fix: Tests for Windows
d3xter666 Mar 27, 2024
9242980
fix: Tests for Windows
d3xter666 Mar 27, 2024
4dd42cf
refactor: Remove redundant checks
d3xter666 Mar 27, 2024
69ef393
refactor: Rewrite --file-paths filtering fallback to js
d3xter666 Mar 27, 2024
f3fce8d
refactor: Treat html files as JSX
d3xter666 Mar 28, 2024
dc916c0
feat: Parse HTML files as JSX
d3xter666 Mar 28, 2024
95ad30c
fix: Fix tests
d3xter666 Mar 28, 2024
8042a49
refactor: Address GH comments
d3xter666 Mar 29, 2024
7ece8f9
fix: Use jsx: preserve option for scanned documents
d3xter666 Mar 29, 2024
615f5c1
refactor: Optimize resource filtering
d3xter666 Mar 29, 2024
810c8d0
refactor: Enforce utf8 encoding for html fixtures
d3xter666 Mar 29, 2024
8992d55
refactor: Adjust report details for CSP
d3xter666 Apr 4, 2024
4914973
refactor: Avoid parsing of HTML content as JSX
d3xter666 Apr 4, 2024
ed9d9e7
refactor: Move js script checks in the parser
d3xter666 Apr 5, 2024
6f6bb45
fix: Rename html files to JSX
d3xter666 Apr 5, 2024
3fd956f
fix: Add new lines at the end of html tests
d3xter666 Apr 5, 2024
0cd6efb
fix: Ignore case when checking script's type
d3xter666 Apr 9, 2024
93495c6
feat: Add tests for application/javascript
d3xter666 Apr 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/detectors/BaseReporter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type ts from "typescript";
import type {LintMessage, CoverageInfo, LintResult} from "../detectors/AbstractDetector.js";
import type {Tag as SaxTag} from "sax-wasm";

export interface BaseReporter {
addMessage(args: ReporterMessage): void;
Expand All @@ -8,7 +9,7 @@ export interface BaseReporter {
}

export interface ReporterMessage {
node?: ts.Node | string;
node?: ts.Node | SaxTag | string;
message: LintMessage["message"];
messageDetails?: LintMessage["messageDetails"];
severity: LintMessage["severity"];
Expand Down
76 changes: 76 additions & 0 deletions src/detectors/transpilers/html/parser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import type {ReadStream} from "node:fs";
import {Detail, SaxEventType, SAXParser, Tag as SaxTag} from "sax-wasm";
import {finished} from "node:stream/promises";
import fs from "node:fs/promises";
import {createRequire} from "node:module";
const require = createRequire(import.meta.url);

let saxWasmBuffer: Buffer;
async function initSaxWasm() {
if (!saxWasmBuffer) {
const saxPath = require.resolve("sax-wasm/lib/sax-wasm.wasm");
saxWasmBuffer = await fs.readFile(saxPath);
}

return saxWasmBuffer;
}

async function parseHtml(contentStream: ReadStream, parseHandler: (type: SaxEventType, tag: Detail) => void) {
const options = {highWaterMark: 32 * 1024}; // 32k chunks
const saxWasmBuffer = await initSaxWasm();
const saxParser = new SAXParser(SaxEventType.CloseTag, options);

saxParser.eventHandler = parseHandler;

// Instantiate and prepare the wasm for parsing
if (!await saxParser.prepareWasm(saxWasmBuffer)) {
throw new Error("Unknown error during WASM Initialization");
}

// stream from a file in the current directory
contentStream.on("data", (chunk: Uint8Array) => {
try {
saxParser.write(chunk);
} catch (err) {
if (err instanceof Error) {
// In case of an error, destroy the content stream to make the
// error bubble up to our callers
contentStream.destroy(err);
} else {
throw err;
}
}
});
await finished(contentStream);
saxParser.end();
}

export async function extractJSScriptTags(contentStream: ReadStream) {
const scriptTags: SaxTag[] = [];

await parseHtml(contentStream, (event, tag) => {
if (tag instanceof SaxTag &&
event === SaxEventType.CloseTag &&
tag.value === "script") {
const isJSScriptTag = tag.attributes.every((attr) => {
// The "type" attribute of the script tag should be
// 1. not set (default),
// 2. an empty string,
// 3. or a JavaScript MIME type (text/javascript)
// https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script/type#attribute_is_not_set_default_an_empty_string_or_a_javascript_mime_type
return attr.name.value !== "type" ||
(attr.name.value === "type" &&
["",
"text/javascript",
"application/javascript", /* legacy */
d3xter666 marked this conversation as resolved.
Show resolved Hide resolved
].includes(attr.value.value.toLowerCase()));
});

if (isJSScriptTag) {
scriptTags.push(tag);
}
}
});

return scriptTags;
}
38 changes: 33 additions & 5 deletions src/detectors/typeChecker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {taskStart} from "../util/perf.js";
import {amdToEsm} from "../transpilers/amd/transpiler.js";
import {xmlToJs} from "../transpilers/xml/transpiler.js";
import {lintManifest} from "../../linter/json/linter.js";
import {lintHtml} from "../../linter/html/linter.js";
import {
FileBasedDetector, LintMessage, LintMessageSeverity, LintResult, ProjectBasedDetector,
} from "../AbstractDetector.js";
Expand Down Expand Up @@ -116,6 +117,13 @@ export class TsProjectDetector extends ProjectBasedDetector {
resourcePath = resourcePath.replace(/\.json$/, ".js");
const resourceContent = await resource.getString();
({source, messages} = await lintManifest(resourcePath, resourceContent));
} else if (resourcePath.endsWith(".html")) {
resourcePath = resourcePath.replace(/\.html$/, ".jsx");
d3xter666 marked this conversation as resolved.
Show resolved Hide resolved
// TODO: Enable when implement script extraction and parse
// Details: TS treats HTML as JSX, but parsing results are not consistent. https://github.com/SAP/ui5-linter/pull/48#discussion_r1551412367
// source = await resource.getString();
source = "";
({messages} = await lintHtml(resourcePath, resource.getStream()));
} else {
throw new Error(`Unsupported file type for ${resourcePath}`);
}
Expand Down Expand Up @@ -159,7 +167,7 @@ export class TsProjectDetector extends ProjectBasedDetector {

// Read all resources and test-resources and their content since tsc works completely synchronous
const globEnd = taskStart("Locating Resources");
const fileTypes = "{*.js,*.view.xml,*.fragment.xml,manifest.json}";
const fileTypes = "{*.js,*.view.xml,*.fragment.xml,manifest.json,*.html}";
const allResources = await reader.byGlob("/resources/**/" + fileTypes);
const allTestResources = await reader.byGlob("/test-resources/**/" + fileTypes);
globEnd();
Expand Down Expand Up @@ -212,11 +220,20 @@ export class TsProjectDetector extends ProjectBasedDetector {
});

// Rewrite fs-paths to virtual paths
resourcePaths = allResources.map((res: Resource) => {
if (absoluteFilePaths.includes(res.getSourceMetadata().fsPath)) {
return res.getPath();
resourcePaths = [...allResources, ...allTestResources].map((res: Resource) => {
if (!absoluteFilePaths.includes(res.getSourceMetadata().fsPath)) {
return;
}
}).filter(($: string | undefined) => $);

let resPath = res.getPath();
if (resPath.endsWith(".html")) {
resPath = resPath.replace(/\.[a-z]+$/, ".jsx");
} else if (!resPath.endsWith(".js")) {
resPath = resPath.replace(/\.[a-z]+$/, ".js");
}
return resPath;
})
.filter(($: string | undefined) => $);
} else {
resourcePaths = Array.from(resources.keys());
}
Expand Down Expand Up @@ -290,6 +307,17 @@ export class TsFileDetector extends FileBasedDetector {
}
internalfilePath = internalfilePath.replace(/\.json$/, ".js");
transformationResult = await lintManifest(filePath.replace(/\.json$/, ".js"), fileContent);
} else if (filePath.endsWith(".html")) {
// TODO: Enable when implement script extraction and parse
// Details: TS treats HTML as JSX, but parsing results are not consistent. https://github.com/SAP/ui5-linter/pull/48#discussion_r1551412367
// const fileContent = ts.sys.readFile(filePath);
// if (!fileContent) {
// throw new Error(`Failed to read file ${filePath}`);
// }
internalfilePath = internalfilePath.replace(/\.html$/, ".jsx");
transformationResult = await lintHtml(path.basename(filePath), fs.createReadStream(filePath));
// transformationResult.source = fileContent;
transformationResult.source = "";
} else {
throw new Error(`Unsupported file type for ${filePath}`);
}
Expand Down
84 changes: 84 additions & 0 deletions src/linter/html/HtmlReporter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import type {BaseReporter, ReporterMessage, ReporterCoverageInfo} from "../../detectors/BaseReporter.js";
import type {LintMessage} from "../../detectors/AbstractDetector.js";
import {Tag as SaxTag} from "sax-wasm";
import {LintMessageSeverity, CoverageInfo} from "../../detectors/AbstractDetector.js";
import {resolveLinks} from "../../formatter/lib/resolveLinks.js";

export default class HtmlReporter implements BaseReporter {
#filePath: string;
#messages: LintMessage[] = [];
#coverageInfo: CoverageInfo[] = [];

constructor(filePath: string) {
this.#filePath = filePath;
}

addMessage({node, message, messageDetails, severity, ruleId, fatal = undefined}: ReporterMessage) {
if (fatal && severity !== LintMessageSeverity.Error) {
throw new Error(`Reports flagged as "fatal" must be of severity "Error"`);
}

let line = 0, column = 0;
if (node instanceof SaxTag) {
({line, character: column} = node.openStart);
}

const msg: LintMessage = {
ruleId,
severity,
fatal,
line: line + 1,
column: column + 1,
message,
};

if (messageDetails) {
msg.messageDetails = resolveLinks(messageDetails);
}

this.#messages.push(msg);
}

addCoverageInfo({node, message, category}: ReporterCoverageInfo) {
let line = 0, column = 0, endLine = 0, endColumn = 0;
if (node instanceof SaxTag) {
({line, character: column} = node.openStart);
({line: endLine, character: endColumn} = node.closeEnd);
}

this.#coverageInfo.push({
category,
// One-based to be aligned with most IDEs
line: line + 1,
column: column + 1,
endLine: endLine + 1,
endColumn: endColumn + 1,
message,
});
}

getReport() {
let errorCount = 0;
let warningCount = 0;
let fatalErrorCount = 0;
for (const {severity, fatal} of this.#messages) {
if (severity === LintMessageSeverity.Error) {
errorCount++;
if (fatal) {
fatalErrorCount++;
}
} else {
warningCount++;
}
}

return {
filePath: this.#filePath,
messages: this.#messages,
coverageInfo: this.#coverageInfo,
errorCount,
warningCount,
fatalErrorCount,
};
}
}
32 changes: 32 additions & 0 deletions src/linter/html/linter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import {taskStart} from "../../detectors/util/perf.js";
import {extractJSScriptTags} from "../../detectors/transpilers/html/parser.js";
import {LintMessageSeverity} from "../../detectors/AbstractDetector.js";
import HtmlReporter from "./HtmlReporter.js";

import type {TranspileResult} from "../../detectors/transpilers/AbstractTranspiler.js";
import type {ReadStream} from "node:fs";

export async function lintHtml(resourceName: string, contentStream: ReadStream): Promise<TranspileResult> {
const taskLintEnd = taskStart("Linting HTML", resourceName);
const report = new HtmlReporter(resourceName);
const jsScriptTags = await extractJSScriptTags(contentStream);

jsScriptTags.forEach((tag) => {
const scriptContent = tag.textNodes?.map((tNode) => tNode.value).join("").trim();

if (scriptContent) {
report.addMessage({
node: tag,
severity: LintMessageSeverity.Warning,
ruleId: "ui5-linter-csp-unsafe-inline-script",
message: `Use of unsafe inline script`,
messageDetails: "{@link topic:fe1a6dba940e479fb7c3bc753f92b28c Content Security Policy}",
});
}
});

taskLintEnd();

const {messages} = report.getReport();
return {messages, source: "", map: ""};
}
48 changes: 48 additions & 0 deletions test/fixtures/linter/rules/CSPCompliance/NoInlineJS.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<html>

<head>
<meta charset="UTF-8">
<title>No inline JS</title>
matz3 marked this conversation as resolved.
Show resolved Hide resolved
</head>

<body>
<script type="text/javascript">
sap.ui.controller("my.own.controller", {
doSomething: function () {
alert("Hello World!");
}
});
</script>

<script type="application/javascript">
sap.ui.controller("my.own.controller", {
doSomething: function () {
alert("Hello World!");
}
});
</script>

<script type="">
(function() {
alert("test")
})();
</script>

<script>
sap.ui.controller("myController", {
onInit: function () {
var model = new sap.ui.model.json.JSONModel();
model.setData({
buttonText: "Click Me!"
});
this.getView().setModel(model);
},
doSomething: function () {
alert("Hello World!");
}
});
sap.ui.xmlview({ viewContent: jQuery('#myXml').html() }).placeAt("content");
</script>
</body>

</html>
32 changes: 32 additions & 0 deletions test/fixtures/linter/rules/CSPCompliance/NoInlineJS_negative.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<html>

<head>
<meta charset="UTF-8">
<title>No inline JS</title>
</head>

<body>
<script id="myXml" type="text/xmldata">
<mvc:View xmlns:mvc="sap.ui.core.mvc" xmlns="sap.m" controllerName="myController" displayBlock="true">
<App>
<Page title="Hello">
<Button text="{/buttonText}" press= "doSomething"/>
</Page>
</App>
</mvc:View>
</script>

<script type="module">
import { log } from "utils";

log("Exporting dog names.");

export const names = ["Kayla", "Bentley", "Gilligan"];
</script>

<script type="" src="./path/to/js.js"></script>

<script src="./another/path/to/js.js"></script>
</body>

</html>
d3xter666 marked this conversation as resolved.
Show resolved Hide resolved
Binary file modified test/lib/detectors/transpilers/xml/snapshots/transpiler.ts.snap
Binary file not shown.
7 changes: 5 additions & 2 deletions test/lib/linter/_linterHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,11 @@ export function createTestsForFixtures(fixturesPath: string) {
throw new Error(`Failed to find any fixtures in directory ${fixturesPath}`);
}
for (const fileName of testFiles) {
if (!fileName.endsWith(".js") && !fileName.endsWith(".xml") && !fileName.endsWith(".json")) {
// Ignore non-JavaScript, non-XML and non-JSON files
if (!fileName.endsWith(".js") &&
!fileName.endsWith(".xml") &&
!fileName.endsWith(".json") &&
!fileName.endsWith(".html")) {
// Ignore non-JavaScript, non-XML, non-JSON and non-HTML files
continue;
}

Expand Down
10 changes: 10 additions & 0 deletions test/lib/linter/rules/CSPCompliance.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import path from "node:path";
import {fileURLToPath} from "node:url";
import {createTestsForFixtures} from "../_linterHelper.js";

const filePath = fileURLToPath(import.meta.url);
const __dirname = path.dirname(filePath);
const fileName = path.basename(filePath, ".ts");
const fixturesPath = path.join(__dirname, "..", "..", "..", "fixtures", "linter", "rules", fileName);

createTestsForFixtures(fixturesPath);
Loading