Skip to content

Commit d8a89a2

Browse files
authored
fix: upload small checksum assets reliably (#815)
Signed-off-by: Rui Chen <rui@chenrui.dev>
1 parent 45ece40 commit d8a89a2

3 files changed

Lines changed: 352 additions & 18 deletions

File tree

__tests__/upload-stream.test.ts

Lines changed: 320 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,320 @@
1+
import { getOctokit } from '@actions/github';
2+
import { createHash } from 'crypto';
3+
import { createServer, type Server } from 'http';
4+
import { type AddressInfo } from 'net';
5+
import { mkdtemp, open, rm, writeFile } from 'fs/promises';
6+
import { tmpdir } from 'os';
7+
import { join } from 'path';
8+
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
9+
import { GitHubReleaser, upload } from '../src/github';
10+
import type { Config } from '../src/util';
11+
12+
const openFile = vi.hoisted(() => vi.fn());
13+
14+
vi.mock('fs/promises', async (importOriginal) => {
15+
const actual = await importOriginal<typeof import('fs/promises')>();
16+
openFile.mockImplementation(actual.open);
17+
return { ...actual, open: openFile };
18+
});
19+
20+
type Fixture = {
21+
name: string;
22+
path: string;
23+
size: number;
24+
digest: string;
25+
contentType: string;
26+
bytes?: Buffer;
27+
};
28+
29+
type Receipt = {
30+
method: string | undefined;
31+
pathname: string;
32+
filename: string | null;
33+
contentLength: string | undefined;
34+
contentType: string | undefined;
35+
size: number;
36+
digest: string;
37+
chunks: number;
38+
chunkTypes: string[];
39+
bytes?: Buffer;
40+
};
41+
42+
const config: Config = {
43+
github_token: 'not-a-real-token',
44+
github_ref: 'refs/tags/v1.0.0',
45+
github_repository: 'owner/repo',
46+
input_files: [],
47+
input_fail_on_unmatched_files: false,
48+
input_generate_release_notes: false,
49+
input_append_body: false,
50+
input_make_latest: undefined,
51+
};
52+
53+
const sha256 = (data: Buffer): string => createHash('sha256').update(data).digest('hex');
54+
55+
const expectLastFileHandleClosed = async (): Promise<void> => {
56+
const result = openFile.mock.results.at(-1);
57+
expect(result?.type).toBe('return');
58+
const fileHandle = await result?.value;
59+
await expect(fileHandle.stat()).rejects.toMatchObject({ code: 'EBADF' });
60+
};
61+
62+
const closeServer = async (server: Server): Promise<void> => {
63+
server.closeIdleConnections();
64+
server.closeAllConnections();
65+
await new Promise<void>((resolve, reject) => {
66+
server.close((error) => (error ? reject(error) : resolve()));
67+
});
68+
};
69+
70+
const startUploadServer = async (
71+
responseStatus: (requestIndex: number) => number = () => 201,
72+
): Promise<{ server: Server; uploadUrl: string; receipts: Receipt[] }> => {
73+
const receipts: Receipt[] = [];
74+
const server = createServer(async (request, response) => {
75+
const url = new URL(request.url || '/', 'http://127.0.0.1');
76+
const hash = createHash('sha256');
77+
const bufferedChunks: Buffer[] = [];
78+
const chunkTypes: string[] = [];
79+
let size = 0;
80+
let chunks = 0;
81+
82+
for await (const chunk of request) {
83+
chunkTypes.push(chunk?.constructor?.name || typeof chunk);
84+
const bytes = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
85+
chunks += 1;
86+
size += bytes.length;
87+
hash.update(bytes);
88+
if (size <= 1024 * 1024) {
89+
bufferedChunks.push(Buffer.from(bytes));
90+
}
91+
}
92+
93+
receipts.push({
94+
method: request.method,
95+
pathname: url.pathname,
96+
filename: url.searchParams.get('name'),
97+
contentLength: request.headers['content-length'],
98+
contentType: request.headers['content-type'],
99+
size,
100+
digest: hash.digest('hex'),
101+
chunks,
102+
chunkTypes,
103+
bytes: size <= 1024 * 1024 ? Buffer.concat(bufferedChunks) : undefined,
104+
});
105+
106+
const status = responseStatus(receipts.length - 1);
107+
response.writeHead(status, { 'content-type': 'application/json' });
108+
response.end(
109+
JSON.stringify(
110+
status === 201
111+
? { id: 123, name: url.searchParams.get('name') }
112+
: {
113+
message: 'Validation Failed',
114+
errors: [{ code: 'already_exists' }],
115+
},
116+
),
117+
);
118+
});
119+
120+
await new Promise<void>((resolve, reject) => {
121+
server.once('error', reject);
122+
server.listen(0, '127.0.0.1', resolve);
123+
});
124+
const address = server.address() as AddressInfo;
125+
return {
126+
server,
127+
uploadUrl: `http://127.0.0.1:${address.port}/repos/owner/repo/releases/1/assets`,
128+
receipts,
129+
};
130+
};
131+
132+
describe('release asset upload transport', () => {
133+
let tempDirectory: string;
134+
let fixtures: Fixture[];
135+
136+
beforeAll(async () => {
137+
tempDirectory = await mkdtemp(join(tmpdir(), 'action-gh-release-upload-'));
138+
139+
const smallFixtures = [
140+
{
141+
name: 'artifact.zip.sha512',
142+
bytes: Buffer.from(`${'a'.repeat(128)}\n`),
143+
contentType: 'application/octet-stream',
144+
},
145+
{
146+
name: 'artifact.md5',
147+
bytes: Buffer.from(`${'b'.repeat(32)}\n`),
148+
contentType: 'application/octet-stream',
149+
},
150+
{ name: 'one-byte.txt', bytes: Buffer.from([0x7f]), contentType: 'text/plain' },
151+
{ name: 'empty.bin', bytes: Buffer.alloc(0), contentType: 'application/octet-stream' },
152+
{
153+
name: 'binary.bin',
154+
bytes: Buffer.from([0x00, 0x01, 0x7f, 0x80, 0xfe, 0xff]),
155+
contentType: 'application/octet-stream',
156+
},
157+
];
158+
159+
fixtures = [];
160+
for (const fixture of smallFixtures) {
161+
const path = join(tempDirectory, fixture.name);
162+
await writeFile(path, fixture.bytes);
163+
fixtures.push({
164+
...fixture,
165+
path,
166+
size: fixture.bytes.length,
167+
digest: sha256(fixture.bytes),
168+
});
169+
}
170+
171+
const largePath = join(tempDirectory, 'large.bin');
172+
const largeHandle = await open(largePath, 'w');
173+
const largeHash = createHash('sha256');
174+
const block = Buffer.alloc(64 * 1024);
175+
for (let index = 0; index < block.length; index += 1) {
176+
block[index] = index % 251;
177+
}
178+
try {
179+
for (let index = 0; index < 128; index += 1) {
180+
await largeHandle.write(block);
181+
largeHash.update(block);
182+
}
183+
} finally {
184+
await largeHandle.close();
185+
}
186+
fixtures.push({
187+
name: 'large.bin',
188+
path: largePath,
189+
size: 8 * 1024 * 1024,
190+
digest: largeHash.digest('hex'),
191+
contentType: 'application/octet-stream',
192+
});
193+
});
194+
195+
afterAll(async () => {
196+
await rm(tempDirectory, { recursive: true, force: true });
197+
});
198+
199+
it.each([0, 1, 2, 3, 4, 5])(
200+
'uploads fixture %i through the real Octokit request path',
201+
async (fixtureIndex) => {
202+
const fixture = fixtures[fixtureIndex];
203+
const { server, uploadUrl, receipts } = await startUploadServer();
204+
const releaser = new GitHubReleaser(getOctokit(config.github_token));
205+
openFile.mockClear();
206+
207+
try {
208+
await expect(upload(config, releaser, uploadUrl, fixture.path, [])).resolves.toEqual({
209+
id: 123,
210+
name: fixture.name,
211+
});
212+
213+
expect(receipts).toHaveLength(1);
214+
expect(receipts[0]).toMatchObject({
215+
method: 'POST',
216+
pathname: '/repos/owner/repo/releases/1/assets',
217+
filename: fixture.name,
218+
contentLength: String(fixture.size),
219+
contentType: fixture.contentType,
220+
size: fixture.size,
221+
digest: fixture.digest,
222+
});
223+
if (fixture.bytes) {
224+
expect(receipts[0].bytes).toEqual(fixture.bytes);
225+
}
226+
expect(new Set(receipts[0].chunkTypes)).toEqual(
227+
fixture.size === 0 ? new Set() : new Set(['Buffer']),
228+
);
229+
if (fixture.size > 1024 * 1024) {
230+
expect(receipts[0].chunks).toBeGreaterThan(1);
231+
}
232+
await expectLastFileHandleClosed();
233+
} finally {
234+
await closeServer(server);
235+
}
236+
},
237+
);
238+
239+
it('closes the file handle when the request fails', async () => {
240+
const fixture = fixtures[0];
241+
const { server, uploadUrl } = await startUploadServer(() => 500);
242+
const releaser = new GitHubReleaser(getOctokit(config.github_token));
243+
openFile.mockClear();
244+
245+
try {
246+
await expect(upload(config, releaser, uploadUrl, fixture.path, [])).rejects.toThrow(
247+
'Validation Failed',
248+
);
249+
await expectLastFileHandleClosed();
250+
} finally {
251+
await closeServer(server);
252+
}
253+
});
254+
255+
it('normalizes ArrayBuffer chunks before they reach the Octokit transport', async () => {
256+
const fixture = fixtures[0];
257+
const { server, uploadUrl, receipts } = await startUploadServer();
258+
const releaser = new GitHubReleaser(getOctokit(config.github_token));
259+
const actual = await vi.importActual<typeof import('fs/promises')>('fs/promises');
260+
openFile.mockImplementationOnce(async (...args: Parameters<typeof actual.open>) => {
261+
const fileHandle = await actual.open(...args);
262+
const readableWebStream = fileHandle.readableWebStream.bind(fileHandle);
263+
Object.defineProperty(fileHandle, 'readableWebStream', {
264+
configurable: true,
265+
value: () =>
266+
readableWebStream().pipeThrough(
267+
new TransformStream<Uint8Array, ArrayBuffer>({
268+
transform(chunk, controller) {
269+
controller.enqueue(chunk.slice().buffer);
270+
},
271+
}),
272+
),
273+
});
274+
return fileHandle;
275+
});
276+
277+
try {
278+
await expect(upload(config, releaser, uploadUrl, fixture.path, [])).resolves.toEqual({
279+
id: 123,
280+
name: fixture.name,
281+
});
282+
expect(receipts).toHaveLength(1);
283+
expect(receipts[0]).toMatchObject({
284+
size: fixture.size,
285+
digest: fixture.digest,
286+
});
287+
await expectLastFileHandleClosed();
288+
} finally {
289+
await closeServer(server);
290+
}
291+
});
292+
293+
it('opens a fresh upload body after an already-exists response', async () => {
294+
const fixture = fixtures[0];
295+
const { server, uploadUrl, receipts } = await startUploadServer((index) =>
296+
index === 0 ? 422 : 201,
297+
);
298+
const releaser = new GitHubReleaser(getOctokit(config.github_token));
299+
vi.spyOn(releaser, 'listReleaseAssets').mockResolvedValue([{ id: 9, name: fixture.name }]);
300+
vi.spyOn(releaser, 'deleteReleaseAsset').mockResolvedValue(undefined);
301+
openFile.mockClear();
302+
303+
try {
304+
await expect(upload(config, releaser, uploadUrl, fixture.path, [])).resolves.toEqual({
305+
id: 123,
306+
name: fixture.name,
307+
});
308+
expect(receipts).toHaveLength(2);
309+
expect(receipts.map(({ size }) => size)).toEqual([fixture.size, fixture.size]);
310+
expect(receipts.map(({ digest }) => digest)).toEqual([fixture.digest, fixture.digest]);
311+
expect(openFile).toHaveBeenCalledTimes(2);
312+
for (const result of openFile.mock.results) {
313+
const fileHandle = await result.value;
314+
await expect(fileHandle.stat()).rejects.toMatchObject({ code: 'EBADF' });
315+
}
316+
} finally {
317+
await closeServer(server);
318+
}
319+
});
320+
});

dist/index.js

Lines changed: 14 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/github.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,26 @@
11
import { GitHub } from '@actions/github/lib/utils';
22
import { statSync } from 'fs';
3-
import { open } from 'fs/promises';
3+
import { open, type FileHandle } from 'fs/promises';
44
import { lookup } from 'mime-types';
55
import { basename } from 'path';
66
import { alignAssetName, Config, errorMessage, isTag, normalizeTagName, releaseBody } from './util';
77

88
type GitHub = InstanceType<typeof GitHub>;
99

10+
type UploadChunk = ArrayBuffer | Uint8Array<ArrayBufferLike>;
11+
type UploadBody = ReadableStream<Uint8Array<ArrayBufferLike>>;
12+
13+
const fileUploadStream = (fileHandle: FileHandle): UploadBody => {
14+
const source = fileHandle.readableWebStream() as ReadableStream<UploadChunk>;
15+
return source.pipeThrough(
16+
new TransformStream<UploadChunk, Uint8Array<ArrayBufferLike>>({
17+
transform(chunk, controller) {
18+
controller.enqueue(chunk instanceof Uint8Array ? chunk : new Uint8Array(chunk));
19+
},
20+
}),
21+
);
22+
};
23+
1024
export interface ReleaseAsset {
1125
name: string;
1226
mime: string;
@@ -99,7 +113,7 @@ export interface Releaser {
99113
size: number;
100114
mime: string;
101115
token: string;
102-
data: any;
116+
data: UploadBody;
103117
}): Promise<{ status: number; data: any }>;
104118
}
105119

@@ -239,7 +253,7 @@ export class GitHubReleaser implements Releaser {
239253
size: number;
240254
mime: string;
241255
token: string;
242-
data: any;
256+
data: UploadBody;
243257
}): Promise<{ status: number; data: any }> {
244258
return this.github.request({
245259
method: 'POST',
@@ -367,7 +381,7 @@ export const upload = async (
367381
size,
368382
mime,
369383
token: config.github_token,
370-
data: fh.readableWebStream(),
384+
data: fileUploadStream(fh),
371385
});
372386
} finally {
373387
await fh.close();

0 commit comments

Comments
 (0)