fix(01-review): WR-07 base64ToBlob empty-input shortcut + SW-side empty-segment filter

This commit is contained in:
2026-05-16 10:24:38 +02:00
parent 349ae88a8e
commit e9aae09f6d
3 changed files with 81 additions and 7 deletions

View File

@@ -153,21 +153,43 @@ async function getVideoBufferFromOffscreen(): Promise<VideoBufferResponse> {
// RESEARCH.md Pattern 3. // RESEARCH.md Pattern 3.
const wireSegments = const wireSegments =
(msg as { segments?: TransferredVideoSegment[] }).segments ?? []; (msg as { segments?: TransferredVideoSegment[] }).segments ?? [];
const segments: VideoSegment[] = []; // WR-07 fix: filter empty wire segments BEFORE base64 decode.
for (const wire of wireSegments) { // An empty wire.data would decode to a zero-byte Blob; the
try { // SW-side mergeVideoSegments would then concat it into the
segments.push({ // output WebM, producing a stray empty EBML segment that
data: base64ToBlob(wire.data, wire.type || VIDEO_MIME_FALLBACK), // breaks Chrome playback. We split into two passes (filter →
timestamp: wire.timestamp, // decode → filter-non-empty) so the iteration semantics stay
// declarative (no early-return in the loop body).
const nonEmptyWires = wireSegments.filter((wire) => {
const isEmpty = !wire.data || wire.data.length === 0;
if (isEmpty) {
logger.warn(
'Skipping empty wire segment (zero-length base64)',
'timestamp:', wire.timestamp,
);
}
return !isEmpty;
}); });
const segments: VideoSegment[] = nonEmptyWires.flatMap((wire) => {
try {
const blob = base64ToBlob(wire.data, wire.type || VIDEO_MIME_FALLBACK);
if (blob.size === 0) {
logger.warn(
'Skipping segment that decoded to zero bytes',
'timestamp:', wire.timestamp,
);
return [];
}
return [{ data: blob, timestamp: wire.timestamp }];
} catch (err) { } catch (err) {
logger.warn( logger.warn(
'base64ToBlob failed; skipping segment', 'base64ToBlob failed; skipping segment',
'timestamp:', wire.timestamp, 'timestamp:', wire.timestamp,
'error:', err, 'error:', err,
); );
return [];
} }
} });
resolve({ segments }); resolve({ segments });
} }
}; };

View File

@@ -65,6 +65,17 @@ export async function blobToBase64(blob: Blob): Promise<string> {
* @returns A Blob whose bytes match the original encoded blob exactly. * @returns A Blob whose bytes match the original encoded blob exactly.
*/ */
export function base64ToBlob(b64: string, mimeType: string): Blob { export function base64ToBlob(b64: string, mimeType: string): Blob {
// WR-07 fix: defensive early-return for empty input. `atob('')` returns
// an empty string and the resulting Blob has size 0 — currently no
// caller filters zero-size segments, which would corrupt the
// concatenated WebM with a stray empty EBML segment. Return an
// explicit empty Blob here so the SW-side filter on
// `segment.data.size > 0` (see src/background/index.ts
// getVideoBufferFromOffscreen) can pre-filter cleanly without an
// extra atob round-trip.
if (b64.length === 0) {
return new Blob([], { type: mimeType });
}
const binary = atob(b64); const binary = atob(b64);
const bytes = new Uint8Array(binary.length); const bytes = new Uint8Array(binary.length);
for (let i = 0; i < binary.length; i++) { for (let i = 0; i < binary.length; i++) {

View File

@@ -193,3 +193,44 @@ describe('port serialization (GREEN — pins the eventual fix contract)', () =>
expect(merged.size).not.toBe(75); // the bug is gone in the fixed form. expect(merged.size).not.toBe(75); // the bug is gone in the fixed form.
}); });
}); });
describe('binary.ts adversarial input (WR-07 + sweep target #6)', () => {
// These tests exercise the PRODUCTION helpers in src/shared/binary.ts,
// not local re-implementations. They pin the WR-07 + sweep-#6
// defensive contracts:
// 1. Empty-string input → zero-byte Blob (early-return shortcut,
// no atob('') round-trip).
// 2. Invalid base64 input → throws DOMException-like InvalidCharacterError,
// caller is expected to wrap in try/catch.
//
// The empty-segment filter in SW's getVideoBufferFromOffscreen relies
// on (1) so the multi-EBML-header concat output never includes a stray
// zero-byte segment. The error-classification step relies on (2) being
// a regular throw so the per-segment try/catch can drop a single bad
// entry without aborting the whole transfer.
it('base64ToBlob("") returns a zero-byte Blob with the requested MIME', async () => {
const { base64ToBlob } = await import('../../src/shared/binary');
const blob = base64ToBlob('', 'video/webm');
expect(blob).toBeInstanceOf(Blob);
expect(blob.size).toBe(0);
expect(blob.type).toBe('video/webm');
});
it('blobToBase64(emptyBlob) round-trips through base64ToBlob to zero bytes', async () => {
const { blobToBase64, base64ToBlob } = await import('../../src/shared/binary');
const original = new Blob([], { type: 'video/webm' });
const b64 = await blobToBase64(original);
expect(b64).toBe('');
const restored = base64ToBlob(b64, 'video/webm');
expect(restored.size).toBe(0);
});
it('base64ToBlob throws on invalid base64 alphabet (caller wraps with try/catch)', async () => {
const { base64ToBlob } = await import('../../src/shared/binary');
// Characters outside the base64 alphabet — atob throws InvalidCharacterError
// synchronously. The SW caller (getVideoBufferFromOffscreen) has a
// try/catch that drops a single bad segment without aborting the batch.
expect(() => base64ToBlob('!!!not-base64!!!', 'video/webm')).toThrow();
});
});