From 8d07a73b1f3a2dd702f7edf6503b7b35831a9884 Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Fri, 28 Oct 2011 12:46:23 -0400 Subject: Made Verifyfile() much more efficient and resilient: * Use file size to do an initial comparison to see if the file on disk differs from the data in the config; * Only do diffs if necessary: unified diff if -I is used, ndiff if a file is not sensitive or binary; * Both diffs have timeouts; * Fixed edge case where null-content binary files would produce stack traces, not errors. --- src/lib/Client/Tools/POSIX.py | 247 +++++++++++++++++++++++++----------------- 1 file changed, 148 insertions(+), 99 deletions(-) (limited to 'src/lib/Client/Tools') diff --git a/src/lib/Client/Tools/POSIX.py b/src/lib/Client/Tools/POSIX.py index 372d4d9e4..899b1dfa5 100644 --- a/src/lib/Client/Tools/POSIX.py +++ b/src/lib/Client/Tools/POSIX.py @@ -1,9 +1,6 @@ """All POSIX Type client support for Bcfg2.""" __revision__ = '$Revision$' -from stat import S_ISVTX, S_ISGID, S_ISUID, S_IXUSR, S_IWUSR, S_IRUSR, S_IXGRP -from stat import S_IWGRP, S_IRGRP, S_IXOTH, S_IWOTH, S_IROTH, ST_MODE, S_ISDIR -from stat import S_IFREG, ST_UID, ST_GID, S_ISREG, S_IFDIR, S_ISLNK, ST_MTIME import binascii from datetime import datetime import difflib @@ -34,10 +31,10 @@ device_map = {'block': stat.S_IFBLK, def calcPerms(initial, perms): """This compares ondisk permissions with specified ones.""" - pdisp = [{1:S_ISVTX, 2:S_ISGID, 4:S_ISUID}, - {1:S_IXUSR, 2:S_IWUSR, 4:S_IRUSR}, - {1:S_IXGRP, 2:S_IWGRP, 4:S_IRGRP}, - {1:S_IXOTH, 2:S_IWOTH, 4:S_IROTH}] + pdisp = [{1:stat.S_ISVTX, 2:stat.S_ISGID, 4:stat.S_ISUID}, + {1:stat.S_IXUSR, 2:stat.S_IWUSR, 4:stat.S_IRUSR}, + {1:stat.S_IXGRP, 2:stat.S_IWGRP, 4:stat.S_IRGRP}, + {1:stat.S_IXOTH, 2:stat.S_IWOTH, 4:stat.S_IROTH}] tempperms = initial if len(perms) == 3: perms = '0%s' % (perms) @@ -141,19 +138,11 @@ class POSIX(Bcfg2.Client.Tools.Tool): (entry.tag, entry.get('name'))) return False try: - entry.set('current_owner', str(ondisk[ST_UID])) - entry.set('current_group', str(ondisk[ST_GID])) + entry.set('current_owner', str(ondisk[stat.ST_UID])) + entry.set('current_group', str(ondisk[stat.ST_GID])) except (OSError, KeyError): pass - entry.set('perms', str(oct(ondisk[ST_MODE])[-4:])) - try: - content = open(entry.get('name')).read() - if entry.get('sensitive') not in ['true', 'True']: - entry.set('current_bfile', binascii.b2a_base64(content)) - except IOError: - error = sys.exc_info()[1] - self.logger.error("Failed to read %s: %s" % (error.filename, - error.strerror)) + entry.set('perms', str(oct(ondisk[stat.ST_MODE])[-4:])) def Verifydevice(self, entry, _): """Verify device entry.""" @@ -283,17 +272,17 @@ class POSIX(Bcfg2.Client.Tools.Tool): (entry.tag, entry.get('name'))) return False try: - owner = str(ondisk[ST_UID]) - group = str(ondisk[ST_GID]) + owner = str(ondisk[stat.ST_UID]) + group = str(ondisk[stat.ST_GID]) except (OSError, KeyError): self.logger.error('User/Group resolution failed for path %s' % \ entry.get('name')) owner = 'root' group = '0' finfo = os.stat(entry.get('name')) - perms = oct(finfo[ST_MODE])[-4:] + perms = oct(finfo[stat.ST_MODE])[-4:] if entry.get('mtime', '-1') != '-1': - mtime = str(finfo[ST_MTIME]) + mtime = str(finfo[stat.ST_MTIME]) else: mtime = '-1' pTrue = ((owner == str(normUid(entry))) and @@ -385,7 +374,7 @@ class POSIX(Bcfg2.Client.Tools.Tool): self.logger.info("Installing directory %s" % (entry.get('name'))) try: fmode = os.lstat(entry.get('name')) - if not S_ISDIR(fmode[ST_MODE]): + if not stat.S_ISDIR(fmode[stat.ST_MODE]): self.logger.debug("Found a non-directory entry at %s" % \ (entry.get('name'))) try: @@ -420,7 +409,7 @@ class POSIX(Bcfg2.Client.Tools.Tool): continue except OSError: return False - if not S_ISDIR(sloc[ST_MODE]): + if not stat.S_ISDIR(sloc[stat.ST_MODE]): try: os.unlink(current) os.mkdir(current) @@ -456,16 +445,16 @@ class POSIX(Bcfg2.Client.Tools.Tool): # permissions check + content check permissionStatus = self.Verifydirectory(entry, _) tbin = False + if entry.text == None and entry.get('empty', 'false') == 'false': + self.logger.error("Cannot verify incomplete Path type='%s' %s" % + (entry.get('type'), entry.get('name'))) + return False if entry.get('encoding', 'ascii') == 'base64': tempdata = binascii.a2b_base64(entry.text) tbin = True elif entry.get('empty', 'false') == 'true': tempdata = '' else: - if entry.text == None: - self.logger.error("Cannot verify incomplete Path type='%s' %s" % \ - (entry.get('type'), entry.get('name'))) - return False tempdata = entry.text if type(tempdata) == unicode: try: @@ -474,72 +463,104 @@ class POSIX(Bcfg2.Client.Tools.Tool): e = sys.exc_info()[1] self.logger.error("Error encoding file %s:\n %s" % \ (entry.get('name'), e)) - try: - content = open(entry.get('name')).read() - except IOError: - error = sys.exc_info()[1] - if error.strerror == "No such file or directory": - # print diff for files that don't exist (yet) - content = '' - else: - self.logger.error("Failed to read %s: %s" % \ - (error.filename, error.strerror)) + + different = False + content = None + if not os.path.exists(entry.get("name")): + # first, see if the target file exists at all; if not, + # they're clearly different + different = True + else: + # next, see if the size of the target file is different + # from the size of the desired content + try: + estat = os.stat(entry.get('name')) + except OSError: + err = sys.exc_info()[1] + self.logger.error("Failed to stat %s: %s" % + (err.filename, err)) return False - # comparison should be done with fingerprints or - # md5sum so it would be faster for big binary files - contentStatus = content == tempdata - if not contentStatus: - if tbin or not isString(content, self.setup['encoding']): - if entry.get('sensitive') not in ['true', 'True']: - entry.set('current_bfile', binascii.b2a_base64(content)) - nqtext = entry.get('qtext', '') - nqtext += '\nBinary file, no printable diff' + if len(tempdata) != estat[stat.ST_SIZE]: + different = True else: - do_diff = True - rawdiff = [] - start = time.time() - longtime = False - for x in difflib.ndiff(content.split('\n'), - tempdata.split('\n')): - now = time.time() - rawdiff.append(x) - if now - start > 5 and not longtime: - self.logger.info("Diff of %s taking a long time" % \ - (entry.get('name'))) - longtime = True - elif now - start > 30: - self.logger.error("Diff of %s took too long; giving up" % \ - (entry.get('name'))) - do_diff = False - break - if do_diff: - if entry.get('sensitive') not in ['true', 'True']: - diff = '\n'.join(rawdiff) - entry.set("current_bdiff", binascii.b2a_base64(diff)) -# entry.set("current_diff", diff) - udiff = '\n'.join([x for x in \ - difflib.unified_diff(content.split('\n'), \ - tempdata.split('\n'))]) + # finally, read in the target file and compare them + # directly. comparison could be done with a checksum, + # which might be faster for big binary files, but + # slower for everything else + try: + content = open(entry.get('name')).read() + except IOError: + err = sys.exc_info()[1] + self.logger.error("Failed to read %s: %s" % + (err.filename, err)) + return False + different = content != tempdata + + if different: + if self.setup['interactive']: + prompt = [entry.get('qtext', '')] + if not tbin and content is None: + # it's possible that we figured out the files are + # different without reading in the local file. if + # the supplied version of the file is not binary, + # we now have to read in the local file to figure + # out if _it_ is binary, and either include that + # fact or the diff in our prompts for -I try: - dudiff = udiff.decode(self.setup['encoding']) - except: - dudiff = "Binary file: no diff printed" - - nqtext = entry.get('qtext', '') + content = open(entry.get('name')).read() + except IOError: + err = sys.exc_info()[1] + self.logger.error("Failed to read %s: %s" % + (err.filename, err)) + return False + if tbin or not isString(content, self.setup['encoding']): + # don't compute diffs if the file is binary + prompt.append('Binary file, no printable diff') + else: + diff = self._diff(content, tempdata, + difflib.unified_diff, + filename=entry.get("name")) + if diff: + udiff = '\n'.join(diff) + try: + prompt.append(udiff.decode(self.setup['encoding'])) + except UnicodeDecodeError: + prompt.append("Binary file, no printable diff") + else: + prompt.append("Diff took too long to compute, no " + "printable diff") + prompt.append("Install %s %s: (y/N): " % (entry.tag, + entry.get('name'))) + entry.set("qtext", "\n".join(prompt)) + + if entry.get('sensitive', 'false').lower() != 'true': + if content is None: + # it's possible that we figured out the files are + # different without reading in the local file. we + # now have to read in the local file to figure out + # if _it_ is binary, and either include the whole + # file or the diff for reports + try: + content = open(entry.get('name')).read() + except IOError: + err = sys.exc_info()[1] + self.logger.error("Failed to read %s: %s" % + (err.filename, err)) + return False - if nqtext: - nqtext += '\n' - nqtext += dudiff + if tbin or not isString(content, self.setup['encoding']): + # don't compute diffs if the file is binary + entry.set('current_bfile', binascii.b2a_base64(content)) else: - if entry.get('sensitive') not in ['true', 'True']: + diff = self._diff(content, tempdata, difflib.ndiff, + filename=entry.get("name")) + if diff: + entry.set("current_bdiff", + binascii.b2a_base64("\n".join(diff))) + elif not tbin and isString(content, self.setup['encoding']): entry.set('current_bfile', binascii.b2a_base64(content)) - nqtext = entry.get('qtext', '') - nqtext += '\nDiff took too long to compute, no printable diff' - entry.set('qtext', nqtext) - qtxt = entry.get('qtext', '') - qtxt += "\nInstall %s %s: (y/N): " % (entry.tag, entry.get('name')) - entry.set('qtext', qtxt) - return contentStatus and permissionStatus + + return permissionStatus and not different def Installfile(self, entry): """Install Path type='file' entry.""" @@ -558,7 +579,7 @@ class POSIX(Bcfg2.Client.Tools.Tool): try: sloc = os.stat(current) try: - if not S_ISDIR(sloc[ST_MODE]): + if not stat.S_ISDIR(sloc[stat.ST_MODE]): self.logger.debug('%s is not a directory; recreating' \ % (current)) os.unlink(current) @@ -626,11 +647,15 @@ class POSIX(Bcfg2.Client.Tools.Tool): try: os.chown(newfile.name, normUid(entry), normGid(entry)) except KeyError: - self.logger.error("Failed to chown %s to %s:%s" % \ - (entry.get('name'), entry.get('owner'), + self.logger.error("Failed to chown %s to %s:%s" % + (newfile.name, entry.get('owner'), entry.get('group'))) os.chown(newfile.name, 0, 0) - os.chmod(newfile.name, calcPerms(S_IFREG, entry.get('perms'))) + except OSError: + err = sys.exc_info()[1] + self.logger.error("Could not chown %s: %s" % (newfile.name, + err)) + os.chmod(newfile.name, calcPerms(stat.S_IFREG, entry.get('perms'))) os.rename(newfile.name, entry.get('name')) if entry.get('mtime', '-1') != '-1': try: @@ -682,12 +707,12 @@ class POSIX(Bcfg2.Client.Tools.Tool): self.logger.info("Installing Hardlink %s" % (entry.get('name'))) if os.path.lexists(entry.get('name')): try: - fmode = os.lstat(entry.get('name'))[ST_MODE] - if S_ISREG(fmode) or S_ISLNK(fmode): + fmode = os.lstat(entry.get('name'))[stat.ST_MODE] + if stat.S_ISREG(fmode) or stat.S_ISLNK(fmode): self.logger.debug("Non-directory entry already exists at " "%s. Unlinking entry." % (entry.get('name'))) os.unlink(entry.get('name')) - elif S_ISDIR(fmode): + elif stat.S_ISDIR(fmode): self.logger.debug("Directory already exists at %s" % \ (entry.get('name'))) self.cmd.run("mv %s/ %s.bak" % \ @@ -790,6 +815,30 @@ class POSIX(Bcfg2.Client.Tools.Tool): return False return self.Verifydirectory(entry, _) + def _diff(self, content1, content2, difffunc, filename=None): + rv = [] + start = time.time() + longtime = True + for diffline in difffunc(content1.split('\n'), + content2.split('\n')): + now = time.time() + rv.append(diffline) + if now - start > 5 and not longtime: + if filename: + self.logger.info("Diff of %s taking a long time" % + filename) + else: + self.logger.info("Diff taking a long time") + longtime = True + elif now - start > 30: + if filename: + self.logger.error("Diff of %s took too long; giving up" % + filename) + else: + self.logger.error("Diff took too long; giving up") + return False + return rv + def Installpermissions(self, entry): """Install POSIX permissions""" if entry.get('perms') == None or \ @@ -814,7 +863,7 @@ class POSIX(Bcfg2.Client.Tools.Tool): try: for p in plist: os.chown(p, normUid(entry), normGid(entry)) - os.chmod(p, calcPerms(S_IFDIR, entry.get('perms'))) + os.chmod(p, calcPerms(stat.S_IFDIR, entry.get('perms'))) return True except (OSError, KeyError): self.logger.error('Permission fixup failed for %s' % \ @@ -854,13 +903,13 @@ class POSIX(Bcfg2.Client.Tools.Tool): self.logger.info("Installing symlink %s" % (entry.get('name'))) if os.path.lexists(entry.get('name')): try: - fmode = os.lstat(entry.get('name'))[ST_MODE] - if S_ISREG(fmode) or S_ISLNK(fmode): + fmode = os.lstat(entry.get('name'))[stat.ST_MODE] + if stat.S_ISREG(fmode) or stat.S_ISLNK(fmode): self.logger.debug("Non-directory entry already exists at " "%s. Unlinking entry." % \ (entry.get('name'))) os.unlink(entry.get('name')) - elif S_ISDIR(fmode): + elif stat.S_ISDIR(fmode): self.logger.debug("Directory already exists at %s" %\ (entry.get('name'))) self.cmd.run("mv %s/ %s.bak" % \ -- cgit v1.2.3-1-g7c22