diff --git a/src/main/providers/filesystem-provider-contract.ts b/src/main/providers/filesystem-provider-contract.ts index fb266bf1490..27dc5a6ec33 100644 --- a/src/main/providers/filesystem-provider-contract.ts +++ b/src/main/providers/filesystem-provider-contract.ts @@ -55,7 +55,8 @@ export type IFilesystemProvider = { * * `position` must be a non-negative safe integer and `length` a positive one * no larger than `MAX_FILE_RANGE_READ_BYTES`; anything else throws - * `FileRangeReadRequestError` without reaching the host. */ + * `FileRangeReadRequestError` without reaching the host. The final byte + * offset must also remain a safe integer. */ readFileRange?( filePath: string, position: number, diff --git a/src/main/providers/ssh-filesystem-provider-capabilities.test.ts b/src/main/providers/ssh-filesystem-provider-capabilities.test.ts index 5a34b2a0c1f..66691119419 100644 --- a/src/main/providers/ssh-filesystem-provider-capabilities.test.ts +++ b/src/main/providers/ssh-filesystem-provider-capabilities.test.ts @@ -99,6 +99,31 @@ describe('SSH filesystem capability document', () => { expect(mux.request).toHaveBeenCalledTimes(1) }) + it('retries when the fetch fails after its only waiter aborts', async () => { + let rejectFetch: (error: Error) => void = () => {} + const mux = { + request: vi + .fn() + .mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectFetch = reject + }) + ) + .mockResolvedValue({ rangedReadVersion: 1 }) + } + const controller = new AbortController() + const abandoned = probeSshRangedReadCapability(mux as never, controller.signal) + controller.abort() + await expect(abandoned).rejects.toThrow('client_disconnected') + + rejectFetch(new Error('connection closed')) + await new Promise((resolve) => setImmediate(resolve)) + + await expect(probeSshRangedReadCapability(mux as never)).resolves.toBe(true) + expect(mux.request).toHaveBeenCalledTimes(2) + }) + it('treats a non-object response as no capabilities', async () => { const mux = { request: vi.fn().mockResolvedValue('yes') } await expect(probeSshRangedReadCapability(mux as never)).resolves.toBe(false) diff --git a/src/main/providers/ssh-filesystem-provider-capabilities.ts b/src/main/providers/ssh-filesystem-provider-capabilities.ts index 86451e5228b..2703c4d16c7 100644 --- a/src/main/providers/ssh-filesystem-provider-capabilities.ts +++ b/src/main/providers/ssh-filesystem-provider-capabilities.ts @@ -36,16 +36,13 @@ function readSshFsCapabilities( }) if (!cached) { capabilitiesByMux.set(mux, probe) - } - return waitForSshCapabilityProbe(probe, signal).then( - (capabilities) => capabilities, - (error) => { - if (!signal?.aborted && capabilitiesByMux.get(mux) === probe) { + void probe.catch(() => { + if (capabilitiesByMux.get(mux) === probe) { capabilitiesByMux.delete(mux) } - throw error - } - ) + }) + } + return waitForSshCapabilityProbe(probe, signal) } export function probeSshQuickOpenSearchCapability( diff --git a/src/main/providers/ssh-filesystem-provider-range.test.ts b/src/main/providers/ssh-filesystem-provider-range.test.ts index a947d53845e..3dee006d59d 100644 --- a/src/main/providers/ssh-filesystem-provider-range.test.ts +++ b/src/main/providers/ssh-filesystem-provider-range.test.ts @@ -74,7 +74,8 @@ describe('SshFilesystemProvider.readFileRange', () => { ['a fractional position', 0.5, 4], ['a zero length', 0, 0], ['a negative length', 0, -1], - ['an over-cap length', 0, MAX_FILE_RANGE_READ_BYTES + 1] + ['an over-cap length', 0, MAX_FILE_RANGE_READ_BYTES + 1], + ['a window past safe-integer offsets', Number.MAX_SAFE_INTEGER, 2] ])('rejects %s without a round trip', async (_label, position, length) => { const request = vi.fn() await expect( diff --git a/src/relay/fs-handler-file-range-dispatch.test.ts b/src/relay/fs-handler-file-range-dispatch.test.ts index b408b6360da..3f1de4693a4 100644 --- a/src/relay/fs-handler-file-range-dispatch.test.ts +++ b/src/relay/fs-handler-file-range-dispatch.test.ts @@ -125,10 +125,7 @@ describe('fs.readFileRange over the real dispatcher', () => { 0 ) ) - for (let i = 0; i < 200 && frames.length === 0; i++) { - await new Promise((resolve) => setImmediate(resolve)) - } - expect(frames).toHaveLength(1) + await vi.waitFor(() => expect(frames).toHaveLength(1), { timeout: 4_000 }) const response = decodePayload(frames[0]) as { id: number error?: { code: number } diff --git a/src/relay/fs-handler-file-range.test.ts b/src/relay/fs-handler-file-range.test.ts index 4690b6a14c7..3ecca25ec4c 100644 --- a/src/relay/fs-handler-file-range.test.ts +++ b/src/relay/fs-handler-file-range.test.ts @@ -4,7 +4,11 @@ import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { readRelayFileRange } from './fs-handler-file-range' import { readFullStreamChunk } from './fs-handler-file-read' -import { FileRangeReadRequestError, MAX_FILE_RANGE_READ_BYTES } from '../shared/file-range-read' +import { + FileRangeReadRequestError, + MAX_FILE_RANGE_READ_BYTES, + validateFileRangeRequest +} from '../shared/file-range-read' import { HEADER_LENGTH, prepareJsonRpcPayload } from './protocol' import { DISPATCHER_CONTROL_QUEUE_MAX_BYTES } from './dispatcher-writer-admission' @@ -148,6 +152,7 @@ describe('readRelayFileRange', () => { ['a NaN position', Number.NaN, 4], ['an infinite position', Number.POSITIVE_INFINITY, 4], ['a position past the safe-integer range', Number.MAX_SAFE_INTEGER + 2, 4], + ['a window past the safe-integer range', Number.MAX_SAFE_INTEGER - 1, 3], ['a zero length', 0, 0], ['a negative length', 0, -4], ['a fractional length', 0, 4.5], @@ -204,6 +209,22 @@ describe('readFullStreamChunk', () => { ]) }) + it('keeps every partial-read offset safe at the upper boundary', async () => { + const { position: start, length } = validateFileRangeRequest(Number.MAX_SAFE_INTEGER - 2, 3) + const calls: number[] = [] + const reader = { + read(buffer: Buffer, offset: number, _length: number, position: number) { + calls.push(position) + buffer[offset] = 1 + return Promise.resolve({ bytesRead: 1 }) + } + } + const bytesRead = await readFullStreamChunk(reader, Buffer.alloc(length), length, start) + expect(bytesRead).toBe(length) + expect(calls).toEqual([start, start + 1, Number.MAX_SAFE_INTEGER]) + expect(calls.every(Number.isSafeInteger)).toBe(true) + }) + // Without the zero check the loop would spin forever on a reader at EOF. it('stops at the first empty read and reports only what it filled', async () => { const source = Buffer.from('abcd') diff --git a/src/shared/file-range-read.ts b/src/shared/file-range-read.ts index 6af436fcf48..fe695e9a2c9 100644 --- a/src/shared/file-range-read.ts +++ b/src/shared/file-range-read.ts @@ -54,5 +54,8 @@ export function validateFileRangeRequest( `fs.readFileRange length ${length} exceeds the ${MAX_FILE_RANGE_READ_BYTES}-byte limit` ) } + if (position > Number.MAX_SAFE_INTEGER - (length - 1)) { + throw new FileRangeReadRequestError('fs.readFileRange window exceeds safe-integer offsets') + } return { position, length } }