Improve attachment memory safety, downloads, and high-memory alert UX.
All checks were successful
Queue Release Build / prepare (push) Successful in 20s
Deploy Web Apps / deploy (push) Successful in 9m2s
Queue Release Build / build-windows (push) Successful in 28m8s
Queue Release Build / build-linux (push) Successful in 47m26s
Queue Release Build / build-android (push) Successful in 19m52s
Queue Release Build / finalize (push) Successful in 4m42s
All checks were successful
Queue Release Build / prepare (push) Successful in 20s
Deploy Web Apps / deploy (push) Successful in 9m2s
Queue Release Build / build-windows (push) Successful in 28m8s
Queue Release Build / build-linux (push) Successful in 47m26s
Queue Release Build / build-android (push) Successful in 19m52s
Queue Release Build / finalize (push) Successful in 4m42s
Stream large receives to disk with chunk acks to cap renderer RAM, evict off-screen display blobs, and route exports through a disk-aware download service. Fix the high-memory dialog (backdrop dismiss, copy, log actions), allow diagnostics paths in the path jail, and restore persisted image hydration after reload. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,61 @@
|
||||
import {
|
||||
describe,
|
||||
expect,
|
||||
it
|
||||
} from 'vitest';
|
||||
|
||||
import {
|
||||
buildAttachmentDisplayPinKey,
|
||||
canRevokeAttachmentDisplayBlob,
|
||||
shouldRevokeDisplayBlobForAttachment
|
||||
} from './attachment-blob-eviction.rules';
|
||||
|
||||
describe('attachment-blob-eviction rules', () => {
|
||||
it('builds a stable pin key from message and attachment ids', () => {
|
||||
expect(buildAttachmentDisplayPinKey('msg-1', 'att-1')).toBe('msg-1:att-1');
|
||||
});
|
||||
|
||||
it('allows revoking blob urls when a disk path can rehydrate the attachment', () => {
|
||||
expect(canRevokeAttachmentDisplayBlob({
|
||||
objectUrl: 'blob:http://localhost/abc',
|
||||
savedPath: '/appdata/photo.png',
|
||||
receivedBytes: 0,
|
||||
available: true
|
||||
})).toBe(true);
|
||||
});
|
||||
|
||||
it('refuses to revoke blobs that are the only local copy', () => {
|
||||
expect(canRevokeAttachmentDisplayBlob({
|
||||
objectUrl: 'blob:http://localhost/abc',
|
||||
receivedBytes: 0,
|
||||
available: true
|
||||
})).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses to revoke blobs while a download is still in progress', () => {
|
||||
expect(canRevokeAttachmentDisplayBlob({
|
||||
objectUrl: 'blob:http://localhost/abc',
|
||||
savedPath: '/appdata/photo.png',
|
||||
receivedBytes: 1024,
|
||||
available: false
|
||||
})).toBe(false);
|
||||
});
|
||||
|
||||
it('skips revocation for pinned attachments', () => {
|
||||
const attachment = {
|
||||
id: 'att-1',
|
||||
objectUrl: 'blob:http://localhost/abc',
|
||||
savedPath: '/appdata/photo.png',
|
||||
receivedBytes: 0,
|
||||
available: true
|
||||
};
|
||||
|
||||
expect(shouldRevokeDisplayBlobForAttachment(
|
||||
'msg-1',
|
||||
attachment,
|
||||
new Set([buildAttachmentDisplayPinKey('msg-1', 'att-1')])
|
||||
)).toBe(false);
|
||||
|
||||
expect(shouldRevokeDisplayBlobForAttachment('msg-1', attachment, new Set())).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,50 @@
|
||||
import { isBlobObjectUrl } from './attachment-display-url.rules';
|
||||
|
||||
/** Margin around the chat scrollport used to hydrate blobs before they enter view. */
|
||||
export const ATTACHMENT_BLOB_VISIBILITY_ROOT_MARGIN = '200px';
|
||||
|
||||
export interface AttachmentDisplayBlobCandidate {
|
||||
available?: boolean;
|
||||
filePath?: string;
|
||||
objectUrl?: string;
|
||||
receivedBytes?: number;
|
||||
savedPath?: string;
|
||||
}
|
||||
|
||||
export function buildAttachmentDisplayPinKey(messageId: string, attachmentId: string): string {
|
||||
return `${messageId}:${attachmentId}`;
|
||||
}
|
||||
|
||||
export function canRevokeAttachmentDisplayBlob(
|
||||
attachment: AttachmentDisplayBlobCandidate
|
||||
): boolean {
|
||||
if (!attachment.objectUrl || !isBlobObjectUrl(attachment.objectUrl)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!hasNonEmptyString(attachment.savedPath) && !hasNonEmptyString(attachment.filePath)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ((attachment.receivedBytes ?? 0) > 0 && attachment.available !== true) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
export function shouldRevokeDisplayBlobForAttachment(
|
||||
messageId: string,
|
||||
attachment: AttachmentDisplayBlobCandidate & { id: string },
|
||||
pinnedKeys: ReadonlySet<string>
|
||||
): boolean {
|
||||
if (pinnedKeys.has(buildAttachmentDisplayPinKey(messageId, attachment.id))) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return canRevokeAttachmentDisplayBlob(attachment);
|
||||
}
|
||||
|
||||
function hasNonEmptyString(value: string | null | undefined): boolean {
|
||||
return typeof value === 'string' && value.trim().length > 0;
|
||||
}
|
||||
@@ -4,7 +4,10 @@ import {
|
||||
it
|
||||
} from 'vitest';
|
||||
|
||||
import { decodeBase64ToUint8Array } from './attachment-blob.rules';
|
||||
import {
|
||||
base64DecodedByteLength,
|
||||
decodeBase64ToUint8Array
|
||||
} from './attachment-blob.rules';
|
||||
|
||||
describe('attachment blob rules', () => {
|
||||
it('decodes base64 payloads into byte arrays', () => {
|
||||
@@ -16,4 +19,9 @@ describe('attachment blob rules', () => {
|
||||
67
|
||||
]);
|
||||
});
|
||||
|
||||
it('estimates decoded base64 byte length without allocating bytes', () => {
|
||||
expect(base64DecodedByteLength('QUJD')).toBe(3);
|
||||
expect(base64DecodedByteLength('YQ==')).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -29,6 +29,13 @@ export function encodeUint8ArrayToBase64(bytes: Uint8Array): string {
|
||||
return btoa(binary);
|
||||
}
|
||||
|
||||
/** Returns the decoded byte length of a base64 payload without allocating the bytes. */
|
||||
export function base64DecodedByteLength(base64: string): number {
|
||||
const padding = base64.endsWith('==') ? 2 : base64.endsWith('=') ? 1 : 0;
|
||||
|
||||
return Math.max(0, Math.floor((base64.length * 3) / 4) - padding);
|
||||
}
|
||||
|
||||
/** Yield control back to the browser so long attachment hydration cannot freeze Electron. */
|
||||
export function yieldToAttachmentHydrationLoop(): Promise<void> {
|
||||
return new Promise((resolve) => {
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
import {
|
||||
describe,
|
||||
expect,
|
||||
it
|
||||
} from 'vitest';
|
||||
|
||||
import { buildAttachmentChunkAckKey } from './attachment-chunk-ack.rules';
|
||||
|
||||
describe('attachment-chunk-ack rules', () => {
|
||||
it('builds a stable ack key from message, file, and chunk index', () => {
|
||||
expect(buildAttachmentChunkAckKey('msg-1', 'file-1', 42)).toBe('msg-1:file-1:42');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,3 @@
|
||||
export function buildAttachmentChunkAckKey(messageId: string, fileId: string, index: number): string {
|
||||
return `${messageId}:${fileId}:${index}`;
|
||||
}
|
||||
@@ -0,0 +1,44 @@
|
||||
import {
|
||||
describe,
|
||||
expect,
|
||||
it
|
||||
} from 'vitest';
|
||||
|
||||
import {
|
||||
canDownloadAttachment,
|
||||
resolveAttachmentDiskPath
|
||||
} from './attachment-download.rules';
|
||||
|
||||
describe('attachment-download.rules', () => {
|
||||
it('allows download when a completed disk-only attachment has no object URL', () => {
|
||||
expect(canDownloadAttachment({
|
||||
available: true,
|
||||
savedPath: '/appdata/server/room/files/large.bin'
|
||||
})).toBe(true);
|
||||
});
|
||||
|
||||
it('allows download when a blob object URL is available', () => {
|
||||
expect(canDownloadAttachment({
|
||||
available: true,
|
||||
objectUrl: 'blob:http://localhost/abc'
|
||||
})).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects incomplete or empty local copies', () => {
|
||||
expect(canDownloadAttachment({
|
||||
available: false,
|
||||
savedPath: '/appdata/server/room/files/large.bin'
|
||||
})).toBe(false);
|
||||
|
||||
expect(canDownloadAttachment({
|
||||
available: true
|
||||
})).toBe(false);
|
||||
});
|
||||
|
||||
it('prefers savedPath over filePath for disk export', () => {
|
||||
expect(resolveAttachmentDiskPath({
|
||||
savedPath: '/appdata/copy.bin',
|
||||
filePath: '/home/me/original.bin'
|
||||
})).toBe('/appdata/copy.bin');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,25 @@
|
||||
import type { Attachment } from '../models/attachment.model';
|
||||
|
||||
export function canDownloadAttachment(
|
||||
attachment: Pick<Attachment, 'available' | 'objectUrl' | 'savedPath' | 'filePath'>
|
||||
): boolean {
|
||||
if (attachment.available !== true) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return hasNonEmptyString(attachment.objectUrl) ||
|
||||
hasNonEmptyString(attachment.savedPath) ||
|
||||
hasNonEmptyString(attachment.filePath);
|
||||
}
|
||||
|
||||
export function resolveAttachmentDiskPath(
|
||||
attachment: Pick<Attachment, 'savedPath' | 'filePath'>
|
||||
): string | null {
|
||||
const diskPath = attachment.savedPath?.trim() || attachment.filePath?.trim();
|
||||
|
||||
return diskPath || null;
|
||||
}
|
||||
|
||||
function hasNonEmptyString(value: string | null | undefined): boolean {
|
||||
return typeof value === 'string' && value.trim().length > 0;
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
import {
|
||||
dedupeImageAttachmentsForDisplay,
|
||||
hasImageFilename,
|
||||
isAttachmentPendingInlineHydration,
|
||||
isImageAttachment,
|
||||
isInlineDisplayableImage,
|
||||
resolvePublishAttachmentIsImage
|
||||
@@ -38,6 +39,27 @@ describe('attachment-image rules', () => {
|
||||
})).toBe(true);
|
||||
});
|
||||
|
||||
it('detects images waiting for on-demand blob hydration', () => {
|
||||
expect(isAttachmentPendingInlineHydration({
|
||||
id: '1',
|
||||
filename: 'photo.png',
|
||||
mime: 'image/png',
|
||||
isImage: true,
|
||||
available: false,
|
||||
savedPath: '/appdata/photo.png'
|
||||
})).toBe(true);
|
||||
|
||||
expect(isAttachmentPendingInlineHydration({
|
||||
id: '2',
|
||||
filename: 'photo.png',
|
||||
mime: 'image/png',
|
||||
isImage: true,
|
||||
available: true,
|
||||
objectUrl: 'blob:http://localhost/photo',
|
||||
savedPath: '/appdata/photo.png'
|
||||
})).toBe(false);
|
||||
});
|
||||
|
||||
it('dedupes image attachments by filename and prefers displayable copies', () => {
|
||||
const deduped = dedupeImageAttachmentsForDisplay([
|
||||
{
|
||||
|
||||
@@ -22,6 +22,7 @@ export interface ImageAttachmentCandidate {
|
||||
isImage: boolean;
|
||||
mime: string;
|
||||
objectUrl?: string;
|
||||
receivedBytes?: number;
|
||||
savedPath?: string;
|
||||
}
|
||||
|
||||
@@ -50,6 +51,27 @@ export function isInlineDisplayableImage(
|
||||
!needsBlobObjectUrlForInlineDisplay(attachment.objectUrl);
|
||||
}
|
||||
|
||||
export function isAttachmentPendingInlineHydration(
|
||||
attachment: Pick<
|
||||
ImageAttachmentCandidate,
|
||||
'available' | 'filePath' | 'filename' | 'isImage' | 'mime' | 'objectUrl' | 'receivedBytes' | 'savedPath'
|
||||
>
|
||||
): boolean {
|
||||
if (isInlineDisplayableImage(attachment)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!isImageAttachment(attachment)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ((attachment.receivedBytes ?? 0) > 0 && attachment.available !== true) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return !!(attachment.savedPath?.trim() || attachment.filePath?.trim());
|
||||
}
|
||||
|
||||
export function imageAttachmentDisplayRank(
|
||||
attachment: Pick<ImageAttachmentCandidate, 'available' | 'filePath' | 'isImage' | 'objectUrl' | 'savedPath'>
|
||||
): number {
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
import { selectFileRequestPeer } from './attachment-request.rules';
|
||||
|
||||
describe('selectFileRequestPeer', () => {
|
||||
const uploader = 'uploader-peer';
|
||||
const mirror = 'mirror-peer';
|
||||
const other = 'other-peer';
|
||||
|
||||
it('prefers a mirror host over the original uploader when both are available', () => {
|
||||
expect(selectFileRequestPeer({
|
||||
connectedPeers: [
|
||||
uploader,
|
||||
mirror,
|
||||
other
|
||||
],
|
||||
triedPeers: new Set(),
|
||||
announcedHosts: new Set([uploader, mirror]),
|
||||
uploaderPeerId: uploader
|
||||
})).toBe(mirror);
|
||||
});
|
||||
|
||||
it('falls back to the uploader when no mirror hosts are announced', () => {
|
||||
expect(selectFileRequestPeer({
|
||||
connectedPeers: [uploader, other],
|
||||
triedPeers: new Set(),
|
||||
announcedHosts: new Set([uploader]),
|
||||
uploaderPeerId: uploader
|
||||
})).toBe(uploader);
|
||||
});
|
||||
|
||||
it('skips peers that were already tried', () => {
|
||||
expect(selectFileRequestPeer({
|
||||
connectedPeers: [mirror, uploader],
|
||||
triedPeers: new Set([mirror]),
|
||||
announcedHosts: new Set([mirror, uploader]),
|
||||
uploaderPeerId: uploader
|
||||
})).toBe(uploader);
|
||||
});
|
||||
|
||||
it('returns undefined when every connected peer was already tried', () => {
|
||||
expect(selectFileRequestPeer({
|
||||
connectedPeers: [mirror, uploader],
|
||||
triedPeers: new Set([mirror, uploader]),
|
||||
announcedHosts: new Set([mirror, uploader]),
|
||||
uploaderPeerId: uploader
|
||||
})).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,39 @@
|
||||
export interface FileRequestPeerSelectionInput {
|
||||
connectedPeers: string[];
|
||||
triedPeers: ReadonlySet<string>;
|
||||
announcedHosts: ReadonlySet<string>;
|
||||
uploaderPeerId?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Pick the next peer to request a file from. Mirror hosts (peers that announced
|
||||
* they hold the bytes) are preferred over the original uploader so the sharer's
|
||||
* device is not the only upload source.
|
||||
*/
|
||||
export function selectFileRequestPeer(input: FileRequestPeerSelectionInput): string | undefined {
|
||||
const candidates = input.connectedPeers.filter((peerId) => !input.triedPeers.has(peerId));
|
||||
|
||||
if (candidates.length === 0) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const mirrorHosts = candidates.filter(
|
||||
(peerId) => input.announcedHosts.has(peerId) && peerId !== input.uploaderPeerId
|
||||
);
|
||||
|
||||
if (mirrorHosts.length > 0) {
|
||||
return mirrorHosts[0];
|
||||
}
|
||||
|
||||
if (input.uploaderPeerId && candidates.includes(input.uploaderPeerId)) {
|
||||
return input.uploaderPeerId;
|
||||
}
|
||||
|
||||
const announcedCandidate = candidates.find((peerId) => input.announcedHosts.has(peerId));
|
||||
|
||||
if (announcedCandidate) {
|
||||
return announcedCandidate;
|
||||
}
|
||||
|
||||
return candidates[0];
|
||||
}
|
||||
@@ -1,4 +1,5 @@
|
||||
import {
|
||||
canHostAttachment,
|
||||
deviceHasLocalCopy,
|
||||
isSharingFromThisDevice,
|
||||
isUploaderUser
|
||||
@@ -66,4 +67,10 @@ describe('attachment sharing rules', () => {
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('canHostAttachment', () => {
|
||||
it('is true for any device that holds the bytes locally', () => {
|
||||
expect(canHostAttachment({ available: false, savedPath: '/appdata/file.bin' })).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -35,6 +35,13 @@ export function isSharingFromThisDevice(
|
||||
return isUploaderUser(attachment, currentUserId) && deviceHasLocalCopy(attachment);
|
||||
}
|
||||
|
||||
/** True when this device can serve the attachment bytes to other peers. */
|
||||
export function canHostAttachment(
|
||||
attachment: Pick<Attachment, 'available' | 'objectUrl' | 'savedPath' | 'filePath'>
|
||||
): boolean {
|
||||
return deviceHasLocalCopy(attachment);
|
||||
}
|
||||
|
||||
function hasNonEmptyString(value: string | null | undefined): boolean {
|
||||
return typeof value === 'string' && value.trim().length > 0;
|
||||
}
|
||||
|
||||
@@ -58,7 +58,7 @@ describe('attachment logic', () => {
|
||||
}, undefined, true)).toBe(false);
|
||||
});
|
||||
|
||||
it('streams oversized generic files to disk when the store supports it', () => {
|
||||
it('streams any persistable download to disk when the store supports streaming', () => {
|
||||
const capabilities = {
|
||||
canStreamToDisk: true,
|
||||
canPersistSize: (bytes: number) => bytes <= 256 * 1024 * 1024
|
||||
@@ -69,6 +69,18 @@ describe('attachment logic', () => {
|
||||
mime: 'application/zip',
|
||||
filePath: undefined
|
||||
}, capabilities)).toBe(true);
|
||||
|
||||
expect(shouldStreamAttachmentReceiveToDisk({
|
||||
size: 3,
|
||||
mime: 'application/zip',
|
||||
filePath: undefined
|
||||
}, capabilities)).toBe(true);
|
||||
|
||||
expect(shouldStreamAttachmentReceiveToDisk({
|
||||
size: 200 * 1024 * 1024,
|
||||
mime: 'application/zip',
|
||||
filePath: '/home/ludde/archive.zip'
|
||||
}, capabilities)).toBe(true);
|
||||
});
|
||||
|
||||
it('receives browser-sized files in memory when disk streaming is unavailable', () => {
|
||||
|
||||
@@ -67,19 +67,11 @@ export function shouldStreamAttachmentReceiveToDisk(
|
||||
attachment: Pick<Attachment, 'size' | 'mime' | 'filePath'>,
|
||||
capabilities: AttachmentReceiveCapabilities
|
||||
): boolean {
|
||||
if (attachment.filePath?.trim()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!capabilities.canStreamToDisk || !capabilities.canPersistSize(attachment.size)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (attachment.size > MAX_AUTO_SAVE_SIZE_BYTES) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return isAttachmentMedia(attachment);
|
||||
return true;
|
||||
}
|
||||
|
||||
export function canReceiveAttachmentInMemory(
|
||||
|
||||
@@ -17,6 +17,14 @@ export type FileChunkEvent = ChatEvent & {
|
||||
fromPeerId?: string;
|
||||
};
|
||||
|
||||
export type FileChunkAckEvent = ChatEvent & {
|
||||
type: 'file-chunk-ack';
|
||||
messageId: string;
|
||||
fileId: string;
|
||||
index: number;
|
||||
fromPeerId?: string;
|
||||
};
|
||||
|
||||
export type FileRequestEvent = ChatEvent & {
|
||||
type: 'file-request';
|
||||
messageId: string;
|
||||
@@ -37,7 +45,7 @@ export type FileNotFoundEvent = ChatEvent & {
|
||||
fileId: string;
|
||||
};
|
||||
|
||||
export type FileAnnouncePayload = Pick<ChatEvent, 'messageId' | 'file'>;
|
||||
export type FileAnnouncePayload = Pick<ChatEvent, 'messageId' | 'file' | 'fromPeerId'>;
|
||||
|
||||
export interface FileChunkPayload {
|
||||
messageId?: string;
|
||||
@@ -48,6 +56,13 @@ export interface FileChunkPayload {
|
||||
data?: ChatEvent['data'];
|
||||
}
|
||||
|
||||
export interface FileChunkAckPayload {
|
||||
messageId?: string;
|
||||
fileId?: string;
|
||||
fromPeerId?: string;
|
||||
index?: number;
|
||||
}
|
||||
|
||||
export type FileRequestPayload = Pick<ChatEvent, 'messageId' | 'fileId' | 'fromPeerId'>;
|
||||
export type FileCancelPayload = Pick<ChatEvent, 'messageId' | 'fileId' | 'fromPeerId'>;
|
||||
export type FileNotFoundPayload = Pick<ChatEvent, 'messageId' | 'fileId'>;
|
||||
|
||||
Reference in New Issue
Block a user