Skip to content

Makes document attachment usage consider external attachments #1600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
35 changes: 35 additions & 0 deletions app/server/lib/ActiveDoc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1035,6 +1035,8 @@ export class ActiveDoc extends EventEmitter {
unused: 0,
};

const sizesToUpdate = new Map<string, number>();

await unpackTarArchive(tarFile, async (file) => {
try {
const fileIdent = archiveFilePathToAttachmentIdent(file.path);
Expand All @@ -1044,6 +1046,7 @@ export class ActiveDoc extends EventEmitter {
fallbackStoreId,
);
if (isAdded) {
sizesToUpdate.set(fileIdent, file.size);
results.added += 1;
} else {
results.unused += 1;
Expand All @@ -1057,6 +1060,11 @@ export class ActiveDoc extends EventEmitter {
}
});

// This updates _grist_Attachments.fileSize with the size of the uploaded files.
// This prevents a loophole where a user has altered `fileSize`, imported the altered document,
// then restored the originals.
await this._updateAttachmentFileSizesUsingFileIdent(docSession, sizesToUpdate);

return results;
}

Expand Down Expand Up @@ -2590,6 +2598,33 @@ export class ActiveDoc extends EventEmitter {
}];
}

private async _updateAttachmentFileSizesUsingFileIdent(docSession: OptDocSession,
newFileSizesByFileIdent: Map<string, number>): Promise<void> {
const rowIdsToUpdate: number[] = [];
const newFileSizesForRows: CellValue[] = [];
const attachments = this.docData?.getMetaTable("_grist_Attachments").getRecords();
for (const attachmentRec of attachments ?? []) {
const newSize = newFileSizesByFileIdent.get(attachmentRec.fileIdent);
if (newSize && newSize !== attachmentRec.fileSize) {
rowIdsToUpdate.push(attachmentRec.id);
newFileSizesForRows.push(newSize);
}
}

const action: BulkUpdateRecord = ['BulkUpdateRecord', '_grist_Attachments', rowIdsToUpdate, {
fileSize: newFileSizesForRows
}];

await this._applyUserActionsWithExtendedOptions(
docSession,
[action],
{ attachment: true },
);

// Updates doc's overall attachment usage to reflect any changes to file sizes.
await this._updateAttachmentsSize();
}

/**
* If the software is newer than the document, migrate the document by fetching all tables, and
* giving them to the sandbox so that it can produce migration actions.
Expand Down
3 changes: 3 additions & 0 deletions app/server/lib/Archive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ export function create_tar_archive(
export interface UnpackedFile {
path: string;
data: stream.Readable;
size: number;
}

export async function unpackTarArchive(
Expand All @@ -144,6 +145,8 @@ export async function unpackTarArchive(
onFile({
path: header.name,
data: contentStream,
// Realistically this should never be undefined - it's mandatory for files in a .tar archive
size: header.size ?? 0,
})
// No sensible behaviour when an error is thrown by onFile - it's onFile's responsibility
// to handle it.
Expand Down
9 changes: 8 additions & 1 deletion app/server/lib/DocStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1385,7 +1385,14 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage {
-- to make LENGTH() quickly read the stored length instead of actually reading the blob data.
-- We use LENGTH() in the first place instead of _grist_Attachments.fileSize because the latter can
-- be changed by users.
SELECT MAX(LENGTH(files.data)) AS len
-- External attachments have no internal blob data, so we need to use fileSize for those.
-- To avoid tampering with fileSize in offline records, we update it whenever files are
-- uploaded or retrieved.
SELECT
CASE WHEN files.storageId IS NOT NULL
THEN MAX(meta.fileSize)
ELSE MAX(LENGTH(files.data))
END AS len
FROM _gristsys_Files AS files
JOIN _grist_Attachments AS meta
ON meta.fileIdent = files.ident
Expand Down
Loading