fix(extensions): resolve GitHub API 415 error for source tarballs (#13319)

This commit is contained in:
Justin Poehnelt
2025-12-04 10:16:08 -07:00
committed by GitHub
parent 8b0a8f47c1
commit 205d0f456e
2 changed files with 276 additions and 9 deletions

View File

@@ -285,6 +285,131 @@ describe('github.ts', () => {
expect(result.failureReason).toBe('failed to fetch release data');
}
});
it('should use correct headers for release assets', async () => {
vi.mocked(fetchJson).mockResolvedValue({
tag_name: 'v1.0.0',
assets: [{ name: 'asset.tar.gz', url: 'http://asset.url' }],
});
vi.mocked(os.platform).mockReturnValue('linux');
vi.mocked(os.arch).mockReturnValue('x64');
// Mock https.get and fs.createWriteStream for downloadFile
const mockReq = new EventEmitter();
const mockRes =
new EventEmitter() as unknown as import('node:http').IncomingMessage;
Object.assign(mockRes, { statusCode: 200, pipe: vi.fn() });
vi.mocked(https.get).mockImplementation((url, options, cb) => {
if (typeof options === 'function') {
cb = options;
}
if (cb) cb(mockRes);
return mockReq as unknown as import('node:http').ClientRequest;
});
const mockStream = new EventEmitter() as unknown as fs.WriteStream;
Object.assign(mockStream, { close: vi.fn((cb) => cb && cb()) });
vi.mocked(fs.createWriteStream).mockReturnValue(mockStream);
// Mock fs.promises.readdir to return empty array (no cleanup needed)
vi.mocked(fs.promises.readdir).mockResolvedValue([]);
// Mock fs.promises.unlink
vi.mocked(fs.promises.unlink).mockResolvedValue(undefined);
const promise = downloadFromGitHubRelease(
{
type: 'github-release',
source: 'owner/repo',
ref: 'v1.0.0',
} as unknown as ExtensionInstallMetadata,
'/dest',
{ owner: 'owner', repo: 'repo' },
);
// Wait for downloadFile to be called and stream to be created
await vi.waitUntil(
() => vi.mocked(fs.createWriteStream).mock.calls.length > 0,
);
// Trigger stream events to complete download
mockRes.emit('end');
mockStream.emit('finish');
await promise;
expect(https.get).toHaveBeenCalledWith(
'http://asset.url',
expect.objectContaining({
headers: expect.objectContaining({
Accept: 'application/octet-stream',
}),
}),
expect.anything(),
);
});
it('should use correct headers for source tarballs', async () => {
vi.mocked(fetchJson).mockResolvedValue({
tag_name: 'v1.0.0',
assets: [],
tarball_url: 'http://tarball.url',
});
// Mock https.get and fs.createWriteStream for downloadFile
const mockReq = new EventEmitter();
const mockRes =
new EventEmitter() as unknown as import('node:http').IncomingMessage;
Object.assign(mockRes, { statusCode: 200, pipe: vi.fn() });
vi.mocked(https.get).mockImplementation((url, options, cb) => {
if (typeof options === 'function') {
cb = options;
}
if (cb) cb(mockRes);
return mockReq as unknown as import('node:http').ClientRequest;
});
const mockStream = new EventEmitter() as unknown as fs.WriteStream;
Object.assign(mockStream, { close: vi.fn((cb) => cb && cb()) });
vi.mocked(fs.createWriteStream).mockReturnValue(mockStream);
// Mock fs.promises.readdir to return empty array
vi.mocked(fs.promises.readdir).mockResolvedValue([]);
// Mock fs.promises.unlink
vi.mocked(fs.promises.unlink).mockResolvedValue(undefined);
const promise = downloadFromGitHubRelease(
{
type: 'github-release',
source: 'owner/repo',
ref: 'v1.0.0',
} as unknown as ExtensionInstallMetadata,
'/dest',
{ owner: 'owner', repo: 'repo' },
);
// Wait for downloadFile to be called and stream to be created
await vi.waitUntil(
() => vi.mocked(fs.createWriteStream).mock.calls.length > 0,
);
// Trigger stream events to complete download
mockRes.emit('end');
mockStream.emit('finish');
await promise;
expect(https.get).toHaveBeenCalledWith(
'http://tarball.url',
expect.objectContaining({
headers: expect.objectContaining({
Accept: 'application/vnd.github+json',
}),
}),
expect.anything(),
);
});
});
describe('findReleaseAsset', () => {
@@ -349,6 +474,120 @@ describe('github.ts', () => {
'Request failed with status code 404',
);
});
it('should follow redirects', async () => {
const mockReq = new EventEmitter();
const mockResRedirect =
new EventEmitter() as unknown as import('node:http').IncomingMessage;
Object.assign(mockResRedirect, {
statusCode: 302,
headers: { location: 'new-url' },
});
const mockResSuccess =
new EventEmitter() as unknown as import('node:http').IncomingMessage;
Object.assign(mockResSuccess, { statusCode: 200, pipe: vi.fn() });
vi.mocked(https.get)
.mockImplementationOnce((url, options, cb) => {
if (typeof options === 'function') cb = options;
if (cb) cb(mockResRedirect);
return mockReq as unknown as import('node:http').ClientRequest;
})
.mockImplementationOnce((url, options, cb) => {
if (typeof options === 'function') cb = options;
if (cb) cb(mockResSuccess);
return mockReq as unknown as import('node:http').ClientRequest;
});
const mockStream = new EventEmitter() as unknown as fs.WriteStream;
Object.assign(mockStream, { close: vi.fn((cb) => cb && cb()) });
vi.mocked(fs.createWriteStream).mockReturnValue(mockStream);
const promise = downloadFile('url', '/dest');
mockResSuccess.emit('end');
mockStream.emit('finish');
await expect(promise).resolves.toBeUndefined();
expect(https.get).toHaveBeenCalledTimes(2);
expect(https.get).toHaveBeenLastCalledWith(
'new-url',
expect.anything(),
expect.anything(),
);
});
it('should fail after too many redirects', async () => {
const mockReq = new EventEmitter();
const mockResRedirect =
new EventEmitter() as unknown as import('node:http').IncomingMessage;
Object.assign(mockResRedirect, {
statusCode: 302,
headers: { location: 'new-url' },
});
vi.mocked(https.get).mockImplementation((url, options, cb) => {
if (typeof options === 'function') cb = options;
if (cb) cb(mockResRedirect);
return mockReq as unknown as import('node:http').ClientRequest;
});
await expect(downloadFile('url', '/dest')).rejects.toThrow(
'Too many redirects',
);
}, 10000); // Increase timeout for this test if needed, though with mocks it should be fast
it('should fail if redirect location is missing', async () => {
const mockReq = new EventEmitter();
const mockResRedirect =
new EventEmitter() as unknown as import('node:http').IncomingMessage;
Object.assign(mockResRedirect, {
statusCode: 302,
headers: {}, // No location
});
vi.mocked(https.get).mockImplementation((url, options, cb) => {
if (typeof options === 'function') cb = options;
if (cb) cb(mockResRedirect);
return mockReq as unknown as import('node:http').ClientRequest;
});
await expect(downloadFile('url', '/dest')).rejects.toThrow(
'Redirect response missing Location header',
);
});
it('should pass custom headers', async () => {
const mockReq = new EventEmitter();
const mockRes =
new EventEmitter() as unknown as import('node:http').IncomingMessage;
Object.assign(mockRes, { statusCode: 200, pipe: vi.fn() });
vi.mocked(https.get).mockImplementation((url, options, cb) => {
if (typeof options === 'function') cb = options;
if (cb) cb(mockRes);
return mockReq as unknown as import('node:http').ClientRequest;
});
const mockStream = new EventEmitter() as unknown as fs.WriteStream;
Object.assign(mockStream, { close: vi.fn((cb) => cb && cb()) });
vi.mocked(fs.createWriteStream).mockReturnValue(mockStream);
const promise = downloadFile('url', '/dest', {
headers: { 'X-Custom': 'value' },
});
mockRes.emit('end');
mockStream.emit('finish');
await expect(promise).resolves.toBeUndefined();
expect(https.get).toHaveBeenCalledWith(
'url',
expect.objectContaining({
headers: expect.objectContaining({ 'X-Custom': 'value' }),
}),
expect.anything(),
);
});
});
describe('extractFile', () => {

View File

@@ -355,7 +355,17 @@ export async function downloadFromGitHubRelease(
}
try {
await downloadFile(archiveUrl, downloadedAssetPath);
// GitHub API requires different Accept headers for different types of downloads:
// 1. Binary Assets (e.g. release artifacts): Require 'application/octet-stream' to return the raw content.
// 2. Source Tarballs (e.g. /tarball/{ref}): Require 'application/vnd.github+json' (or similar) to return
// a 302 Redirect to the actual download location (codeload.github.com).
// Sending 'application/octet-stream' for tarballs results in a 415 Unsupported Media Type error.
const headers = {
...(asset
? { Accept: 'application/octet-stream' }
: { Accept: 'application/vnd.github+json' }),
};
await downloadFile(archiveUrl, downloadedAssetPath, { headers });
} catch (error) {
return {
failureReason: 'failed to download asset',
@@ -472,24 +482,42 @@ export function findReleaseAsset(assets: Asset[]): Asset | undefined {
return undefined;
}
export async function downloadFile(url: string, dest: string): Promise<void> {
const headers: {
'User-agent': string;
Accept: string;
Authorization?: string;
} = {
export interface DownloadOptions {
headers?: Record<string, string>;
}
export async function downloadFile(
url: string,
dest: string,
options?: DownloadOptions,
redirectCount: number = 0,
): Promise<void> {
const headers: Record<string, string> = {
'User-agent': 'gemini-cli',
Accept: 'application/octet-stream',
...options?.headers,
};
const token = getGitHubToken();
if (token) {
headers.Authorization = `token ${token}`;
headers['Authorization'] = `token ${token}`;
}
return new Promise((resolve, reject) => {
https
.get(url, { headers }, (res) => {
if (res.statusCode === 302 || res.statusCode === 301) {
downloadFile(res.headers.location!, dest).then(resolve).catch(reject);
if (redirectCount >= 10) {
return reject(new Error('Too many redirects'));
}
if (!res.headers.location) {
return reject(
new Error('Redirect response missing Location header'),
);
}
downloadFile(res.headers.location, dest, options, redirectCount + 1)
.then(resolve)
.catch(reject);
return;
}
if (res.statusCode !== 200) {