Skip to content

Fix Graphviz WASM module not loading for graph viewer #2085

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Mar 2, 2023
Merged

Fix Graphviz WASM module not loading for graph viewer #2085

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bc51e74
Fix Graphviz WASM module not loading for graph viewer
koesie10 Feb 15, 2023
d3e6453
Only allow WASM execution in results view
koesie10 Feb 28, 2023
b3d9804
Only enable graph viewer in canary mode
koesie10 Mar 1, 2023
074229e
Clarify some comments
koesie10 Mar 1, 2023
3043300
Add some additional safety to `allowWasmEval`
koesie10 Mar 1, 2023
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
2 changes: 2 additions & 0 deletions extensions/ql-vscode/src/abstract-webview.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type WebviewPanelConfig = {
view: WebviewView;
preserveFocus?: boolean;
additionalOptions?: WebviewPanelOptions & WebviewOptions;
allowWasmEval?: boolean;
};

export abstract class AbstractWebview<
Expand Down Expand Up @@ -116,6 +117,7 @@ export abstract class AbstractWebview<
config.view,
{
allowInlineStyles: true,
allowWasmEval: config.allowWasmEval ?? false,
},
);
this.push(
Expand Down
9 changes: 7 additions & 2 deletions extensions/ql-vscode/src/interface-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,13 @@ export function getHtmlForWebview(
view: WebviewView,
{
allowInlineStyles,
allowWasmEval,
}: {
allowInlineStyles?: boolean;
allowWasmEval?: boolean;
} = {
allowInlineStyles: false,
allowWasmEval: false,
},
): string {
const scriptUriOnDisk = Uri.file(ctx.asAbsolutePath("out/webview.js"));
Expand Down Expand Up @@ -163,7 +166,7 @@ export function getHtmlForWebview(
/*
* Content security policy:
* default-src: allow nothing by default.
* script-src: allow only the given script, using the nonce.
* script-src: allow the given script, using the nonce. also allow loading WebAssembly modules.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* script-src: allow the given script, using the nonce. also allow loading WebAssembly modules.
* script-src: allow the given script, using the nonce.
* wasm-unsafe-eval: possibly allow loading WebAssembly modules

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'wasm-unsafe-eval' string is actually part of script-src, so I've split it broken up the script-src into two parts: one for the script and one for wasm eval.

* style-src: allow only the given stylesheet, using the nonce.
* connect-src: only allow fetch calls to webview resource URIs
* (this is used to load BQRS result files).
Expand All @@ -172,7 +175,9 @@ export function getHtmlForWebview(
<html>
<head>
<meta http-equiv="Content-Security-Policy"
content="default-src 'none'; script-src 'nonce-${nonce}'; font-src ${fontSrc}; style-src ${styleSrc}; connect-src ${
content="default-src 'none'; script-src 'nonce-${nonce}'${
allowWasmEval ? " 'wasm-unsafe-eval'" : ""
}; font-src ${fontSrc}; style-src ${styleSrc}; connect-src ${
webview.cspSource
};">
${stylesheetsHtmlLines.join(` ${EOL}`)}
Expand Down
7 changes: 5 additions & 2 deletions extensions/ql-vscode/src/interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import {
ResultSetSchema,
} from "./pure/bqrs-cli-types";
import { AbstractWebview, WebviewPanelConfig } from "./abstract-webview";
import { PAGE_SIZE } from "./config";
import { isCanary, PAGE_SIZE } from "./config";
import { HistoryItemLabelProvider } from "./query-history/history-item-label-provider";
import { telemetryListener } from "./telemetry";
import { redactableError } from "./pure/errors";
Expand Down Expand Up @@ -221,6 +221,8 @@ export class ResultsView extends AbstractWebview<
viewColumn: this.chooseColumnForWebview(),
preserveFocus: true,
view: "results",
// Required for the graph viewer which is using d3-graphviz WASM module. Only supported in canary mode.
allowWasmEval: isCanary(),
};
}

Expand Down Expand Up @@ -656,7 +658,8 @@ export class ResultsView extends AbstractWebview<
}
let data;
let numTotalResults;
if (metadata?.kind === GRAPH_TABLE_NAME) {
// Graph results are only supported in canary mode because the graph viewer is unsupported
if (metadata?.kind === GRAPH_TABLE_NAME && isCanary()) {
data = await interpretGraphResults(
this.cliServer,
metadata,
Expand Down
8 changes: 4 additions & 4 deletions extensions/ql-vscode/src/view/results/graph.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
InterpretedResultSet,
GraphInterpretationData,
} from "../../pure/interface-types";
import { graphviz } from "d3-graphviz";
import { graphviz, GraphvizOptions } from "d3-graphviz";
import { tryGetLocationFromString } from "../../pure/bqrs-utils";
export type GraphProps = ResultTableProps & {
resultSet: InterpretedResultSet<GraphInterpretationData>;
Expand Down Expand Up @@ -59,11 +59,12 @@ export class Graph extends React.Component<GraphProps> {
return;
}

const options = {
const options: GraphvizOptions = {
fit: true,
fade: false,
growEnteringEdges: false,
zoom: true,
useWorker: false,
};

const element = document.querySelector(`#${graphId}`);
Expand All @@ -77,8 +78,7 @@ export class Graph extends React.Component<GraphProps> {
const borderColor = getComputedStyle(element).borderColor;
let firstPolygon = true;

graphviz(`#${graphId}`)
.options(options)
graphviz(`#${graphId}`, options)
.attributer(function (d) {
if (d.tag === "a") {
const url = d.attributes["xlink:href"] || d.attributes["href"];
Expand Down