diff --git a/.lgtm.yml b/.lgtm.yml new file mode 100644 index 000000000..efc2479ca --- /dev/null +++ b/.lgtm.yml @@ -0,0 +1,22 @@ +extraction: + python: + after_prepare: + - | + # https://discuss.lgtm.com/t/determination-of-python-requirements/974/4 + sed -i 's/\("pyOpenSSL\)/\# Dependency removed for lgtm (see .lgtm.yml): \1/g' src/allmydata/_auto_deps.py + +queries: + # This generates spurious errors for calls by interface because of the + # zope.interface choice to exclude self from method signatures. So, turn it + # off. + - exclude: "py/call/wrong-arguments" + + # The premise of this query is broken. The errors it produces are nonsense. + # There is no such thing as a "procedure" in Python and "None" is not + # meaningless. + - exclude: "py/procedure-return-value-used" + + # It is true that this query identifies things which are sometimes mistakes. + # However, it also identifies things which are entirely valid. Therefore, + # it produces noisy results. + - exclude: "py/implicit-string-concatenation-in-list" \ No newline at end of file diff --git a/docs/proposed/magic-folder/filesystem-integration.rst b/docs/proposed/magic-folder/filesystem-integration.rst index 6ed0671ed..589db2322 100644 --- a/docs/proposed/magic-folder/filesystem-integration.rst +++ b/docs/proposed/magic-folder/filesystem-integration.rst @@ -62,13 +62,15 @@ to the target folder cease for a sufficiently long period of time. *Detecting filesystem changes* -For the Linux implementation, we will use the inotify Linux kernel +For the Linux implementation, we will use the `inotify`_ Linux kernel subsystem to gather events on the local Magic Folder directory tree. This implementation was already present in Tahoe-LAFS 1.9.0, but needs to be changed to gather directory creation and move events, in addition to the events indicating that a file has been written that are gathered by the current code. +.. _`inotify`: https://en.wikipedia.org/wiki/Inotify + For the Windows implementation, we will use the ``ReadDirectoryChangesW`` Win32 API. The prototype implementation simulates a Python interface to the inotify API in terms of ``ReadDirectoryChangesW``, allowing most of diff --git a/misc/coding_tools/make-canary-files.py b/misc/coding_tools/make-canary-files.py index 57f900a5c..4ba06cd9c 100644 --- a/misc/coding_tools/make-canary-files.py +++ b/misc/coding_tools/make-canary-files.py @@ -50,7 +50,7 @@ system where Tahoe is installed, or in a source tree with setup.py like this: setup.py run_with_pythonpath -p -c 'misc/make-canary-files.py ARGS..' """ -import os, sha +import os, hashlib from twisted.python import usage from allmydata.immutable import upload from allmydata.util import base32 @@ -96,7 +96,7 @@ convergence = base32.a2b(convergence_s) def get_permuted_peers(key): results = [] for nodeid in nodes: - permuted = sha.new(key + nodeid).digest() + permuted = hashlib.sha1(key + nodeid).digest() results.append((permuted, nodeid)) results.sort(lambda a,b: cmp(a[0], b[0])) return [ r[1] for r in results ] diff --git a/misc/operations_helpers/munin/tahoe_nodememory b/misc/operations_helpers/munin/tahoe_nodememory index fd3f8b0a4..1ecf53fc9 100644 --- a/misc/operations_helpers/munin/tahoe_nodememory +++ b/misc/operations_helpers/munin/tahoe_nodememory @@ -6,10 +6,9 @@ import os, sys, re -if 0: - # for testing - os.environ["nodememory_warner1"] = "run/warner1" - os.environ["nodememory_warner2"] = "run/warner2" +# for testing +# os.environ["nodememory_warner1"] = "run/warner1" +# os.environ["nodememory_warner2"] = "run/warner2" nodedirs = [] for k,v in os.environ.items(): diff --git a/misc/simulators/simulator.py b/misc/simulators/simulator.py index c1f9e9b54..a01c5c8c8 100644 --- a/misc/simulators/simulator.py +++ b/misc/simulators/simulator.py @@ -1,6 +1,6 @@ #! /usr/bin/env python -import sha as shamodule +import hashlib import os, random from pkg_resources import require @@ -10,7 +10,7 @@ from pyrrd.rrd import DataSource, RRD, RRA def sha(s): - return shamodule.new(s).digest() + return hashlib.sha1(s).digest() def randomid(): return os.urandom(20) @@ -70,10 +70,7 @@ class Node: return False def decide(self, sharesize): - if sharesize > self.capacity: - return False return False - return random.random() > 0.5 def make_space(self, sharesize): assert sharesize <= self.capacity diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 341ada081..71397f961 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -593,23 +593,7 @@ class _Client(node.Node, pollmixin.PollMixin): for (name, mf_config) in magic_folders.items(): self.log("Starting magic_folder '{}'".format(name)) - db_filename = os.path.join(self.basedir, "private", "magicfolder_{}.sqlite".format(name)) - local_dir_config = mf_config['directory'] - try: - poll_interval = int(mf_config["poll_interval"]) - except ValueError: - raise ValueError("'poll_interval' option must be an int") - - s = magic_folder.MagicFolder( - client=self, - upload_dircap=mf_config["upload_dircap"].encode('ascii'), - collective_dircap=mf_config["collective_dircap"].encode('ascii'), - local_path_u=abspath_expanduser_unicode(local_dir_config, base=self.basedir), - dbfile=abspath_expanduser_unicode(db_filename), - umask=self.get_config("magic_folder", "download.umask", 0077), - name=name, - downloader_delay=poll_interval, - ) + s = magic_folder.MagicFolder.from_config(self, name, mf_config) self._magic_folders[name] = s s.setServiceParent(self) s.startService() diff --git a/src/allmydata/frontends/magic_folder.py b/src/allmydata/frontends/magic_folder.py index 5727a5343..24fd9cb0f 100644 --- a/src/allmydata/frontends/magic_folder.py +++ b/src/allmydata/frontends/magic_folder.py @@ -18,7 +18,12 @@ from zope.interface import Interface, Attribute, implementer from allmydata.util import fileutil, configutil, yamlutil from allmydata.interfaces import IDirectoryNode from allmydata.util import log -from allmydata.util.fileutil import precondition_abspath, get_pathinfo, ConflictError +from allmydata.util.fileutil import ( + precondition_abspath, + get_pathinfo, + ConflictError, + abspath_expanduser_unicode, +) from allmydata.util.assertutil import precondition, _assert from allmydata.util.deferredutil import HookMixin from allmydata.util.progress import PercentProgress @@ -30,6 +35,9 @@ from allmydata.immutable.upload import FileName, Data from allmydata import magicfolderdb, magicpath +# Mask off all non-owner permissions for magic-folders files by default. +_DEFAULT_DOWNLOAD_UMASK = 0o077 + IN_EXCL_UNLINK = 0x04000000L def get_inotify_module(): @@ -119,6 +127,7 @@ def load_magic_folders(node_directory): old-style to new-style config (but WILL read old-style config and return in the same way as if it was new-style). + :param node_directory: path where node data is stored :returns: dict mapping magic-folder-name to its config (also a dict) """ yaml_fname = os.path.join(node_directory, u"private", u"magic_folders.yaml") @@ -156,11 +165,17 @@ def load_magic_folders(node_directory): ) ) + if config.has_option("magic_folder", "download.umask"): + umask = int(config.get("magic_folder", "download.umask"), 8) + else: + umask = _DEFAULT_DOWNLOAD_UMASK + folders[u"default"] = { u"directory": directory, u"upload_dircap": fileutil.read(up_fname), u"collective_dircap": fileutil.read(coll_fname), u"poll_interval": interval, + u"umask": umask, } else: # without any YAML file AND no local.directory option it's @@ -212,6 +227,12 @@ def load_magic_folders(node_directory): if isinstance(mf_config[k], unicode): mf_config[k] = mf_config[k].encode('ascii') + if not isinstance( + mf_config.setdefault(u"umask", _DEFAULT_DOWNLOAD_UMASK), + int, + ): + raise Exception("magic-folder download umask must be an integer") + return folders @@ -228,6 +249,43 @@ def save_magic_folders(node_directory, folders): class MagicFolder(service.MultiService): + @classmethod + def from_config(cls, client_node, name, config): + """ + Create a ``MagicFolder`` from a client node and magic-folder + configuration. + + :param _Client client_node: The client node the magic-folder is + attached to. + + :param dict config: Magic-folder configuration like that in the list + returned by ``load_magic_folders``. + """ + db_filename = os.path.join( + client_node.basedir, + "private", + "magicfolder_{}.sqlite".format(name), + ) + local_dir_config = config['directory'] + try: + poll_interval = int(config["poll_interval"]) + except ValueError: + raise ValueError("'poll_interval' option must be an int") + + return cls( + client=client_node, + upload_dircap=config["upload_dircap"], + collective_dircap=config["collective_dircap"], + local_path_u=abspath_expanduser_unicode( + local_dir_config, + base=client_node.basedir, + ), + dbfile=abspath_expanduser_unicode(db_filename), + umask=config["umask"], + name=name, + downloader_delay=poll_interval, + ) + def __init__(self, client, upload_dircap, collective_dircap, local_path_u, dbfile, umask, name, uploader_delay=1.0, clock=None, downloader_delay=60): precondition_abspath(local_path_u) diff --git a/src/allmydata/frontends/sftpd.py b/src/allmydata/frontends/sftpd.py index 028ea7ee5..83d1e540a 100644 --- a/src/allmydata/frontends/sftpd.py +++ b/src/allmydata/frontends/sftpd.py @@ -38,24 +38,9 @@ from allmydata.dirnode import update_metadata from allmydata.util.fileutil import EncryptedTemporaryFile noisy = True -use_foolscap_logging = True from allmydata.util.log import NOISY, OPERATIONAL, WEIRD, \ - msg as _msg, err as _err, PrefixingLogMixin as _PrefixingLogMixin - -if use_foolscap_logging: - (logmsg, logerr, PrefixingLogMixin) = (_msg, _err, _PrefixingLogMixin) -else: # pragma: no cover - def logmsg(s, level=None): - print s - def logerr(s, level=None): - print s - class PrefixingLogMixin: - def __init__(self, facility=None, prefix=''): - self.prefix = prefix - def log(self, s, level=None): - print "%r %s" % (self.prefix, s) - + msg as logmsg, PrefixingLogMixin def eventually_callback(d): return lambda res: eventually(d.callback, res) diff --git a/src/allmydata/immutable/layout.py b/src/allmydata/immutable/layout.py index c634702ab..320f742e0 100644 --- a/src/allmydata/immutable/layout.py +++ b/src/allmydata/immutable/layout.py @@ -438,9 +438,7 @@ class ReadBucketProxy(object): def _get_share_hashes(self, unused=None): if hasattr(self, '_share_hashes'): return self._share_hashes - else: - return self._get_share_hashes_the_old_way() - return self._share_hashes + return self._get_share_hashes_the_old_way() def get_share_hashes(self): d = self._start_if_needed() diff --git a/src/allmydata/immutable/upload.py b/src/allmydata/immutable/upload.py index cef226a8e..26d0af30d 100644 --- a/src/allmydata/immutable/upload.py +++ b/src/allmydata/immutable/upload.py @@ -533,7 +533,6 @@ class Tahoe2ServerSelector(log.PrefixingLogMixin): # we haven't improved over the last iteration; give up break; if errors_before == self._query_stats.bad: - if False: print("no more errors; break") break; last_happiness = effective_happiness # print("write trackers left: {}".format(len(write_trackers))) diff --git a/src/allmydata/mutable/publish.py b/src/allmydata/mutable/publish.py index ee17847e4..50db1e2e3 100644 --- a/src/allmydata/mutable/publish.py +++ b/src/allmydata/mutable/publish.py @@ -909,9 +909,7 @@ class Publish: level=log.NOISY) def update_goal(self): - # if log.recording_noisy - if True: - self.log_goal(self.goal, "before update: ") + self.log_goal(self.goal, "before update: ") # first, remove any bad servers from our goal self.goal = set([ (server, shnum) @@ -976,8 +974,7 @@ class Publish: i += 1 if i >= len(serverlist): i = 0 - if True: - self.log_goal(self.goal, "after update: ") + self.log_goal(self.goal, "after update: ") def _got_write_answer(self, answer, writer, started): diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py index d616cf199..70ff75bf2 100644 --- a/src/allmydata/scripts/runner.py +++ b/src/allmydata/scripts/runner.py @@ -47,7 +47,6 @@ class Options(usage.Options): stdout = sys.stdout stderr = sys.stderr - synopsis = "\nUsage: tahoe [command options]" subCommands = ( GROUP("Administration") + create_node.subCommands + stats_gatherer.subCommands diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py index cf1d92b7b..b99769d3c 100644 --- a/src/allmydata/test/test_client.py +++ b/src/allmydata/test/test_client.py @@ -401,11 +401,18 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test _check("helper.furl = pb://blah\n", "pb://blah") def test_create_magic_folder_service(self): - class MockMagicFolder(service.MultiService): + boom = False + class Boom(Exception): + pass + + class MockMagicFolder(allmydata.frontends.magic_folder.MagicFolder): name = 'magic-folder' def __init__(self, client, upload_dircap, collective_dircap, local_path_u, dbfile, umask, name, inotify=None, uploader_delay=1.0, clock=None, downloader_delay=3): + if boom: + raise Boom() + service.MultiService.__init__(self) self.client = client self._umask = umask @@ -415,6 +422,12 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test self.dbfile = dbfile self.inotify = inotify + def startService(self): + self.running = True + + def stopService(self): + self.running = False + def ready(self): pass @@ -461,12 +474,8 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test self.failUnless(magicfolder.inotify is None, magicfolder.inotify) self.failUnless(magicfolder.running) - class Boom(Exception): - pass - def BoomMagicFolder(client, upload_dircap, collective_dircap, local_path_u, dbfile, name, - umask, inotify=None, uploader_delay=1.0, clock=None, downloader_delay=3): - raise Boom() - self.patch(allmydata.frontends.magic_folder, 'MagicFolder', BoomMagicFolder) + # See above. + boom = True basedir2 = "test_client.Basic.test_create_magic_folder_service2" os.mkdir(basedir2) diff --git a/src/allmydata/test/test_magic_folder.py b/src/allmydata/test/test_magic_folder.py index 451545584..0f26c99f2 100644 --- a/src/allmydata/test/test_magic_folder.py +++ b/src/allmydata/test/test_magic_folder.py @@ -65,6 +65,7 @@ class NewConfigUtilTests(unittest.TestCase): def test_load(self): folders = magic_folder.load_magic_folders(self.basedir) self.assertEqual(['default'], list(folders.keys())) + self.assertEqual(folders['default'][u'umask'], 0o077) def test_both_styles_of_config(self): os.unlink(join(self.basedir, u"private", u"magic_folders.yaml")) @@ -115,6 +116,23 @@ class NewConfigUtilTests(unittest.TestCase): str(ctx.exception) ) + def test_wrong_umask_obj(self): + """ + If a umask is given for a magic-folder that is not an integer, an + exception is raised. + """ + self.folders[u"default"][u"umask"] = "0077" + yaml_fname = join(self.basedir, u"private", u"magic_folders.yaml") + with open(yaml_fname, "w") as f: + f.write(yamlutil.safe_dump({u"magic-folders": self.folders})) + + with self.assertRaises(Exception) as ctx: + magic_folder.load_magic_folders(self.basedir) + self.assertIn( + "umask must be an integer", + str(ctx.exception) + ) + def test_wrong_sub_obj(self): yaml_fname = join(self.basedir, u"private", u"magic_folders.yaml") with open(yaml_fname, "w") as f: diff --git a/src/allmydata/util/encodingutil.py b/src/allmydata/util/encodingutil.py index 65f5911a1..11ab942b6 100644 --- a/src/allmydata/util/encodingutil.py +++ b/src/allmydata/util/encodingutil.py @@ -335,8 +335,8 @@ def listdir_unicode_fallback(path): try: return [unicode(fn, filesystem_encoding) for fn in os.listdir(byte_path)] - except UnicodeDecodeError: - raise FilenameEncodingError(fn) + except UnicodeDecodeError as e: + raise FilenameEncodingError(e.object) def listdir_unicode(path): """ diff --git a/src/allmydata/util/tor_provider.py b/src/allmydata/util/tor_provider.py index ccb963775..14d871cd0 100644 --- a/src/allmydata/util/tor_provider.py +++ b/src/allmydata/util/tor_provider.py @@ -72,13 +72,9 @@ def _launch_tor(reactor, tor_executable, private_dir, txtorcon): tor_config = txtorcon.TorConfig() tor_config.DataDirectory = data_directory(private_dir) - if True: # unix-domain control socket - tor_config.ControlPort = "unix:" + os.path.join(private_dir, "tor.control") - tor_control_endpoint_desc = tor_config.ControlPort - else: - # we allocate a new TCP control port each time - tor_config.ControlPort = allocate_tcp_port() - tor_control_endpoint_desc = "tcp:127.0.0.1:%d" % tor_config.ControlPort + # unix-domain control socket + tor_config.ControlPort = "unix:" + os.path.join(private_dir, "tor.control") + tor_control_endpoint_desc = tor_config.ControlPort tor_config.SOCKSPort = allocate_tcp_port() diff --git a/src/allmydata/util/verlib.py b/src/allmydata/util/verlib.py index 6d8d8e254..619f1a845 100644 --- a/src/allmydata/util/verlib.py +++ b/src/allmydata/util/verlib.py @@ -254,7 +254,7 @@ def suggest_normalized_version(s): # if we have something like "b-2" or "a.2" at the end of the # version, that is pobably beta, alpha, etc # let's remove the dash or dot - rs = re.sub(r"([abc|rc])[\-\.](\d+)$", r"\1\2", rs) + rs = re.sub(r"([abc]|rc)[\-\.](\d+)$", r"\1\2", rs) # 1.0-dev-r371 -> 1.0.dev371 # 0.1-dev-r79 -> 0.1.dev79 @@ -324,4 +324,3 @@ def suggest_normalized_version(s): except IrrationalVersionError: pass return None - diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index f21e1dffa..1cfd7a17b 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -74,8 +74,6 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): return d def got_child(self, node_or_failure, ctx, name): - DEBUG = False - if DEBUG: print "GOT_CHILD", name, node_or_failure req = IRequest(ctx) method = req.method nonterminal = len(req.postpath) > 1 @@ -84,24 +82,18 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): f = node_or_failure f.trap(NoSuchChildError) # No child by this name. What should we do about it? - if DEBUG: print "no child", name - if DEBUG: print "postpath", req.postpath if nonterminal: - if DEBUG: print " intermediate" if should_create_intermediate_directories(req): # create intermediate directories - if DEBUG: print " making intermediate directory" d = self.node.create_subdirectory(name) d.addCallback(make_handler_for, self.client, self.node, name) return d else: - if DEBUG: print " terminal" # terminal node if (method,t) in [ ("POST","mkdir"), ("PUT","mkdir"), ("POST", "mkdir-with-children"), ("POST", "mkdir-immutable") ]: - if DEBUG: print " making final directory" # final directory kids = {} if t in ("mkdir-with-children", "mkdir-immutable"): @@ -122,14 +114,12 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): self.client, self.node, name) return d if (method,t) in ( ("PUT",""), ("PUT","uri"), ): - if DEBUG: print " PUT, making leaf placeholder" # we were trying to find the leaf filenode (to put a new # file in its place), and it didn't exist. That's ok, # since that's the leaf node that we're about to create. # We make a dummy one, which will respond to the PUT # request by replacing itself. return PlaceHolderNodeHandler(self.client, self.node, name) - if DEBUG: print " 404" # otherwise, we just return a no-such-child error return f @@ -138,11 +128,9 @@ class DirectoryNodeHandler(RenderMixin, rend.Page, ReplaceMeMixin): if not IDirectoryNode.providedBy(node): # we would have put a new directory here, but there was a # file in the way. - if DEBUG: print "blocking" raise WebError("Unable to create directory '%s': " "a file was in the way" % name, http.CONFLICT) - if DEBUG: print "good child" return make_handler_for(node, self.client, self.node, name) def render_DELETE(self, ctx): diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index a07fbb4f2..bd404bbd4 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -2,6 +2,10 @@ import re, time from twisted.application import service, strports, internet from twisted.web import http, static from twisted.internet import defer +from twisted.internet.address import ( + IPv4Address, + IPv6Address, +) from nevow import appserver, inevow from allmydata.util import log, fileutil @@ -118,16 +122,32 @@ class MyRequest(appserver.NevowRequest): if self._tahoe_request_had_error: error = " [ERROR]" - log.msg(format="web: %(clientip)s %(method)s %(uri)s %(code)s %(length)s%(error)s", - clientip=self.getClientIP(), - method=self.method, - uri=uri, - code=self.code, - length=(self.sentLength or "-"), - error=error, - facility="tahoe.webish", - level=log.OPERATIONAL, - ) + log.msg( + format=( + "web: %(clientip)s %(method)s %(uri)s %(code)s " + "%(length)s%(error)s" + ), + clientip=_get_client_ip(self), + method=self.method, + uri=uri, + code=self.code, + length=(self.sentLength or "-"), + error=error, + facility="tahoe.webish", + level=log.OPERATIONAL, + ) + + +def _get_client_ip(request): + try: + get = request.getClientAddress + except AttributeError: + return request.getClientIP() + else: + client_addr = get() + if isinstance(client_addr, (IPv4Address, IPv6Address)): + return client_addr.host + return None class WebishServer(service.MultiService): diff --git a/src/allmydata/windows/registry.py b/src/allmydata/windows/registry.py index 2b87689b8..4801310d3 100644 --- a/src/allmydata/windows/registry.py +++ b/src/allmydata/windows/registry.py @@ -5,9 +5,9 @@ _AMD_KEY = r"Software\Allmydata" _BDIR_KEY = 'Base Dir Path' if sys.platform not in ('win32'): - raise ImportError, "registry cannot be used on non-windows systems" class WindowsError(Exception): # stupid voodoo to appease pyflakes pass + raise ImportError("registry cannot be used on non-windows systems") def get_registry_setting(key, name, _topkey=None): """