Address artifact PR review feedback

This commit is contained in:
Dotta 2026-05-30 20:51:46 +00:00
parent bbf77fcb69
commit 04a19cbc6e
8 changed files with 59 additions and 38 deletions

View file

@ -134,10 +134,14 @@ function createStorageService(body = Buffer.from("test")): TestStorageService {
originalFilename: input.originalFilename,
};
},
getObject: vi.fn(async () => ({
stream: Readable.from(body),
contentLength: body.length,
})),
getObject: vi.fn(async (_companyId, _objectKey, options) => {
const range = options?.range;
const streamBody = range ? body.subarray(range.start, range.end + 1) : body;
return {
stream: Readable.from(streamBody),
contentLength: streamBody.length,
};
}),
headObject: vi.fn(),
deleteObject: vi.fn(),
};
@ -401,6 +405,11 @@ describe("issue attachment routes", () => {
expect(res.headers["content-length"]).toBe("3");
expect(res.headers["content-disposition"]).toBe('inline; filename="clip.mp4"');
expect(Buffer.from(res.body).toString("utf8")).toBe("bcd");
expect(storage.getObject).toHaveBeenCalledWith(
"company-1",
"issues/issue-1/clip.mp4",
{ range: { start: 1, end: 3 } },
);
});
it("forces video downloads when the download path is requested", async () => {

View file

@ -42,6 +42,26 @@ describe("local disk storage provider", () => {
expect(stored.sha256).toHaveLength(64);
});
it("streams only requested byte ranges", async () => {
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-storage-"));
tempRoots.push(root);
const service = createStorageService(createLocalDiskStorageProvider(root));
const stored = await service.putFile({
companyId: "company-1",
namespace: "issues/issue-1",
originalFilename: "demo.mp4",
contentType: "video/mp4",
body: Buffer.from("0123456789", "utf8"),
});
const fetched = await service.getObject("company-1", stored.objectKey, { range: { start: 2, end: 5 } });
const fetchedBody = await readStreamToBuffer(fetched.stream);
expect(fetchedBody.toString("utf8")).toBe("2345");
expect(fetched.contentLength).toBe(4);
});
it("blocks cross-company object access", async () => {
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-storage-"));
tempRoots.push(root);

View file

@ -1,5 +1,4 @@
import { randomUUID } from "node:crypto";
import { Transform } from "node:stream";
import { Router, type Request, type Response } from "express";
import multer from "multer";
import { z } from "zod";
@ -1166,27 +1165,6 @@ export function issueRoutes(
return { kind: "range", start, end: Math.min(end, contentLength - 1) };
}
function createByteRangeStream(start: number, end: number) {
let offset = 0;
return new Transform({
transform(chunk: Buffer | string, _encoding, callback) {
const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
const chunkStart = offset;
const chunkEnd = offset + buffer.length - 1;
offset += buffer.length;
if (chunkEnd < start || chunkStart > end) {
callback();
return;
}
const sliceStart = Math.max(start - chunkStart, 0);
const sliceEnd = Math.min(end - chunkStart + 1, buffer.length);
callback(null, buffer.subarray(sliceStart, sliceEnd));
},
});
}
function parseBooleanQuery(value: unknown) {
return value === true || value === "true" || value === "1";
}
@ -6284,7 +6262,11 @@ export function issueRoutes(
return;
}
const object = await storage.getObject(attachment.companyId, attachment.objectKey);
const object = await storage.getObject(
attachment.companyId,
attachment.objectKey,
range.kind === "range" ? { range: { start: range.start, end: range.end } } : undefined,
);
const responseContentType = normalizeContentType(attachment.contentType || object.contentType);
res.setHeader("Content-Type", responseContentType);
res.setHeader("Cache-Control", "private, max-age=60");
@ -6306,11 +6288,7 @@ export function issueRoutes(
res.status(206);
res.setHeader("Content-Length", String(rangeLength));
res.setHeader("Content-Range", `bytes ${range.start}-${range.end}/${contentLength}`);
const rangeStream = createByteRangeStream(range.start, range.end);
rangeStream.on("error", (err) => {
next(err);
});
object.stream.pipe(rangeStream).pipe(res);
object.stream.pipe(res);
return;
}

View file

@ -57,9 +57,15 @@ export function createLocalDiskStorageProvider(baseDir: string): StorageProvider
if (!stat || !stat.isFile()) {
throw notFound("Object not found");
}
const streamOptions = input.range
? { start: input.range.start, end: input.range.end }
: undefined;
const contentLength = input.range
? input.range.end - input.range.start + 1
: stat.size;
return {
stream: createReadStream(filePath),
contentLength: stat.size,
stream: createReadStream(filePath, streamOptions),
contentLength,
lastModified: stat.mtime,
};
},

View file

@ -99,6 +99,7 @@ export function createS3StorageProvider(config: S3ProviderConfig): StorageProvid
new GetObjectCommand({
Bucket: bucket,
Key: key,
Range: input.range ? `bytes=${input.range.start}-${input.range.end}` : undefined,
}),
);

View file

@ -113,9 +113,9 @@ export function createStorageService(provider: StorageProvider): StorageService
};
},
async getObject(companyId: string, objectKey: string) {
async getObject(companyId: string, objectKey: string, options) {
ensureCompanyPrefix(companyId, objectKey);
return provider.getObject({ objectKey });
return provider.getObject({ objectKey, range: options?.range });
},
async headObject(companyId: string, objectKey: string) {

View file

@ -10,6 +10,10 @@ export interface PutObjectInput {
export interface GetObjectInput {
objectKey: string;
range?: {
start: number;
end: number;
};
}
export interface GetObjectResult {
@ -56,7 +60,7 @@ export interface PutFileResult {
export interface StorageService {
provider: StorageProviderId;
putFile(input: PutFileInput): Promise<PutFileResult>;
getObject(companyId: string, objectKey: string): Promise<GetObjectResult>;
getObject(companyId: string, objectKey: string, options?: Pick<GetObjectInput, "range">): Promise<GetObjectResult>;
headObject(companyId: string, objectKey: string): Promise<HeadObjectResult>;
deleteObject(companyId: string, objectKey: string): Promise<void>;
}