Skip to content

Commit

Permalink
fix: half-written asset zips can be uploaded if process is interrupted
Browse files Browse the repository at this point in the history
Here's a possible scenario that can lead to "Uploaded file must be a
non-empty zip":

- Bundling starts
- A partial zip file is written
- The process is interrupted (`^C` or similar) while writing.
- On next run, the destination file already exists, is taken to be
  complete, and uploaded.

In this fix, we'll write the zip to a temporary file and atomically
rename it into place once finished, thereby avoiding this possible
scenario.

Attempts to fix #18459.
  • Loading branch information
rix0rrr committed Oct 6, 2022
1 parent b342566 commit 904c2a2
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 3 deletions.
53 changes: 51 additions & 2 deletions packages/cdk-assets/lib/private/archive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,17 @@ import * as glob from 'glob';
// eslint-disable-next-line @typescript-eslint/no-require-imports
const archiver = require('archiver');

export function zipDirectory(directory: string, outputFile: string): Promise<void> {
type Logger = (x: string) => void;

export async function zipDirectory(directory: string, outputFile: string, logger: Logger): Promise<void> {
// We write to a temporary file and rename at the last moment. This is so that if we are
// interrupted during this process, we don't leave a half-finished file in the target location.
const temporaryOutputFile = `${outputFile}._tmp`;
await writeZipFile(directory, temporaryOutputFile);
await moveIntoPlace(temporaryOutputFile, outputFile, logger);
}

function writeZipFile(directory: string, outputFile: string): Promise<void> {
return new Promise(async (ok, fail) => {
// The below options are needed to support following symlinks when building zip files:
// - nodir: This will prevent symlinks themselves from being copied into the zip.
Expand Down Expand Up @@ -44,6 +54,45 @@ export function zipDirectory(directory: string, outputFile: string): Promise<voi
}

await archive.finalize();

});
}

/**
* Rename the file to the target location, taking into account that we may see EPERM on Windows
* while an Antivirus scanner still has the file open, so retry a couple of times.
*/
async function moveIntoPlace(source: string, target: string, logger: Logger) {
let delay = 100;
let attempts = 5;
while (true) {
try {
if (await pathExists(target)) {
await fs.unlink(target);
}
await fs.rename(source, target);
} catch (e) {
if (e.code !== 'EPERM' || attempts-- <= 0) {
throw e;
}
logger(e.message);
await sleep(Math.floor(Math.random() * delay));
delay *= 2;
}
}
}

function sleep(ms: number) {
return new Promise(ok => setTimeout(ok, ms));
}

async function pathExists(x: string) {
try {
await fs.stat(x);
return true;
} catch (e) {
if (e.code === 'ENOENT') {
return false;
}
throw e;
}
}
2 changes: 1 addition & 1 deletion packages/cdk-assets/lib/private/handlers/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export class FileAssetHandler implements IAssetHandler {
}

this.host.emitMessage(EventType.BUILD, `Zip ${fullPath} -> ${packagedPath}`);
await zipDirectory(fullPath, packagedPath);
await zipDirectory(fullPath, packagedPath, (m) => this.host.emitMessage(EventType.DEBUG, m));
return { packagedPath, contentType };
} else {
const contentType = mime.getType(fullPath) ?? 'application/octet-stream';
Expand Down

0 comments on commit 904c2a2

Please sign in to comment.