Skip to content

Commit

Permalink
Fixes an import error when using S3/Minio with no redis
Browse files Browse the repository at this point in the history
Error is caused due to these steps:
- File is uploaded to Home server and attempts to import
- Import ends up in `claimDocument` in `HostedStorageManager`
- Tries to read doc metadata from DocWorkerMap, gets 'unknown' as md5 hash
- Thinks local doc is out of date and erases it.
- Downloads a non-existent file from S3, so import fails as it has no data.

This fixes it by checking for DummyDocWorker's special 'unknown' MD5, forcing an S3 check.
  • Loading branch information
Spoffy committed Sep 24, 2024
1 parent ef6a04a commit 5680d1f
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 1 deletion.
2 changes: 2 additions & 0 deletions app/gen-server/lib/DocWorkerMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,10 @@ let dummyDocWorkerMap: DummyDocWorkerMap|null = null;

export function getDocWorkerMap(): IDocWorkerMap {
if (process.env.REDIS_URL) {
log.info("Creating Redis-based DocWorker");
return new DocWorkerMap();
} else {
log.info("Creating local/dummy DocWorker");
dummyDocWorkerMap = dummyDocWorkerMap || new DummyDocWorkerMap();
return dummyDocWorkerMap;
}
Expand Down
2 changes: 2 additions & 0 deletions app/server/lib/DocManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ export class DocManager extends EventEmitter {
throw new Error('Grist docs must be uploaded individually');
}
const first = uploadInfo.files[0].origName;
log.debug(`DocManager._doImportDoc: Received doc with name ${first}`);
const ext = extname(first);
const basename = path.basename(first, ext).trim() || "Untitled upload";
let id: string;
Expand All @@ -674,6 +675,7 @@ export class DocManager extends EventEmitter {
}
await options.register?.(id, basename);
if (ext === '.grist') {
log.debug(`DocManager._doImportDoc: Importing .grist doc`);
// If the import is a grist file, copy it to the docs directory.
// TODO: We should be skeptical of the upload file to close a possible
// security vulnerability. See https://phab.getgrist.com/T457.
Expand Down
2 changes: 1 addition & 1 deletion app/server/lib/HostedStorageManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ export class HostedStorageManager implements IDocStorageManager {

const existsLocally = await fse.pathExists(this.getPath(docName));
if (existsLocally) {
if (!docStatus.docMD5 || docStatus.docMD5 === DELETED_TOKEN) {
if (!docStatus.docMD5 || docStatus.docMD5 === DELETED_TOKEN || docStatus.docMD5 === 'unknown') {
// New doc appears to already exist, but may not exist in S3.
// Let's check.
const head = await this._ext.head(docName);
Expand Down

0 comments on commit 5680d1f

Please sign in to comment.