Skip to content

Commit

Permalink
Address review comments for manangeLinesInFile
Browse files Browse the repository at this point in the history
- Move the `COPYFILE_EXCL` constants into the call site, so it is more
  obvious that we would throw if the files already exist.
- When reading existing contents, read from the temporary or backup files
  we created, to avoid issues where a second process may modify the file
  away from what we expect.  This still leaves a window where they could
  modify the file just before we copy, but in that case there isn't any
  reasonable expectation that we could deal with it.
- Update an error message to be clearer.

Signed-off-by: Mark Yen <mark.yen@suse.com>
  • Loading branch information
mook-as committed Jul 26, 2024
1 parent cc17a78 commit 4ec5b4f
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions pkg/rancher-desktop/integrations/manageLinesInFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const DEFAULT_FILE_MODE = 0o644;
* @param desiredPresent Whether the lines should be present.
*/
export default async function manageLinesInFile(path: string, desiredManagedLines: string[], desiredPresent: boolean): Promise<void> {
const copyFlags = fs.constants.COPYFILE_EXCL | fs.constants.COPYFILE_FICLONE;
const desired = getDesiredLines(desiredManagedLines, desiredPresent);
let fileStats: fs.Stats;

Expand Down Expand Up @@ -41,10 +40,10 @@ export default async function manageLinesInFile(path: string, desiredManagedLine

const tempName = `${ path }.rd-temp`;

await fs.promises.copyFile(path, tempName, copyFlags);
await fs.promises.copyFile(path, tempName, fs.constants.COPYFILE_EXCL | fs.constants.COPYFILE_FICLONE);

try {
const currentContents = await fs.promises.readFile(path, 'utf-8');
const currentContents = await fs.promises.readFile(tempName, 'utf-8');
const targetContents = computeTargetContents(currentContents, desired);

if (targetContents === undefined) {
Expand Down Expand Up @@ -72,9 +71,9 @@ export default async function manageLinesInFile(path: string, desiredManagedLine
} else if (fileStats.isSymbolicLink()) {
const backupPath = `${ path }.rd-backup~`;

await fs.promises.copyFile(path, backupPath, copyFlags);
await fs.promises.copyFile(path, backupPath, fs.constants.COPYFILE_EXCL | fs.constants.COPYFILE_FICLONE);

const currentContents = await fs.promises.readFile(path, 'utf-8');
const currentContents = await fs.promises.readFile(backupPath, 'utf-8');
const targetContents = computeTargetContents(currentContents, desired);

if (targetContents === undefined) {
Expand All @@ -95,7 +94,7 @@ export default async function manageLinesInFile(path: string, desiredManagedLine
} else {
// Target exists, and is neither a normal file nor a symbolic link.
// Return with an error.
throw new Error(`Refusing to manage ${ path } which is not a regular file`);
throw new Error(`Refusing to manage ${ path } which is neither a regular file nor a symbolic link`);
}
}

Expand Down

0 comments on commit 4ec5b4f

Please sign in to comment.