Fixed restore/regress permissions bug

git-svn-id: http://svn.savannah.nongnu.org/svn/rdiff-backup/trunk@459 2b77aa54-bcbc-44c9-a7ec-4f6cf2b41109

7 files changed, 191 insertions, 21 deletions

diff --git a/rdiff-backup/CHANGELOG b/rdiff-backup/CHANGELOG
index 93aacd6..7a7c0f1 100644
--- a/rdiff-backup/CHANGELOG
+++ b/rdiff-backup/CHANGELOG
@@ -7,6 +7,17 @@ violation errors.
 --list-changed-since and --list-at-time now work remotely.
 Thanks to Morten Werner Olsen for bug report.
 
+Fixed logic bug that could make restoring extremely slow and waste
+memory.  Thanks for Jacques Botha for report.
+
+Fixed bug restoring some directories when mirror_metadata file was
+missing (as when made by 0.10.x version).
+
+Regressing and restoring as non-root user now works on directories
+that contain unreadable files and directories as long as they are
+owned by that user.  Bug report by Arkadiusz Miskiewicz.  Hopefully
+this is the last of the unreadable file bugs...
+
 
 New in v0.13.2 (2003/09/16)
 ---------------------------
diff --git a/rdiff-backup/rdiff-backup.1 b/rdiff-backup/rdiff-backup.1
index cb80810..4e14812 100644
--- a/rdiff-backup/rdiff-backup.1
+++ b/rdiff-backup/rdiff-backup.1
@@ -470,10 +470,10 @@ splits the filename into host_info::pathname.  It then substitutes
 host_info into the remote schema, and runs the resulting command,
 reading its input and output.
 .PP
-The default remote schema is 'ssh %s rdiff-backup --server' meaning if
-the host_info is user@host.net, then rdiff-backup runs 'ssh
-user@host.net rdiff-backup --server'.  The '%s' keyword is substituted
-with the host_info.  Using --remote-schema, rdiff-backup can invoke an
+The default remote schema is 'ssh -C %s rdiff-backup --server' where
+host_info is substituted for '%s'.  So if the host_info is
+user@host.net, then rdiff-backup runs 'ssh user@host.net rdiff-backup
+--server'.  Using --remote-schema, rdiff-backup can invoke an
 arbitrary command in order to open up a remote pipe.  For instance,
 .RS
 rdiff-backup --remote-schema 'cd /usr; %s' foo 'rdiff-backup
diff --git a/rdiff-backup/rdiff_backup/Security.py b/rdiff-backup/rdiff_backup/Security.py
index c3059d3..1805703 100644
--- a/rdiff-backup/rdiff_backup/Security.py
+++ b/rdiff-backup/rdiff_backup/Security.py
@@ -140,6 +140,7 @@ def set_allowed_requests(sec_level):
 				 "fs_abilities.get_fsabilities_restoresource",
 				 "restore.MirrorStruct.set_mirror_and_rest_times",
 				 "restore.MirrorStruct.initialize_rf_cache",
+				 "restore.MirrorStruct.close_rf_cache",
 				 "restore.MirrorStruct.get_diffs",
 				 "backup.SourceStruct.get_source_select",
 				 "backup.SourceStruct.set_source_select",
diff --git a/rdiff-backup/rdiff_backup/regress.py b/rdiff-backup/rdiff_backup/regress.py
index 32fcddb..b508138 100644
--- a/rdiff-backup/rdiff_backup/regress.py
+++ b/rdiff-backup/rdiff_backup/regress.py
@@ -112,14 +112,29 @@ def remove_rbdir_increments():
 		old_current_mirror.delete()
 
 def iterate_raw_rfs(mirror_rp, inc_rp):
-	"""Iterate all RegressFile objects in mirror/inc directory"""
+	"""Iterate all RegressFile objects in mirror/inc directory
+
+	Also changes permissions of unreadable files to allow access and
+	then changes them back later.
+
+	"""
 	root_rf = RegressFile(mirror_rp, inc_rp, restore.get_inclist(inc_rp))
 	def helper(rf):
+		mirror_rp = rf.mirror_rp
+		if (Globals.process_uid != 0 and
+			((mirror_rp.isreg() and not mirror_rp.readable()) or
+			 (mirror_rp.isdir() and not mirror_rp.hasfullperms()))):
+			unreadable, old_perms = 1, mirror_rp.getperms()
+			if mirror_rp.isreg(): mirror_rp.chmod(0400 | old_perms)
+			else: mirror_rp.chmod(0700 | old_perms)
+		else: unreadable = 0
 		yield rf
+		if unreadable and mirror_rp.isreg(): mirror_rp.chmod(old_perms)
 		if rf.mirror_rp.isdir() or rf.inc_rp.isdir():
 			for sub_rf in rf.yield_sub_rfs():
 				for sub_sub_rf in helper(sub_rf):
 					yield sub_sub_rf
+		if unreadable and mirror_rp.isdir(): mirror_rp.chmod(old_perms)
 	return helper(root_rf)
 
 def yield_metadata():
diff --git a/rdiff-backup/rdiff_backup/restore.py b/rdiff-backup/rdiff_backup/restore.py
index 2e8bf35..8d64c36 100644
--- a/rdiff-backup/rdiff_backup/restore.py
+++ b/rdiff-backup/rdiff_backup/restore.py
@@ -48,6 +48,7 @@ def Restore(mirror_rp, inc_rpath, target, restore_to_time):
 	target_iter = TargetS.get_initial_iter(target)
 	diff_iter = MirrorS.get_diffs(target_iter)
 	TargetS.patch(target, diff_iter)
+	MirrorS.close_rf_cache()
 
 def get_inclist(inc_rpath):
 	"""Returns increments with given base"""
@@ -87,7 +88,7 @@ def ListChangedSince(mirror_rp, inc_rp, restore_to_time):
 		path_desc = (old_rorp and old_rorp.get_indexpath() or
 					 cur_rorp.get_indexpath())
 		yield rpath.RORPath(("%-7s %s" % (change, path_desc),))
-
+	MirrorStruct.close_rf_cache()
 
 def ListAtTime(mirror_rp, inc_rp, time):
 	"""List the files in archive at the given time
@@ -158,6 +159,10 @@ class MirrorStruct:
 		cls.root_rf = rf
 		cls.rf_cache = CachedRF(rf)
 
+	def close_rf_cache(cls):
+		"""Run anything remaining on CachedRF object"""
+		cls.rf_cache.close()
+
 	def get_mirror_rorp_iter(cls, rest_time = None, require_metadata = None):
 		"""Return iter of mirror rps at given restore time
 
@@ -195,7 +200,9 @@ class MirrorStruct:
 		rorp = rf.get_attribs()
 		yield rorp
 		if rorp.isdir():
-			for sub_rf in rf.yield_sub_rfs(): yield sub_rf.get_attribs()
+			for sub_rf in rf.yield_sub_rfs():
+				for attribs in cls.get_rorp_iter_from_rf(sub_rf):
+					yield attribs
 
 	def subtract_indicies(cls, index, rorp_iter):
 		"""Subtract index from index of each rorp in rorp_iter
@@ -295,6 +302,8 @@ class CachedRF:
 		"""Initialize CachedRF, self.rf_list variable"""
 		self.root_rf = root_rf
 		self.rf_list = [] # list should filled in index order
+		if Globals.process_uid != 0:
+			self.perm_changer = PermissionChanger(root_rf.mirror_rp)
 
 	def list_rfs_in_cache(self, index):
 		"""Used for debugging, return indicies of cache rfs for printing"""
@@ -309,7 +318,9 @@ class CachedRF:
 			if not self.rf_list:
 				if not self.add_rfs(index): return None
 			rf = self.rf_list[0]
-			if rf.index == index: return rf
+			if rf.index == index:
+				if Globals.process_uid != 0: self.perm_changer(rf.mirror_rp)
+				return rf
 			elif rf.index > index:
 				# Try to add earlier indicies.  But if first is
 				# already from same directory, or we can't find any
@@ -339,6 +350,7 @@ The cause is probably data loss from the destination directory.""" %
 		parent_index = index[:-1]
 		temp_rf = RestoreFile(self.root_rf.mirror_rp.new_index(parent_index),
 							  self.root_rf.inc_rp.new_index(parent_index), [])
+		if Globals.process_uid != 0: self.perm_changer(temp_rf.mirror_rp)
 		new_rfs = list(temp_rf.yield_sub_rfs())
 		if not new_rfs:
 			log.Log("Warning: No RFs added for index %s" % (index,), 2)
@@ -346,6 +358,10 @@ The cause is probably data loss from the destination directory.""" %
 		self.rf_list[0:0] = new_rfs
 		return 1
 
+	def close(self):
+		"""Finish remaining rps in PermissionChanger"""
+		if Globals.process_uid != 0: self.perm_changer.finish()
+
 
 class RestoreFile:
 	"""Hold data about a single mirror file and its related increments
@@ -617,3 +633,65 @@ class PatchITRB(rorpiter.ITRBranch):
 			self.base_rp.rmdir()
 			if self.dir_replacement.lstat():
 				rpath.rename(self.dir_replacement, self.base_rp)
+
+
+class PermissionChanger:
+	"""Change the permission of mirror files and directories
+
+	The problem is that mirror files and directories may need their
+	permissions changed in order to be read and listed, and then
+	changed back when we are done.  This class hooks into the CachedRF
+	object to know when an rp is needed.
+
+	"""
+	def __init__(self, root_rp):
+		self.root_rp = root_rp
+		self.current_index = ()
+		# Below is a list of (index, rp, old_perm) triples in reverse
+		# order that need clearing
+		self.open_index_list = []
+
+	def __call__(self, rp):
+		"""Given rpath, change permissions up and including rp"""
+		index, old_index = rp.index, self.current_index
+		self.current_index = index
+		if not index or index == old_index: return
+		assert index > old_index, (index, old_index)
+		self.restore_old(rp, index)
+		self.add_new(rp, old_index, index)
+
+	def restore_old(self, rp, index):
+		"""Restore permissions for indicies we are done with"""
+		while self.open_index_list:
+			old_index, old_rp, old_perms = self.open_index_list[0]
+			if index[:len(old_index)] > old_index: old_rp.chmod(old_perms)
+			else: break
+			del self.open_index_list[0]
+
+	def add_new(self, rp, old_index, index):
+		"""Change permissions of directories between old_index and index"""
+		for rp in self.get_new_rp_list(rp, old_index, index):
+			if ((rp.isreg() and not rp.readable()) or
+				(rp.isdir() and not rp.hasfullperms())):
+				old_perms = rp.getperms()
+				self.open_index_list.insert(0, (index, rp, old_perms))
+				if rp.isreg(): rp.chmod(0400 | old_perms)
+				else: rp.chmod(0700 | old_perms)
+
+	def get_new_rp_list(self, rp, old_index, index):
+		"""Return list of new rp's between old_index and index"""
+		for i in range(len(index)-1, -1, -1):
+			if old_index[:i] == index[:i]:
+				common_prefix_len = i
+				break
+		else: assert 0
+
+		new_rps = []
+		for total_len in range(common_prefix_len+1, len(index)):
+			new_rps.append(self.root_rp.new_index(index[:total_len]))
+		new_rps.append(rp)
+		return new_rps
+
+	def finish(self):
+		"""Restore any remaining rps"""
+		for index, rp, perms in self.open_index_list: rp.chmod(perms)
diff --git a/rdiff-backup/testing/regresstest.py b/rdiff-backup/testing/regresstest.py
index be02e67..a94f4ae 100644
--- a/rdiff-backup/testing/regresstest.py
+++ b/rdiff-backup/testing/regresstest.py
@@ -93,4 +93,49 @@ class RegressTest(unittest.TestCase):
 		"""Run regress test remotely"""
 		self.runtest(self.regress_to_time_remote)
 
+	def test_unreadable(self):
+		"""Run regress test when regular file is unreadable"""
+		self.output_rp.setdata()
+		if self.output_rp.lstat(): Myrm(self.output_rp.path)
+		unreadable_rp = self.make_unreadable()
+
+		rdiff_backup(1, 1, unreadable_rp.path, self.output_rp.path,
+					 current_time = 1)
+		rbdir = self.output_rp.append('rdiff-backup-data')
+		marker = rbdir.append('current_mirror.2000-12-31T21:33:20-07:00.data')
+		marker.touch()
+		self.change_unreadable()
+
+		cmd = "rdiff-backup --check-destination-dir " + self.output_rp.path
+		print "Executing:", cmd
+		assert not os.system(cmd)
+
+	def make_unreadable(self):
+		"""Make unreadable input directory
+
+		The directory needs to be readable initially (otherwise it
+		just won't get backed up, and then later we will turn it
+		unreadable.
+
+		"""
+		rp = rpath.RPath(Globals.local_connection, "testfiles/regress")
+		if rp.lstat(): Myrm(rp.path)
+		rp.setdata()
+		rp.mkdir()
+		rp1 = rp.append('unreadable_dir')
+		rp1.mkdir()
+		rp1_1 = rp1.append('to_be_unreadable')
+		rp1_1.write_string('aensuthaoeustnahoeu')
+		return rp
+
+	def change_unreadable(self):
+		"""Change attributes in directory, so regress will request fp"""
+		subdir = self.output_rp.append('unreadable_dir')
+		assert subdir.lstat()
+		filerp = subdir.append('to_be_unreadable')
+		filerp.chmod(0)
+		subdir.chmod(0)
+		
+
 if __name__ == "__main__": unittest.main()
+
diff --git a/rdiff-backup/testing/roottest.py b/rdiff-backup/testing/roottest.py
index 176d4ad..a498ff2 100644
--- a/rdiff-backup/testing/roottest.py
+++ b/rdiff-backup/testing/roottest.py
@@ -11,7 +11,7 @@ if you aren't me, check out the 'user' global variable.
 
 Globals.set('change_source_perms', None)
 Globals.counter = 0
-verbosity = 6
+verbosity = 5
 log.Log.setverbosity(verbosity)
 user = 'ben' # Non-root user to su to
 userid = 500 # id of user above
@@ -95,6 +95,13 @@ class HalfRoot(unittest.TestCase):
 		rp1_3.mkdir()
 		rp1_3_1 = rp1_3.append('file_inside')
 		rp1_3_1.write_string('blah')
+		rp1_3_1.chmod(0)
+		rp1_3_2 = rp1_3.append('subdir_inside')
+		rp1_3_2.mkdir()
+		rp1_3_2_1 = rp1_3_2.append('foo')
+		rp1_3_2_1.write_string('saotnhu')
+		rp1_3_2_1.chmod(0)
+		rp1_3_2.chmod(0)
 		rp1_3.chmod(0)
 
 		rp2 = rpath.RPath(Globals.local_connection, "testfiles/root_half2")
@@ -105,8 +112,17 @@ class HalfRoot(unittest.TestCase):
 		rp2_1.chmod(0)
 		rp2_3 = rp2.append('unreadable_dir')
 		rp2_3.mkdir()
-		rp2_3_2 = rp2_3.append('file2')
-		rp2_3_2.touch()
+		rp2_3_1 = rp2_3.append('file_inside')
+		rp2_3_1.write_string('new string')
+		rp2_3_1.chmod(0)
+		rp2_3_2 = rp2_3.append('subdir_inside')
+		rp2_3_2.mkdir()
+		rp2_3_2_1 = rp2_3_2.append('foo')
+		rp2_3_2_1.write_string('asoetn;oet')
+		rp2_3_2_1.chmod(0)
+		rp2_3_2.chmod(0)
+		rp2_3_3 = rp2_3.append('file2')
+		rp2_3_3.touch()
 		rp2_3.chmod(0)
 		return rp1, rp2
 
@@ -135,23 +151,27 @@ class HalfRoot(unittest.TestCase):
 
 		rout_rp = rpath.RPath(Globals.local_connection,
 							  "testfiles/restore_out")
+		restore_schema = ("rdiff-backup -v" + str(verbosity) +
+						  " -r %s --remote-schema '%%s' '%s'::%s %s")
 		Myrm(rout_rp.path)
-		cmd3 = cmd_schema % (10000,
-							 "-r 10000 %s/unreadable_dir" % (outrp.path,),
-							 remote_schema,  rout_rp.path)
+		cmd3 = restore_schema % (10000, remote_schema, outrp.path,
+								 rout_rp.path)
 		print "Executing restore: ", cmd3
 		assert not os.system(cmd3)
-		rout_rp.setdata()
-		assert rout_rp.getperms() == 0, rout_rp.getperms()
+		rout_perms = rout_rp.append('unreadable_dir').getperms()
+		outrp_perms = outrp.append('unreadable_dir').getperms()
+		assert rout_perms == 0, rout_perms
+		assert outrp_perms == 0, outrp_perms
 
 		Myrm(rout_rp.path)
-		cmd4 = cmd_schema % (10000,
-							 "-r now %s/unreadable_dir" % (outrp.path,),
-							 remote_schema,  rout_rp.path)
+		cmd4 = restore_schema % ("now", remote_schema, outrp.path,
+								 rout_rp.path)
 		print "Executing restore: ", cmd4
 		assert not os.system(cmd4)
-		rout_rp.setdata()
-		assert rout_rp.getperms() == 0, rout_rp.getperms()
+		rout_perms = rout_rp.append('unreadable_dir').getperms()
+		outrp_perms = outrp.append('unreadable_dir').getperms()
+		assert rout_perms == 0, rout_perms
+		assert outrp_perms == 0, outrp_perms
 
 
 class NonRoot(unittest.TestCase):