Skip to content

Commit 9c399c7

Browse files
Filmbostock
andauthored
build filters files outside the root (#103)
* build filters files outside the root reverts part of #89 fixes #99 * fix tests and test imports of non-existing files * more tests but I'm not sure if I'm using this correctly * Update test/input/bar.js Co-authored-by: Mike Bostock <[email protected]> * fix test * fix test, align signatures * don’t canReadSync in isLocalPath * syntax error on non-local file path --------- Co-authored-by: Mike Bostock <[email protected]>
1 parent 48c20d1 commit 9c399c7

13 files changed

+277
-30
lines changed

src/files.ts

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,14 @@
1-
import {type Stats, accessSync, constants, statSync} from "node:fs";
1+
import {type Stats} from "node:fs";
22
import {mkdir, readdir, stat} from "node:fs/promises";
33
import {dirname, extname, join, normalize, relative} from "node:path";
44
import {isNodeError} from "./error.js";
55

6-
// A file is local if it exists in the root folder or a subfolder.
7-
export function isLocalFile(ref: string | null, root: string): boolean {
8-
return (
9-
typeof ref === "string" &&
10-
!/^(\w+:)\/\//.test(ref) &&
11-
!normalize(ref).startsWith("../") &&
12-
canReadSync(join(root, ref))
13-
);
14-
}
15-
16-
export function pathFromRoot(ref: string | null, root: string): string | null {
17-
return isLocalFile(ref, root) ? join(root, ref!) : null;
18-
}
19-
20-
function canReadSync(path: string): boolean {
21-
try {
22-
accessSync(path, constants.R_OK);
23-
return statSync(path).isFile();
24-
} catch (error) {
25-
if (isNodeError(error) && error.code === "ENOENT") return false;
26-
throw error;
27-
}
6+
// A path is local if it doesn’t go outside the the root.
7+
export function getLocalPath(sourcePath: string, name: string): string | null {
8+
if (/^(\w+:)\/\//.test(name)) return null; // URL
9+
const path = join(dirname(sourcePath.startsWith("/") ? sourcePath.slice("/".length) : sourcePath), name);
10+
if (path.startsWith("../")) return null; // goes above root
11+
return path;
2812
}
2913

3014
export async function* visitMarkdownFiles(root: string): AsyncGenerator<string> {

src/javascript/features.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {simple} from "acorn-walk";
2+
import {getLocalPath} from "../files.js";
23
import {type Feature} from "../javascript.js";
34
import {isLocalImport} from "./imports.js";
45
import {syntaxError} from "./syntaxError.js";
@@ -36,7 +37,13 @@ export function findFeatures(node, root, sourcePath, references, input) {
3637
throw syntaxError(`${callee.name} requires a single literal string argument`, node, input);
3738
}
3839

39-
features.push({type: callee.name, name: getStringLiteralValue(arg)});
40+
// Forbid file attachments that are not local paths.
41+
const value = getStringLiteralValue(arg);
42+
if (callee.name === "FileAttachment" && !getLocalPath(sourcePath, value)) {
43+
throw syntaxError(`non-local file path: "${value}"`, node, input);
44+
}
45+
46+
features.push({type: callee.name, name: value});
4047
}
4148
});
4249

src/markdown.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {readFile} from "node:fs/promises";
2-
import {dirname, join} from "node:path";
2+
import {join} from "node:path";
33
import {type Patch, type PatchItem, getPatch} from "fast-array-diff";
44
import equal from "fast-deep-equal";
55
import matter from "gray-matter";
@@ -11,7 +11,7 @@ import {type RuleInline} from "markdown-it/lib/parser_inline.js";
1111
import {type RenderRule, type default as Renderer} from "markdown-it/lib/renderer.js";
1212
import MarkdownItAnchor from "markdown-it-anchor";
1313
import mime from "mime";
14-
import {isLocalFile, pathFromRoot} from "./files.js";
14+
import {getLocalPath} from "./files.js";
1515
import {computeHash} from "./hash.js";
1616
import {type FileReference, type ImportReference, type Transpile, transpileJavaScript} from "./javascript.js";
1717
import {transpileTag} from "./tag.js";
@@ -322,8 +322,8 @@ function normalizePieceHtml(html: string, root: string, sourcePath: string, cont
322322
const {document} = parseHTML(html);
323323
for (const element of document.querySelectorAll("link[href]") as any as Iterable<Element>) {
324324
const href = element.getAttribute("href")!;
325-
const path = join(dirname(sourcePath), href);
326-
if (isLocalFile(path, root)) {
325+
const path = getLocalPath(sourcePath, href);
326+
if (path) {
327327
context.files.push({name: href, mimeType: mime.getType(href)});
328328
element.setAttribute("href", `/_file/${path}`);
329329
}
@@ -463,6 +463,6 @@ export function diffMarkdown({parse: prevParse}: ReadMarkdownResult, {parse: nex
463463
}
464464

465465
export async function readMarkdown(path: string, root: string): Promise<ReadMarkdownResult> {
466-
const contents = await readFile(pathFromRoot(path, root)!, "utf-8");
466+
const contents = await readFile(join(root, path), "utf-8");
467467
return {contents, parse: parseMarkdown(contents, root, path), hash: computeHash(contents)};
468468
}

src/preview.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ function handleWatch(socket: WebSocket, options: {root: string; resolver: CellRe
260260
let {path} = message;
261261
if (!(path = normalize(path)).startsWith("/")) throw new Error("Invalid path: " + path);
262262
if (path.endsWith("/")) path += "index";
263-
path = path.slice("/".length) + ".md";
263+
path += ".md";
264264
markdownWatcher = watch(join(root, path), await refreshMarkdown(path));
265265
break;
266266
}

test/input/bar.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const bar = Symbol("bar");

test/input/dynamic-import-noent.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const foo = await import("./noent.js");

test/input/fetch-parent-dir.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# Parent dir
2+
3+
Trying to fetch from the parent directory should fail.
4+
5+
```js
6+
const fail1 = fetch("../NOENT.md").then(d => d.text())
7+
```
8+
9+
```js
10+
const fail2 = FileAttachment("../NOENT.md").text()
11+
```
12+
13+
```js
14+
const fail3 = fetch("../README.md").then(d => d.text())
15+
```
16+
17+
```js
18+
const fail4 = FileAttachment("../README.md").text()
19+
```
20+
21+
```js
22+
const fail5 = fetch("./NOENT.md").then(d => d.text())
23+
```
24+
25+
```js
26+
const fail6 = FileAttachment("./NOENT.md").text()
27+
```
28+
29+
```js
30+
const ok1 = fetch("./tex-expression.md").then(d => d.text())
31+
```
32+
33+
```js
34+
const ok2 = FileAttachment("./tex-expression.md").text()
35+
```
36+

test/input/static-import-noent.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import {foo} from "./noent.js";
2+
3+
display(foo);

test/output/bar.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
define({id: "0", outputs: ["bar"], body: () => {
2+
const bar = Symbol("bar");
3+
return {bar};
4+
}});

test/output/dynamic-import-noent.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
define({id: "0", outputs: ["foo"], body: async () => {
2+
const foo = await import("/_import/noent.js");
3+
return {foo};
4+
}});

0 commit comments

Comments
 (0)