[PATCH 1/3] index.Writer: respect umask/sgid/etc. when creating new index

[Rob Browning]

Use atomically_replaced_file to ensure that the new index respects the
current umask, any directory sgid bit, etc., and use an ExitStack to
try to make sure all the corner cases are covered.

Signed-off-by: Rob Browning <r...@defaultvalue.org>
Tested-by: Rob Browning <r...@defaultvalue.org>
---
lib/bup/index.py | 42 ++++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/lib/bup/index.py b/lib/bup/index.py
index d7c67fb8..448155f9 100644
--- a/lib/bup/index.py
+++ b/lib/bup/index.py
@@ -1,11 +1,13 @@

-from __future__ import absolute_import, print_function
-import errno, os, stat, struct, tempfile
+from contextlib import ExitStack
+import errno, os, stat, struct

from bup import metadata, xstat
from bup._helpers import UINT_MAX, bytescmp
from bup.compat import pending_raise
-from bup.helpers import (add_error, log, merge_iter, mmap_readwrite,
+from bup.helpers import (add_error,
+ atomically_replaced_file,
+ log, merge_iter, mmap_readwrite,
progress, qprogress, resolve_parent, slashappend)

EMPTY_SHA = b'\0' * 20
@@ -540,6 +542,7 @@ class Writer:
def __init__(self, filename, metastore, tmax):
self.closed = False
self.rootlevel = self.level = Level([], None)
+ self.pending_index = None
self.f = None
self.count = 0
self.lastfile = None
@@ -548,9 +551,14 @@ class Writer:
self.metastore = metastore
self.tmax = tmax
(dir,name) = os.path.split(filename)
- ffd, self.tmpname = tempfile.mkstemp(b'.tmp', filename, dir)
- self.f = os.fdopen(ffd, 'wb', 65536)
- self.f.write(INDEX_HDR)
+ with ExitStack() as self.cleanup:
+ self.pending_index = atomically_replaced_file(self.filename,
+ mode='wb',
+ buffering=65536)
+ self.f = self.cleanup.enter_context(self.pending_index)
+ self.cleanup.enter_context(self.f)
+ self.f.write(INDEX_HDR)
+ self.cleanup = self.cleanup.pop_all()

def __enter__(self):
return self
@@ -560,12 +568,7 @@ class Writer:
self.abort()

def abort(self):
- self.closed = True
- f = self.f
- self.f = None
- if f:
- f.close()
- os.unlink(self.tmpname)
+ self.close(abort=True)

def flush(self):
if self.level:
@@ -578,14 +581,13 @@ class Writer:
self.f.flush()
assert(self.level == None)

- def close(self):
+ def close(self, abort=False):
self.closed = True
- self.flush()
- f = self.f
- self.f = None
- if f:
- f.close()
- os.rename(self.tmpname, self.filename)
+ with self.cleanup:
+ if abort:
+ self.pending_index.cancel()
+ else:
+ self.flush()

def __del__(self):
assert self.closed
@@ -633,7 +635,7 @@ class Writer:

def new_reader(self):
self.flush()
- return Reader(self.tmpname)
+ return Reader(self.f.name)


def _slashappend_or_add_error(p, caller):
--
2.30.2

[Rob Browning]

Rob Browning <r...@defaultvalue.org> writes:

> Use atomically_replaced_file to ensure that the new index respects the
> current umask, any directory sgid bit, etc., and use an ExitStack to
> try to make sure all the corner cases are covered.
>
> Signed-off-by: Rob Browning <r...@defaultvalue.org>
> Tested-by: Rob Browning <r...@defaultvalue.org>

Pushed.

--
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4