jenkins-bot has submitted this change and it was merged. Change subject: tools.py: add file_mode_checker() ...................................................................... tools.py: add file_mode_checker() Implement a single function for file permission checks. File permission check is currently done in http.py and login.py, with code repetitions. Tests added (they are skipped if 'mock' module is not available). Change-Id: I47fc750a20b024fce4fad20c4c9a726170a25da0 --- M pywikibot/comms/http.py M pywikibot/login.py M pywikibot/tools/__init__.py M tests/tools_tests.py 4 files changed, 77 insertions(+), 23 deletions(-) Approvals: Xqt: Looks good to me, approved jenkins-bot: Verified diff --git a/pywikibot/comms/http.py b/pywikibot/comms/http.py index 0f6e083..c3acdbc 100644 --- a/pywikibot/comms/http.py +++ b/pywikibot/comms/http.py @@ -23,8 +23,6 @@ __docformat__ = 'epytext' import atexit -import os -import stat import sys from distutils.version import StrictVersion @@ -56,6 +54,7 @@ from pywikibot.logging import critical, debug, error, log, warning from pywikibot.tools import ( deprecate_arg, + file_mode_checker, issue_deprecation_warning, PY2, StringTypes, @@ -77,18 +76,6 @@ config.socket_timeout = min(config.socket_timeout) -def mode_check(filename): - """Check if filename has mode 600 and, if not, set it.""" - mode_600 = stat.S_IRUSR | stat.S_IWUSR - warn_str = 'File {0} had no {1:o} mode; converted to {1:o} mode' - st_mode = os.stat(filename).st_mode - if stat.S_ISREG(st_mode) and (st_mode - stat.S_IFREG != mode_600): - os.chmod(filename, mode_600) - # re-read and check changes - if os.stat(filename).st_mode != st_mode: - pywikibot.warning(warn_str.format(filename, mode_600)) - - def mode_check_decorator(func): """Decorate load()/save() CookieJar methods.""" def wrapper(cls, **kwargs): @@ -97,7 +84,7 @@ except KeyError: filename = cls.filename res = func(cls, **kwargs) - mode_check(filename) + file_mode_checker(filename, mode=0o600) return res return wrapper diff --git a/pywikibot/login.py b/pywikibot/login.py index 4ce223b..dfc5cdf 100644 --- a/pywikibot/login.py +++ b/pywikibot/login.py @@ -3,7 +3,7 @@ """Library to log the bot in to a wiki account.""" # # (C) Rob W.W. Hooft, 2003 -# (C) Pywikibot team, 2003-2015 +# (C) Pywikibot team, 2003-2016 # # Distributed under the terms of the MIT license. # @@ -13,8 +13,9 @@ # import codecs import os -import stat import webbrowser + +from pywikibot.tools import file_mode_checker from warnings import warn @@ -236,12 +237,8 @@ if not os.path.isfile(password_file): password_file = config.password_file - # We fix password file permission first, - # lift upper permission (regular file) from st_mode - # to compare it with private_files_permission. - if os.stat(password_file).st_mode - stat.S_IFREG \ - != config.private_files_permission: - os.chmod(password_file, config.private_files_permission) + # We fix password file permission first. + file_mode_checker(password_file, mode=config.private_files_permission) password_f = codecs.open(password_file, encoding='utf-8') for line_nr, line in enumerate(password_f): diff --git a/pywikibot/tools/__init__.py b/pywikibot/tools/__init__.py index 5e4cb80..ffafbdf 100644 --- a/pywikibot/tools/__init__.py +++ b/pywikibot/tools/__init__.py @@ -12,7 +12,9 @@ import gzip import inspect import itertools +import os import re +import stat import subprocess import sys import threading @@ -168,6 +170,7 @@ class UnicodeMixin(object): + """Mixin class to add __str__ method in Python 2 or 3.""" @py2_encode_utf_8 @@ -1666,3 +1669,21 @@ def open_compressed(filename, use_extension=False): """DEPRECATED: Open a file and uncompress it if needed.""" return open_archive(filename, use_extension=use_extension) + + +def file_mode_checker(filename, mode=0o600): + """Check file mode and update it, if needed. + + @param filename: filename path + @type filename: basestring + @param mode: requested file mode + @type mode: int + + """ + warn_str = 'File {0} had {1:o} mode; converted to {2:o} mode.' + st_mode = os.stat(filename).st_mode + if stat.S_ISREG(st_mode) and (st_mode - stat.S_IFREG != mode): + os.chmod(filename, mode) + # re-read and check changes + if os.stat(filename).st_mode != st_mode: + warn(warn_str.format(filename, st_mode - stat.S_IFREG, mode)) diff --git a/tests/tools_tests.py b/tests/tools_tests.py index b38048b..c6525e9 100644 --- a/tests/tools_tests.py +++ b/tests/tools_tests.py @@ -17,12 +17,19 @@ import tempfile import warnings +try: + import mock +except ImportError as e: + mock = e + from pywikibot import tools from tests import join_xml_data_path + from tests.aspects import ( unittest, require_modules, DeprecationTestCase, TestCase, MetaTestCaseClass ) + from tests.utils import expected_failure_if, add_metaclass @@ -675,6 +682,48 @@ return inspect.getargspec(method) +@require_modules('mock') +class TestFileModeChecker(TestCase): + + """Test parsing password files.""" + + net = False + + def patch(self, name): + """Patch up <name> in self.setUp.""" + patcher = mock.patch(name) + self.addCleanup(patcher.stop) + return patcher.start() + + def setUp(self): + """Patch a variety of dependencies.""" + super(TestFileModeChecker, self).setUp() + self.stat = self.patch('os.stat') + self.chmod = self.patch('os.chmod') + self.file = '~FakeFile' + + def test_auto_chmod_for_dir(self): + """Do not chmod files that have mode private_files_permission.""" + self.stat.return_value.st_mode = 0o040600 # dir + tools.file_mode_checker(self.file, mode=0o600) + self.stat.assert_called_with(self.file) + self.assertFalse(self.chmod.called) + + def test_auto_chmod_OK(self): + """Do not chmod files that have mode private_files_permission.""" + self.stat.return_value.st_mode = 0o100600 # regular file + tools.file_mode_checker(self.file, mode=0o600) + self.stat.assert_called_with(self.file) + self.assertFalse(self.chmod.called) + + def test_auto_chmod_not_OK(self): + """Chmod files that do not have mode private_files_permission.""" + self.stat.return_value.st_mode = 0o100644 # regular file + tools.file_mode_checker(self.file, mode=0o600) + self.stat.assert_called_with(self.file) + self.chmod.assert_called_once_with(self.file, 0o600) + + if __name__ == '__main__': # pragma: no cover try: unittest.main() -- To view, visit https://gerrit.wikimedia.org/r/308354 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I47fc750a20b024fce4fad20c4c9a726170a25da0 Gerrit-PatchSet: 8 Gerrit-Project: pywikibot/core Gerrit-Branch: master Gerrit-Owner: Mpaa &lt;mpaa.wiki(a)gmail.com&gt; Gerrit-Reviewer: John Vandenberg &lt;jayvdb(a)gmail.com&gt; Gerrit-Reviewer: Merlijn van Deen &lt;valhallasw(a)arctus.nl&gt; Gerrit-Reviewer: Mpaa &lt;mpaa.wiki(a)gmail.com&gt; Gerrit-Reviewer: Xqt &lt;info(a)gno.de&gt; Gerrit-Reviewer: jenkins-bot <>