Skip to content

Commit 6b64660

Browse files
anthonykim1anthonykim1
committed
fix chunked transfer without repeated i = key
1 parent 10182cc commit 6b64660

2 files changed

Lines changed: 129 additions & 7 deletions

File tree

‎addons/addon-image/src/kitty/KittyGraphicsHandler.ts‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
6363
// Storage related states
6464

6565
private _pendingTransmissions: Map<number, IPendingTransmission> = new Map();
66+
/**
67+
* Tracks the pending key of the most recently started chunked upload.
68+
* Per spec, subsequent chunks only need m= (and optionally q=), without i=.
69+
* When a chunk arrives with no i=, this key is used to find the pending upload.
70+
*/
71+
private _lastPendingKey: number | undefined;
6672
private _nextImageId = 1;
6773
/** Maps Kitty protocol image ID → ImageStorage internal ID for deletion/lookup. */
6874
private _kittyIdToStorageId: Map<number, number> = new Map();
@@ -91,6 +97,7 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
9197
pending.decoder.release();
9298
}
9399
this._pendingTransmissions.clear();
100+
this._lastPendingKey = undefined;
94101
if (this._activeDecoder) {
95102
this._activeDecoder.release();
96103
this._activeDecoder = null;
@@ -168,8 +175,9 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
168175
if (this._aborted) return;
169176

170177
// Check size limit (compare encoded bytes against pre-calculated limit)
171-
// Include cumulative size from pending transmission for multi-chunk images
172-
const pendingKey = this._parsedCommand?.id ?? 0;
178+
// Include cumulative size from pending transmission for multi-chunk images.
179+
// Per spec, subsequent chunks may omit i=, so fall back to _lastPendingKey.
180+
const pendingKey = this._parsedCommand?.id ?? this._lastPendingKey ?? 0;
173181
const pending = this._pendingTransmissions.get(pendingKey);
174182
const previousEncodedSize = pending?.totalEncodedSize ?? 0;
175183
this._totalEncodedSize += end - start;
@@ -182,6 +190,9 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
182190
this._activeDecoder = null;
183191
if (pending) {
184192
this._pendingTransmissions.delete(pendingKey);
193+
if (this._lastPendingKey === pendingKey) {
194+
this._lastPendingKey = undefined;
195+
}
185196
}
186197
this._aborted = true;
187198
return;
@@ -203,6 +214,9 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
203214
this._decodeError = true;
204215
if (pending) {
205216
this._pendingTransmissions.delete(pendingKey);
217+
if (this._lastPendingKey === pendingKey) {
218+
this._lastPendingKey = undefined;
219+
}
206220
}
207221
}
208222
}
@@ -229,7 +243,8 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
229243
return this._handleDelete(cmd);
230244
}
231245

232-
const pendingKey = cmd.id ?? 0;
246+
// Per spec, subsequent chunks may omit i=, so fall back to _lastPendingKey.
247+
const pendingKey = cmd.id ?? this._lastPendingKey ?? 0;
233248
const isMoreComing = cmd.more === 1;
234249
const pending = this._pendingTransmissions.get(pendingKey);
235250

@@ -246,11 +261,17 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
246261
decodeError: this._decodeError
247262
});
248263
}
264+
this._lastPendingKey = pendingKey;
249265
this._activeDecoder = null;
250266
}
251267
return true;
252268
}
253269

270+
// Final chunk received — clear the last pending key
271+
if (pending) {
272+
this._lastPendingKey = undefined;
273+
}
274+
254275
let decodeError = this._decodeError;
255276
let finalCmd = cmd;
256277
let decoder = this._activeDecoder;
@@ -486,6 +507,7 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
486507
pending.decoder.release();
487508
}
488509
this._pendingTransmissions.clear();
510+
this._lastPendingKey = undefined;
489511
this._deleteAll();
490512
break;
491513
case 'i':
@@ -496,6 +518,9 @@ export class KittyGraphicsHandler implements IApcHandler, IResetHandler {
496518
if (pending) {
497519
pending.decoder.release();
498520
this._pendingTransmissions.delete(cmd.id);
521+
if (this._lastPendingKey === cmd.id) {
522+
this._lastPendingKey = undefined;
523+
}
499524
}
500525
this._deleteById(cmd.id);
501526
}

‎addons/addon-image/test/KittyGraphics.test.ts‎

Lines changed: 101 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,107 @@ test.describe('Kitty Graphics Protocol', () => {
225225
(window as any).smallLimitAddon.dispose();
226226
});
227227
});
228+
229+
test('chunked a=T works when subsequent chunks omit i= (spec pattern)', async () => {
230+
const half = Math.floor(KITTY_BLACK_1X1_BASE64.length / 2);
231+
const part1 = KITTY_BLACK_1X1_BASE64.substring(0, half);
232+
const part2 = KITTY_BLACK_1X1_BASE64.substring(half);
233+
234+
await ctx.proxy.write(`\x1b_Ga=T,f=100,i=400,m=1;${part1}\x1b\\`);
235+
await timeout(50);
236+
strictEqual(await getImageStorageLength(), 0);
237+
238+
await ctx.proxy.write(`\x1b_Gm=0;${part2}\x1b\\`);
239+
await timeout(100);
240+
strictEqual(await getImageStorageLength(), 1);
241+
});
242+
243+
test('chunked a=t works when subsequent chunks omit i= (spec pattern)', async () => {
244+
const half = Math.floor(KITTY_BLACK_1X1_BASE64.length / 2);
245+
const part1 = KITTY_BLACK_1X1_BASE64.substring(0, half);
246+
const part2 = KITTY_BLACK_1X1_BASE64.substring(half);
247+
248+
await ctx.proxy.write(`\x1b_Ga=t,f=100,i=401,m=1;${part1}\x1b\\`);
249+
await ctx.proxy.write(`\x1b_Gm=0;${part2}\x1b\\`);
250+
await timeout(100);
251+
252+
strictEqual(await ctx.page.evaluate(`window.imageAddon._handlers.get('kitty').images.has(401)`), true);
253+
});
254+
255+
test('chunked data without i= on subsequent chunks is assembled correctly', async () => {
256+
const half = Math.floor(KITTY_BLACK_1X1_BASE64.length / 2);
257+
const part1 = KITTY_BLACK_1X1_BASE64.substring(0, half);
258+
const part2 = KITTY_BLACK_1X1_BASE64.substring(half);
259+
260+
await ctx.proxy.write(`\x1b_Ga=t,f=100,i=402,m=1;${part1}\x1b\\`);
261+
await ctx.proxy.write(`\x1b_Gm=0;${part2}\x1b\\`);
262+
await timeout(100);
263+
264+
const storedData = await ctx.page.evaluate(async () => {
265+
const blob = (window as any).imageAddon._handlers.get('kitty').images.get(402).data;
266+
const buffer = await blob.arrayBuffer();
267+
return Array.from(new Uint8Array(buffer));
268+
});
269+
deepStrictEqual(storedData, KITTY_BLACK_1X1_BYTES);
270+
});
271+
272+
test('three-chunk transfer with only m= on middle and last chunks', async () => {
273+
const third = Math.floor(KITTY_BLACK_1X1_BASE64.length / 3);
274+
const part1 = KITTY_BLACK_1X1_BASE64.substring(0, third);
275+
const part2End = third + Math.floor((KITTY_BLACK_1X1_BASE64.length - third) / 2);
276+
const alignedPart2End = part2End - (part2End - third) % 4 + third;
277+
const part2 = KITTY_BLACK_1X1_BASE64.substring(third, alignedPart2End);
278+
const part3 = KITTY_BLACK_1X1_BASE64.substring(alignedPart2End);
279+
280+
await ctx.proxy.write(`\x1b_Ga=t,f=100,i=403,m=1;${part1}\x1b\\`);
281+
await ctx.proxy.write(`\x1b_Gm=1;${part2}\x1b\\`);
282+
await ctx.proxy.write(`\x1b_Gm=0;${part3}\x1b\\`);
283+
await timeout(100);
284+
285+
const storedData = await ctx.page.evaluate(async () => {
286+
const blob = (window as any).imageAddon._handlers.get('kitty').images.get(403).data;
287+
const buffer = await blob.arrayBuffer();
288+
return Array.from(new Uint8Array(buffer));
289+
});
290+
deepStrictEqual(storedData, KITTY_BLACK_1X1_BYTES);
291+
});
292+
293+
test('chunked a=T without i= on any chunk works (no response)', async () => {
294+
const half = Math.floor(KITTY_BLACK_1X1_BASE64.length / 2);
295+
const part1 = KITTY_BLACK_1X1_BASE64.substring(0, half);
296+
const part2 = KITTY_BLACK_1X1_BASE64.substring(half);
297+
298+
await ctx.proxy.write(`\x1b_Ga=T,f=100,m=1;${part1}\x1b\\`);
299+
await timeout(50);
300+
strictEqual(await getImageStorageLength(), 0);
301+
302+
await ctx.proxy.write(`\x1b_Gm=0;${part2}\x1b\\`);
303+
await timeout(100);
304+
strictEqual(await getImageStorageLength(), 1);
305+
});
306+
307+
test('chunked transfer responds OK on final chunk when i= on first only', async () => {
308+
await ctx.page.evaluate(() => {
309+
(window as any).kittyResponse = '';
310+
(window as any).term.onData((data: string) => { (window as any).kittyResponse = data; });
311+
});
312+
313+
const half = Math.floor(KITTY_BLACK_1X1_BASE64.length / 2);
314+
const part1 = KITTY_BLACK_1X1_BASE64.substring(0, half);
315+
const part2 = KITTY_BLACK_1X1_BASE64.substring(half);
316+
317+
await ctx.proxy.write(`\x1b_Ga=T,f=100,i=405,m=1;${part1}\x1b\\`);
318+
await timeout(50);
319+
320+
let response: string = await ctx.page.evaluate('window.kittyResponse');
321+
strictEqual(response, '');
322+
323+
await ctx.proxy.write(`\x1b_Gm=0;${part2}\x1b\\`);
324+
await timeout(100);
325+
326+
response = await ctx.page.evaluate('window.kittyResponse');
327+
strictEqual(response, '\x1b_Gi=405;OK\x1b\\');
328+
});
228329
});
229330

230331
test.describe('Delete commands', () => {
@@ -749,8 +850,6 @@ test.describe('Kitty Graphics Protocol', () => {
749850
strictEqual(response, '\x1b_Gi=204;OK\x1b\\');
750851
});
751852

752-
// TODO: When file-based transmission mediums (t=f, t=t, t=s) are supported,
753-
// update these tests to verify successful transmission instead of EINVAL.
754853
test('transmit rejects t=f with id (EINVAL response)', async () => {
755854
await ctx.page.evaluate(() => {
756855
(window as any).kittyResponse = '';
@@ -803,8 +902,6 @@ test.describe('Kitty Graphics Protocol', () => {
803902
strictEqual(response, '');
804903
});
805904

806-
// TODO: When file-based transmission mediums (t=f, t=t, t=s) are supported,
807-
// update these tests to verify successful transmit+display instead of EINVAL.
808905
test('transmit+display rejects t=f with id (EINVAL response)', async () => {
809906
await ctx.page.evaluate(() => {
810907
(window as any).kittyResponse = '';

0 commit comments

Comments
 (0)