Skip to content

Where people and agents ship software together. The open-source git platform for the whole job: issues, agents, checks and deploys to the edge.

Commit

The site takes an image for a comment box's Markdown

Comment boxes on the site need somewhere to send a pasted or dropped image. The work service already keeps attachments for `g1t attach` and `--attach` (upload_attachment), so the site calls the same method: a POST to /-/attachments with the file as the body, for a repository or a work item, from the signed-in viewer on the same origin. The service still decides who may and what the bytes are; the route only turns away what is plainly too large or not an image before sending 10 MB across. lib/attachments.ts holds the parts with no DOM, tested under Node: which pastes upload (image files and no plain text, so a spreadsheet's cells still paste as text), the placeholder each upload puts in and how only that one is swapped, where the caret goes, and what a refusal says. Part of #159

G1t-Session:
ses_01m575ycxgejntcr3stqnexj6w
syntaqxcommitted Parente59e4c9Browse files
6 files+383−00/6 viewed
+112−0
1+import assert from "node:assert/strict";
2+import { test } from "node:test";
3+
4+import {
5+ MAX_ATTACHMENT_BYTES,
6+ caretAfter,
7+ deferredRefs,
8+ deferredUrl,
9+ dragsFiles,
10+ imageMarkdown,
11+ imageName,
12+ insertion,
13+ pastedImages,
14+ placeholderFor,
15+ refusal,
16+ replacePlaceholder,
17+ resolveDeferred,
18+ uploadFailure,
19+} from "./attachments.ts";
20+
21+const png = (name = "screenshot.png", size = 1000) => ({ name, type: "image/png", size });
22+
23+test("a paste uploads image files only when it has no text", () => {
24+ // A screenshot, or an image copied from a page (its HTML, no text).
25+ assert.deepEqual(pastedImages({ files: [png()], text: "" }), [png()]);
26+ // A spreadsheet's cells or a document's paragraphs, with a picture of themselves.
27+ assert.deepEqual(pastedImages({ files: [png("image.png")], text: "a\tb\n1\t2" }), []);
28+ // Plain text.
29+ assert.deepEqual(pastedImages({ files: [], text: "hello" }), []);
30+ // Files that are not images are left to the browser.
31+ assert.deepEqual(pastedImages({ files: [{ name: "a.pdf", type: "application/pdf", size: 3 }], text: "" }), []);
32+ // Several images, each its own.
33+ assert.equal(pastedImages({ files: [png("a.png"), png("b.png")], text: " " }).length, 2);
34+});
35+
36+test("only a drag that carries files is taken", () => {
37+ assert.equal(dragsFiles(["Files"]), true);
38+ assert.equal(dragsFiles(["text/plain", "text/html"]), false);
39+});
40+
41+test("what is refused before it is sent, in a plain sentence", () => {
42+ assert.equal(refusal(png()), null);
43+ assert.equal(refusal({ name: "big.png", type: "image/png", size: MAX_ATTACHMENT_BYTES + 1 }), "big.png is over 10 MB, so it wasn't attached.");
44+ assert.equal(refusal({ name: "logo.svg", type: "image/svg+xml", size: 10 }), "logo.svg isn't a PNG, JPEG, GIF or WebP image, so it wasn't attached.");
45+ assert.equal(refusal({ name: "x.png", type: "image/png", size: 0 }), "x.png is empty, so it wasn't attached.");
46+ assert.equal(refusal({ name: "a.jpg", type: "image/JPEG", size: 10 }), null);
47+});
48+
49+test("a pasted image with no name is image.png", () => {
50+ assert.equal(imageName({ name: "", type: "image/png", size: 1 }), "image.png");
51+ assert.equal(imageName({ name: "", type: "image/jpeg", size: 1 }), "image.jpg");
52+ assert.equal(imageName({ name: "C:\\shots\\Preview 1.png", type: "image/png", size: 1 }), "Preview 1.png");
53+});
54+
55+test("Markdown escapes the name's brackets", () => {
56+ assert.equal(imageMarkdown("a]b[c\\.png", "https://u/attachments/ab"), "![a\\]b\\[c\\\\.png](https://u/attachments/ab)");
57+});
58+
59+test("each upload has a placeholder of its own", () => {
60+ const first = placeholderFor("image.png", "");
61+ assert.equal(first, "![Uploading image.png…]()");
62+ const second = placeholderFor("image.png", `Look:\n${first}\n`);
63+ assert.equal(second, "![Uploading image.png (2)…]()");
64+ assert.notEqual(placeholderFor("image.png", `${first}${second}`), second);
65+});
66+
67+test("an image goes in on a line of its own", () => {
68+ const md = "![x](u)";
69+ assert.deepEqual(insertion("", 0, 0, md), { text: md, start: 0, end: 0, caret: md.length });
70+ assert.deepEqual(insertion("Before after", 6, 7, md), { text: `\n${md}\n`, start: 6, end: 7, caret: 7 + md.length });
71+ assert.deepEqual(insertion("Line\n", 5, 5, md), { text: md, start: 5, end: 5, caret: 5 + md.length });
72+});
73+
74+test("the placeholder is swapped where it is now, keeping what was written around it", () => {
75+ const placeholder = placeholderFor("shot.png", "");
76+ const typedSince = `Before\n${placeholder}\nAfter, written while it uploaded`;
77+ assert.equal(replacePlaceholder(typedSince, placeholder, "![shot.png](https://u/a)"), "Before\n![shot.png](https://u/a)\nAfter, written while it uploaded");
78+ // A failed upload takes the placeholder and its line out.
79+ assert.equal(replacePlaceholder(typedSince, placeholder, ""), "Before\nAfter, written while it uploaded");
80+ assert.equal(replacePlaceholder(`Text\n${placeholder}`, placeholder, ""), "Text");
81+ // Deleted by the person: nothing changes.
82+ assert.equal(replacePlaceholder("They removed it", placeholder, "![x](u)"), "They removed it");
83+ // Two of the same name: only its own.
84+ const second = placeholderFor("shot.png", placeholder);
85+ assert.equal(replacePlaceholder(`${placeholder}\n${second}`, second, "B"), `${placeholder}\nB`);
86+});
87+
88+test("a refusal from the service reads as a sentence", () => {
89+ assert.equal(uploadFailure("a.png", { code: "too_large", message: "An attachment can be at most 10 MB." }), "a.png is over 10 MB, so it wasn't attached.");
90+ assert.equal(uploadFailure("a.png", { code: "forbidden" }), "You can't attach images here, so a.png wasn't attached.");
91+ assert.equal(uploadFailure("a.png", null), "a.png didn't upload. Try again in a moment.");
92+});
93+
94+test("deferred images get their addresses once the work item is open", () => {
95+ const body = `Steps:\n![a.png](${deferredUrl("k1")})\n![b.png](${deferredUrl("k2")})\nEnd`;
96+ assert.deepEqual([...deferredRefs(body)], ["k1", "k2"]);
97+ const resolved = resolveDeferred(body, new Map([["k1", "https://u/attachments/1"], ["k2", null]]));
98+ assert.equal(resolved, "Steps:\n![a.png](https://u/attachments/1)\nEnd");
99+ // A ref it was not given is left alone.
100+ assert.equal(resolveDeferred(`![c](${deferredUrl("k3")})`, new Map()), "![c](attachment:k3)");
101+});
102+
103+test("the caret stays in the person's writing when a placeholder is swapped", () => {
104+ // "ab[PLACEHOLDER]cd": the placeholder is 2..10, and becomes 20 characters.
105+ assert.equal(caretAfter(1, 2, 10, 20), 1);
106+ assert.equal(caretAfter(2, 2, 10, 20), 2);
107+ // Right at its end, where a paste leaves it: after the image, nothing selected.
108+ assert.equal(caretAfter(10, 2, 10, 20), 22);
109+ assert.equal(caretAfter(12, 2, 10, 20), 24);
110+ // Taken out (a failed upload): after it moves back.
111+ assert.equal(caretAfter(12, 2, 10, 0), 4);
112+});
+183−0
1+/**
2+ * Images pasted, dropped or chosen in a comment box, as GitHub does it: an
3+ * `![Uploading name…]()` placeholder goes in at the caret while the image
4+ * uploads, and is swapped for `![name](url)` when it is kept. The work
5+ * service keeps it (`upload_attachment`, crates/contracts/src/attachments.rs)
6+ * and the usercontent origin serves it. This is the part with no DOM, so it
7+ * can be tested under Node: which pastes upload, the placeholder's text and
8+ * where it goes, and what a refusal says.
9+ */
10+
11+/** As `MAX_ATTACHMENT_BYTES` in the contracts: 10 MB. */
12+export const MAX_ATTACHMENT_BYTES = 10 * 1024 * 1024;
13+
14+/** What an attachment may be, as the work service reads it from the bytes. */
15+export const ATTACHMENT_TYPES = ["image/png", "image/jpeg", "image/gif", "image/webp"] as const;
16+
17+/** What the Attach an image control offers to pick. */
18+export const ATTACHMENT_ACCEPT = ATTACHMENT_TYPES.join(",");
19+
20+/** The form field a box that uploads later (a new work item with no repository) sends its images in. */
21+export const DEFERRED_FILE_FIELD = "attachment";
22+/** With each of those, the reference its Markdown holds: `![name](attachment:<ref>)`. */
23+export const DEFERRED_REF_FIELD = "attachmentRef";
24+
25+/** What an attachment is about to be: as much of a `File` as these need. */
26+export type ImageFile = { name: string; type: string; size: number };
27+
28+/** What the clipboard holds, as much of a `DataTransfer` as the choice needs. */
29+export type Clipboard<T extends ImageFile = ImageFile> = {
30+ /** Its files, from its items of kind `file`. */
31+ files: readonly T[];
32+ /** `getData("text/plain")`. */
33+ text: string;
34+};
35+
36+/**
37+ * The images a paste uploads, or none for the browser to paste as it
38+ * always does. Only when the clipboard holds image files and no plain
39+ * text: a screenshot, or an image copied from a page (which comes with
40+ * its HTML but no text). A spreadsheet's cells or a document's paragraphs
41+ * come with a picture of themselves too, and paste as their text.
42+ */
43+export function pastedImages<T extends ImageFile>(clipboard: Clipboard<T>): T[] {
44+ if (clipboard.text.trim() !== "") return [];
45+ return clipboard.files.filter((file) => file.type.startsWith("image/"));
46+}
47+
48+/** Whether something dragged over the box carries files. */
49+export function dragsFiles(types: readonly string[]): boolean {
50+ return types.includes("Files");
51+}
52+
53+/** Why an image will not be attached, before it is sent; null when it may go. */
54+export function refusal(file: ImageFile): string | null {
55+ if (!(ATTACHMENT_TYPES as readonly string[]).includes(file.type.toLowerCase())) {
56+ return `${file.name || "That file"} isn't a PNG, JPEG, GIF or WebP image, so it wasn't attached.`;
57+ }
58+ if (file.size > MAX_ATTACHMENT_BYTES) return `${file.name || "That image"} is over 10 MB, so it wasn't attached.`;
59+ if (file.size === 0) return `${file.name || "That image"} is empty, so it wasn't attached.`;
60+ return null;
61+}
62+
63+const EXTENSIONS: Record<string, string> = { "image/png": "png", "image/jpeg": "jpg", "image/gif": "gif", "image/webp": "webp" };
64+
65+/** A file's name as its alt text: its own, or `image.png` for a pasted one that has none. */
66+export function imageName(file: ImageFile): string {
67+ const own = file.name.split(/[/\\]/).pop()?.replace(/[\u0000-\u001f\u007f]/g, "").trim() ?? "";
68+ return own || `image.${EXTENSIONS[file.type.toLowerCase()] ?? "png"}`;
69+}
70+
71+/** A name as Markdown alt text: its brackets and backslashes escaped, as the contracts' `attachment_markdown`. */
72+export function altText(name: string): string {
73+ return name.replace(/[[\]\\]/g, (c) => `\\${c}`);
74+}
75+
76+/** The Markdown that shows an attachment. */
77+export function imageMarkdown(name: string, url: string): string {
78+ return `![${altText(name)}](${url})`;
79+}
80+
81+/**
82+ * The placeholder for an image while it uploads, one no other in `text`
83+ * is: a second `image.png` is `image.png (2)`, so each is replaced by its
84+ * own upload alone.
85+ */
86+export function placeholderFor(name: string, text: string): string {
87+ for (let n = 1; ; n++) {
88+ const shown = n === 1 ? name : `${name} (${n})`;
89+ const candidate = `![Uploading ${altText(shown)}…]()`;
90+ if (!text.includes(candidate)) return candidate;
91+ }
92+}
93+
94+/** Text put in at `start`..`end`, and where the caret goes after it. */
95+export type Insertion = { text: string; start: number; end: number; caret: number };
96+
97+/**
98+ * How `markdown` goes in at the selection `start`..`end` of `value`: on a
99+ * line of its own, as an image is shown, with a line break added before
100+ * or after it only where the text beside it needs one.
101+ */
102+export function insertion(value: string, start: number, end: number, markdown: string): Insertion {
103+ const before = start > 0 && value[start - 1] !== "\n" ? "\n" : "";
104+ const after = end < value.length && value[end] !== "\n" ? "\n" : "";
105+ const text = `${before}${markdown}${after}`;
106+ return { text, start, end, caret: start + before.length + markdown.length };
107+}
108+
109+/**
110+ * What of `value` to replace to swap `placeholder` for `replacement`,
111+ * found where it is now (the person may have written around it); an
112+ * empty replacement takes the placeholder's line with it when it was
113+ * alone there. Null when it is gone (they deleted it), so nothing they
114+ * wrote is touched.
115+ */
116+export function placeholderRange(value: string, placeholder: string, replacement: string): { start: number; end: number } | null {
117+ let start = value.indexOf(placeholder);
118+ if (start < 0) return null;
119+ let end = start + placeholder.length;
120+ if (replacement === "" && (start === 0 || value[start - 1] === "\n") && value[end] === "\n") end += 1;
121+ else if (replacement === "" && value[start - 1] === "\n" && end === value.length) start -= 1;
122+ return { start, end };
123+}
124+
125+/**
126+ * Where a caret at `at` belongs once `start`..`end` became `length`
127+ * characters: where it was before them, as far along after them, and at
128+ * the end of them from inside (or right at the end of a placeholder, where
129+ * a paste leaves it), so nothing is left selected to type over.
130+ */
131+export function caretAfter(at: number, start: number, end: number, length: number): number {
132+ if (at <= start) return at;
133+ if (at > end) return at + length - (end - start);
134+ return start + length;
135+}
136+
137+/** `value` with `placeholder` replaced by `replacement`, as {@link placeholderRange} finds it. */
138+export function replacePlaceholder(value: string, placeholder: string, replacement: string): string {
139+ const at = placeholderRange(value, placeholder, replacement);
140+ return at ? value.slice(0, at.start) + replacement + value.slice(at.end) : value;
141+}
142+
143+/** A refusal from the upload route (the work service's code and message), as the box says it. */
144+export function uploadFailure(name: string, failure: { code?: string; message?: string } | null): string {
145+ switch (failure?.code) {
146+ case "too_large":
147+ return `${name} is over 10 MB, so it wasn't attached.`;
148+ case "unsupported_type":
149+ return `${name} isn't a PNG, JPEG, GIF or WebP image, so it wasn't attached.`;
150+ case "forbidden":
151+ case "not_found":
152+ case "unauthenticated":
153+ return `You can't attach images here, so ${name} wasn't attached.`;
154+ default:
155+ return failure?.message ? `${name} wasn't attached: ${failure.message}` : `${name} didn't upload. Try again in a moment.`;
156+ }
157+}
158+
159+/**
160+ * The reference a deferred image's Markdown holds until it is uploaded:
161+ * `attachment:<ref>`, a short random word so two never meet.
162+ */
163+export function deferredUrl(ref: string): string {
164+ return `attachment:${ref}`;
165+}
166+
167+/**
168+ * `body` with each deferred image's `attachment:<ref>` swapped for the
169+ * address it was given; one with no address (its upload failed) is taken
170+ * out, image and all.
171+ */
172+export function resolveDeferred(body: string, urls: ReadonlyMap<string, string | null>): string {
173+ return body.replace(/!\[((?:\\.|[^\]\\])*)\]\(attachment:([A-Za-z0-9_-]{1,64})\)\n?/g, (whole, alt: string, ref: string) => {
174+ if (!urls.has(ref)) return whole;
175+ const url = urls.get(ref);
176+ return url ? `![${alt}](${url})${whole.endsWith("\n") ? "\n" : ""}` : "";
177+ });
178+}
179+
180+/** The refs a body still holds, so an image whose Markdown was deleted is not uploaded. */
181+export function deferredRefs(body: string): Set<string> {
182+ return new Set([...body.matchAll(/\]\(attachment:([A-Za-z0-9_-]{1,64})\)/g)].map((match) => match[1]!));
183+}
+2−0
6868 route("-/hovercard/user/:username", "routes/hovercard-user.ts"),
6969 // A person's picture by username, for avatars drawn from a name alone.
7070 route("-/avatar/user/:username", "routes/avatar-user.ts"),
71+ // An image pasted, dropped or chosen in a comment box, kept for its Markdown.
72+ route("-/attachments", "routes/attachments.ts"),
7173 // Live notifications: each tab's feed socket, and the person's
7274 // notification settings as JSON (services/notify).
7375 route("-/live", "routes/notify/live.ts"),
+60−0
1+/**
2+ * An image pasted, dropped or chosen in a comment box (components/attach-images.tsx):
3+ * `POST /-/attachments?repo=<owner>/<name>&name=<file name>`, or
4+ * `?workspace=<slug>&number=<n>` for a work item, with the file as the
5+ * body. The work service checks the viewer may comment there and keeps it
6+ * (`upload_attachment`); the answer is its address on the usercontent
7+ * origin, where it is served, never on the site. An attachment's address is
8+ * 256 random bits, so a private repository's is seen only by whoever is
9+ * given its Markdown.
10+ */
11+import type { Route } from "./+types/attachments";
12+import { addresses } from "../lib/addresses.server";
13+import { MAX_ATTACHMENT_BYTES, imageMarkdown } from "../lib/attachments";
14+import { sniffImage, toBase64 } from "../lib/avatar-upload";
15+import { work } from "../lib/services.server";
16+import { assertSameOrigin, getViewer } from "../lib/session.server";
17+
18+type Answer = { ok: true; url: string; markdown: string; name: string } | { ok: false; error: { code: string; message: string } };
19+
20+const refused = (code: string, message: string, status: number) => Response.json({ ok: false, error: { code, message } } satisfies Answer, { status, headers: { "cache-control": "no-store" } });
21+
22+export async function action({ request, context }: Route.ActionArgs) {
23+ assertSameOrigin(request);
24+ // A fetch, not a page: an answer to say, never the sign-in page.
25+ const user = getViewer(context);
26+ if (!user) return refused("unauthenticated", "Sign in to attach images.", 401);
27+ const url = new URL(request.url);
28+ const repo = (url.searchParams.get("repo") ?? "").split("/");
29+ const workspace = url.searchParams.get("workspace") ?? "";
30+ const number = Number(url.searchParams.get("number"));
31+ const scope =
32+ repo.length === 2 && repo[0] && repo[1]
33+ ? { repo: { namespace: repo[0], name: repo[1] } }
34+ : workspace && Number.isInteger(number) && number > 0
35+ ? { workspace: workspace.toLowerCase(), number }
36+ : null;
37+ if (!scope) return refused("invalid", "Say which repository or work item it is for.", 400);
38+ const length = Number(request.headers.get("content-length") ?? "0");
39+ if (length > MAX_ATTACHMENT_BYTES) return refused("too_large", "An attachment can be at most 10 MB.", 413);
40+ const bytes = new Uint8Array(await request.arrayBuffer());
41+ if (bytes.byteLength === 0) return refused("invalid", "The file is empty.", 400);
42+ if (bytes.byteLength > MAX_ATTACHMENT_BYTES) return refused("too_large", "An attachment can be at most 10 MB.", 413);
43+ // The service reads the type from the bytes again and is the one that decides.
44+ if (!sniffImage(bytes)) return refused("unsupported_type", "An attachment must be a PNG, JPEG, GIF or WebP image.", 415);
45+ const name = url.searchParams.get("name") || "image.png";
46+ try {
47+ const kept = await work.uploadAttachment(user, scope, { name, data: toBase64(bytes) });
48+ if (!kept.ok) {
49+ const status = kept.error.code === "too_large" ? 413 : kept.error.code === "unsupported_type" ? 415 : kept.error.code === "not_found" ? 404 : 403;
50+ return refused(kept.error.code, kept.error.message, status);
51+ }
52+ const at = `${addresses().usercontent}${kept.value.path}`;
53+ return Response.json({ ok: true, url: at, markdown: imageMarkdown(kept.value.name, at), name: kept.value.name } satisfies Answer, {
54+ headers: { "cache-control": "no-store" },
55+ });
56+ } catch (error) {
57+ console.error("attachments: upload", error);
58+ return refused("unavailable", "The image didn't upload. Try again in a moment.", 503);
59+ }
60+}
+8−0
532532 editComment: (actor, scope, commentId, body) =>
533533 call("edit_comment", { actor, ...commentScope(scope), commentId, body }),
534534 deleteComment: (actor, scope, commentId) => call("delete_comment", { actor, ...commentScope(scope), commentId }),
535+ uploadAttachment: (actor, scope, file) =>
536+ call("upload_attachment", {
537+ actor,
538+ ...("repo" in scope ? { repo: scope.repo } : { workspace: scope.workspace, number: scope.number }),
539+ name: file.name,
540+ content_type: file.contentType,
541+ data: file.data,
542+ }),
535543 react: (actor, thread, reaction) => call("react", { actor, ...threadArgs(thread), ...reaction }),
536544 reportChecks: (runId, token, report) =>
537545 call("report_checks", { runId, token, ...report }),
+18−0
923923 /** Where a comment is kept, for changing it: a repository's pull requests, or a workspace's work items. */
924924 export type CommentScope = { repo: RepoPath } | { workspace: string };
925925
926+/**
927+ * Where an image put in Markdown is kept (`upload_attachment`): a
928+ * repository, for its pull requests, commits and the work items filed
929+ * from it, or a work item, which keeps its own (one filed with no
930+ * repository takes images too, once it is open).
931+ */
932+export type AttachmentScope = { repo: RepoPath } | { workspace: string; number: number };
933+
934+/** An image kept for Markdown: `path` is under the usercontent origin (`/attachments/<id>`). */
935+export type Attachment = { id: string; name: string; content_type: string; size: number; path: string };
936+
926937 /** Work items, pull requests, comments and sessions. */
927938 export interface WorkApi extends RulesApi, ChecksApi, CoordinationApi {
928939 /**
10091020 */
10101021 deleteComment(actor: User, scope: CommentScope, commentId: string): Promise<Result<boolean>>;
10111022 /**
1023+ * Keeps an image to show in Markdown there, for anyone who may comment
1024+ * on the repository or take part in the work item. `data` is the file in
1025+ * base64; its type is read from its bytes. PNG, JPEG, GIF or WebP of at
1026+ * most 10 MB: larger is `too_large`, another type `unsupported_type`.
1027+ */
1028+ uploadAttachment(actor: User, scope: AttachmentScope, file: { name: string; contentType?: string; data: string }): Promise<Result<Attachment>>;
1029+ /**
10121030 * Reacts to comment `commentId` on a work item or pull request, or to its
10131031 * description without one; with `remove`, takes it back. Anyone who can
10141032 * read it may. What it has now.

Comments

Sign in to comment.