diff options
| -rw-r--r-- | changelog.d/4715.misc | 1 | ||||
| -rw-r--r-- | changelog.d/4716.misc | 1 | ||||
| -rw-r--r-- | changelog.d/4721.feature | 1 | ||||
| -rw-r--r-- | changelog.d/4722.misc | 1 | ||||
| -rw-r--r-- | changelog.d/4723.misc | 1 | ||||
| -rw-r--r-- | debian/changelog | 6 | ||||
| -rw-r--r-- | debian/install | 1 | ||||
| -rwxr-xr-x | debian/manage_debconf.pl | 130 | ||||
| -rwxr-xr-x | debian/matrix-synapse-py3.config (renamed from debian/config) | 3 | ||||
| -rw-r--r-- | debian/matrix-synapse-py3.postinst | 33 | ||||
| -rw-r--r-- | synapse/app/frontend_proxy.py | 15 | ||||
| -rw-r--r-- | synapse/crypto/keyring.py | 72 | ||||
| -rw-r--r-- | synapse/federation/federation_client.py | 18 | ||||
| -rw-r--r-- | synapse/federation/federation_server.py | 10 | ||||
| -rw-r--r-- | synapse/push/httppusher.py | 5 | ||||
| -rw-r--r-- | synapse/push/pusher.py | 2 | ||||
| -rw-r--r-- | synapse/push/pusherpool.py | 10 | ||||
| -rw-r--r-- | synapse/replication/slave/storage/_base.py | 7 | ||||
| -rw-r--r-- | synapse/storage/_base.py | 40 |
19 files changed, 308 insertions, 49 deletions
diff --git a/changelog.d/4715.misc b/changelog.d/4715.misc new file mode 100644 index 0000000000..4dc18378e7 --- /dev/null +++ b/changelog.d/4715.misc @@ -0,0 +1 @@ +Improve replication performance by reducing cache invalidation traffic. diff --git a/changelog.d/4716.misc b/changelog.d/4716.misc new file mode 100644 index 0000000000..5935f3af24 --- /dev/null +++ b/changelog.d/4716.misc @@ -0,0 +1 @@ +Reduce pusher logging on startup diff --git a/changelog.d/4721.feature b/changelog.d/4721.feature new file mode 100644 index 0000000000..f932843ce7 --- /dev/null +++ b/changelog.d/4721.feature @@ -0,0 +1 @@ +Return correct error code when inviting a remote user to a room whose homeserver does not support the room version. diff --git a/changelog.d/4722.misc b/changelog.d/4722.misc new file mode 100644 index 0000000000..e9158c4dc2 --- /dev/null +++ b/changelog.d/4722.misc @@ -0,0 +1 @@ +Don't log exceptions when failing to fetch remote server keys diff --git a/changelog.d/4723.misc b/changelog.d/4723.misc new file mode 100644 index 0000000000..96958036ca --- /dev/null +++ b/changelog.d/4723.misc @@ -0,0 +1 @@ +Correctly proxy exception in frontend_proxy worker diff --git a/debian/changelog b/debian/changelog index 124128920b..7631406a68 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +matrix-synapse-py3 (0.99.2) UNRELEASED; urgency=medium + + * Fix overwriting of config settings on upgrade. + + -- Synapse Packaging team <packages@matrix.org> Wed, 20 Feb 2019 17:11:25 +0000 + matrix-synapse-py3 (0.99.1.1) stable; urgency=medium * New synapse release 0.99.1.1 diff --git a/debian/install b/debian/install index 3d916a9718..43dc8c6904 100644 --- a/debian/install +++ b/debian/install @@ -1 +1,2 @@ debian/log.yaml etc/matrix-synapse +debian/manage_debconf.pl /opt/venvs/matrix-synapse/lib/ diff --git a/debian/manage_debconf.pl b/debian/manage_debconf.pl new file mode 100755 index 0000000000..be8ed32050 --- /dev/null +++ b/debian/manage_debconf.pl @@ -0,0 +1,130 @@ +#!/usr/bin/perl +# +# Interface between our config files and the debconf database. +# +# Usage: +# +# manage_debconf.pl <action> +# +# where <action> can be: +# +# read: read the configuration from the yaml into debconf +# update: update the yaml config according to the debconf database +use strict; +use warnings; + +use Debconf::Client::ConfModule (qw/get set/); + +# map from the name of a setting in our .yaml file to the relevant debconf +# setting. +my %MAPPINGS=( + server_name => 'matrix-synapse/server-name', + report_stats => 'matrix-synapse/report-stats', +); + +# enable debug if dpkg --debug +my $DEBUG = $ENV{DPKG_MAINTSCRIPT_DEBUG}; + +sub read_config { + my @files = @_; + + foreach my $file (@files) { + print STDERR "reading $file\n" if $DEBUG; + + open my $FH, "<", $file or next; + + # rudimentary parsing which (a) avoids having to depend on a yaml library, + # and (b) is tolerant of yaml errors + while($_ = <$FH>) { + while (my ($setting, $debconf) = each %MAPPINGS) { + $setting = quotemeta $setting; + if(/^${setting}\s*:(.*)$/) { + my $val = $1; + + # remove leading/trailing whitespace + $val =~ s/^\s*//; + $val =~ s/\s*$//; + + # remove surrounding quotes + if ($val =~ /^"(.*)"$/ || $val =~ /^'(.*)'$/) { + $val = $1; + } + + print STDERR ">> $debconf = $val\n" if $DEBUG; + set($debconf, $val); + } + } + } + close $FH; + } +} + +sub update_config { + my @files = @_; + + my %substs = (); + while (my ($setting, $debconf) = each %MAPPINGS) { + my @res = get($debconf); + $substs{$setting} = $res[1] if $res[0] == 0; + } + + foreach my $file (@files) { + print STDERR "checking $file\n" if $DEBUG; + + open my $FH, "<", $file or next; + + my $updated = 0; + + # read the whole file into memory + my @lines = <$FH>; + + while (my ($setting, $val) = each %substs) { + $setting = quotemeta $setting; + + map { + if (/^${setting}\s*:\s*(.*)\s*$/) { + my $current = $1; + if ($val ne $current) { + $_ = "${setting}: $val\n"; + $updated = 1; + } + } + } @lines; + } + close $FH; + + next unless $updated; + + print STDERR "updating $file\n" if $DEBUG; + open $FH, ">", $file or die "unable to update $file"; + print $FH @lines; + close $FH; + } +} + + +my $cmd = $ARGV[0]; + +my $read = 0; +my $update = 0; + +if (not $cmd) { + die "must specify a command to perform\n"; +} elsif ($cmd eq 'read') { + $read = 1; +} elsif ($cmd eq 'update') { + $update = 1; +} else { + die "unknown command '$cmd'\n"; +} + +my @files = ( + "/etc/matrix-synapse/homeserver.yaml", + glob("/etc/matrix-synapse/conf.d/*.yaml"), +); + +if ($read) { + read_config(@files); +} elsif ($update) { + update_config(@files); +} diff --git a/debian/config b/debian/matrix-synapse-py3.config index 9fb6913298..3bda3292f1 100755 --- a/debian/config +++ b/debian/matrix-synapse-py3.config @@ -4,6 +4,9 @@ set -e . /usr/share/debconf/confmodule +# try to update the debconf db according to whatever is in the config files +/opt/venvs/matrix-synapse/lib/manage_debconf.pl read || true + db_input high matrix-synapse/server-name || true db_input high matrix-synapse/report-stats || true db_go diff --git a/debian/matrix-synapse-py3.postinst b/debian/matrix-synapse-py3.postinst index 0509acd0a4..c0dd7e5534 100644 --- a/debian/matrix-synapse-py3.postinst +++ b/debian/matrix-synapse-py3.postinst @@ -8,19 +8,36 @@ USER="matrix-synapse" case "$1" in configure|reconfigure) - # Set server name in config file - mkdir -p "/etc/matrix-synapse/conf.d/" - db_get matrix-synapse/server-name - if [ "$RET" ]; then - echo "server_name: $RET" > $CONFIGFILE_SERVERNAME + # generate template config files if they don't exist + mkdir -p "/etc/matrix-synapse/conf.d/" + if [ ! -e "$CONFIGFILE_SERVERNAME" ]; then + cat > "$CONFIGFILE_SERVERNAME" <<EOF +# This file is autogenerated, and will be recreated on upgrade if it is deleted. +# Any changes you make will be preserved. + +# The domain name of the server, with optional explicit port. +# This is used by remote servers to connect to this server, +# e.g. matrix.org, localhost:8080, etc. +# This is also the last part of your UserID. +# +server_name: '' +EOF fi - db_get matrix-synapse/report-stats - if [ "$RET" ]; then - echo "report_stats: $RET" > $CONFIGFILE_REPORTSTATS + if [ ! -e "$CONFIGFILE_REPORTSTATS" ]; then + cat > "$CONFIGFILE_REPORTSTATS" <<EOF +# This file is autogenerated, and will be recreated on upgrade if it is deleted. +# Any changes you make will be preserved. + +# Whether to report anonymized homeserver usage statistics. +report_stats: false +EOF fi + # update the config files according to whatever is in the debconf database + /opt/venvs/matrix-synapse/lib/manage_debconf.pl update + if ! getent passwd $USER >/dev/null; then adduser --quiet --system --no-create-home --home /var/lib/matrix-synapse $USER fi diff --git a/synapse/app/frontend_proxy.py b/synapse/app/frontend_proxy.py index d5b954361d..8479fee738 100644 --- a/synapse/app/frontend_proxy.py +++ b/synapse/app/frontend_proxy.py @@ -21,7 +21,7 @@ from twisted.web.resource import NoResource import synapse from synapse import events -from synapse.api.errors import SynapseError +from synapse.api.errors import HttpResponseException, SynapseError from synapse.app import _base from synapse.config._base import ConfigError from synapse.config.homeserver import HomeServerConfig @@ -66,10 +66,15 @@ class PresenceStatusStubServlet(ClientV1RestServlet): headers = { "Authorization": auth_headers, } - result = yield self.http_client.get_json( - self.main_uri + request.uri.decode('ascii'), - headers=headers, - ) + + try: + result = yield self.http_client.get_json( + self.main_uri + request.uri.decode('ascii'), + headers=headers, + ) + except HttpResponseException as e: + raise e.to_synapse_error() + defer.returnValue((200, result)) @defer.inlineCallbacks diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py index cce40fdd2d..7474fd515f 100644 --- a/synapse/crypto/keyring.py +++ b/synapse/crypto/keyring.py @@ -17,6 +17,7 @@ import logging from collections import namedtuple +from six import raise_from from six.moves import urllib from signedjson.key import ( @@ -35,7 +36,12 @@ from unpaddedbase64 import decode_base64 from twisted.internet import defer -from synapse.api.errors import Codes, RequestSendFailed, SynapseError +from synapse.api.errors import ( + Codes, + HttpResponseException, + RequestSendFailed, + SynapseError, +) from synapse.util import logcontext, unwrapFirstError from synapse.util.logcontext import ( LoggingContext, @@ -44,6 +50,7 @@ from synapse.util.logcontext import ( run_in_background, ) from synapse.util.metrics import Measure +from synapse.util.retryutils import NotRetryingDestination logger = logging.getLogger(__name__) @@ -367,13 +374,18 @@ class Keyring(object): server_name_and_key_ids, perspective_name, perspective_keys ) defer.returnValue(result) + except KeyLookupError as e: + logger.warning( + "Key lookup failed from %r: %s", perspective_name, e, + ) except Exception as e: logger.exception( "Unable to get key from %r: %s %s", perspective_name, type(e).__name__, str(e), ) - defer.returnValue({}) + + defer.returnValue({}) results = yield logcontext.make_deferred_yieldable(defer.gatherResults( [ @@ -421,21 +433,30 @@ class Keyring(object): # TODO(mark): Set the minimum_valid_until_ts to that needed by # the events being validated or the current time if validating # an incoming request. - query_response = yield self.client.post_json( - destination=perspective_name, - path="/_matrix/key/v2/query", - data={ - u"server_keys": { - server_name: { - key_id: { - u"minimum_valid_until_ts": 0 - } for key_id in key_ids + try: + query_response = yield self.client.post_json( + destination=perspective_name, + path="/_matrix/key/v2/query", + data={ + u"server_keys": { + server_name: { + key_id: { + u"minimum_valid_until_ts": 0 + } for key_id in key_ids + } + for server_name, key_ids in server_names_and_key_ids } - for server_name, key_ids in server_names_and_key_ids - } - }, - long_retries=True, - ) + }, + long_retries=True, + ) + except (NotRetryingDestination, RequestSendFailed) as e: + raise_from( + KeyLookupError("Failed to connect to remote server"), e, + ) + except HttpResponseException as e: + raise_from( + KeyLookupError("Remote server returned an error"), e, + ) keys = {} @@ -502,11 +523,20 @@ class Keyring(object): if requested_key_id in keys: continue - response = yield self.client.get_json( - destination=server_name, - path="/_matrix/key/v2/server/" + urllib.parse.quote(requested_key_id), - ignore_backoff=True, - ) + try: + response = yield self.client.get_json( + destination=server_name, + path="/_matrix/key/v2/server/" + urllib.parse.quote(requested_key_id), + ignore_backoff=True, + ) + except (NotRetryingDestination, RequestSendFailed) as e: + raise_from( + KeyLookupError("Failed to connect to remote server"), e, + ) + except HttpResponseException as e: + raise_from( + KeyLookupError("Remote server returned an error"), e, + ) if (u"signatures" not in response or server_name not in response[u"signatures"]): diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index 4e4f58b418..58e04d81ab 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -33,6 +33,7 @@ from synapse.api.constants import ( ) from synapse.api.errors import ( CodeMessageException, + Codes, FederationDeniedError, HttpResponseException, SynapseError, @@ -792,10 +793,25 @@ class FederationClient(FederationBase): defer.returnValue(content) except HttpResponseException as e: if e.code in [400, 404]: + err = e.to_synapse_error() + + # If we receive an error response that isn't a generic error, we + # assume that the remote understands the v2 invite API and this + # is a legitimate error. + if err.errcode != Codes.UNKNOWN: + raise err + + # Otherwise, we assume that the remote server doesn't understand + # the v2 invite API. + if room_version in (RoomVersions.V1, RoomVersions.V2): pass # We'll fall through else: - raise Exception("Remote server is too old") + raise SynapseError( + 400, + "User's homeserver does not support this room version", + Codes.UNSUPPORTED_ROOM_VERSION, + ) elif e.code == 403: raise e.to_synapse_error() else: diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 3da86d4ba6..e44342bc85 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -25,9 +25,10 @@ from twisted.internet import defer from twisted.internet.abstract import isIPAddress from twisted.python import failure -from synapse.api.constants import EventTypes, Membership +from synapse.api.constants import KNOWN_ROOM_VERSIONS, EventTypes, Membership from synapse.api.errors import ( AuthError, + Codes, FederationError, IncompatibleRoomVersionError, NotFoundError, @@ -386,6 +387,13 @@ class FederationServer(FederationBase): @defer.inlineCallbacks def on_invite_request(self, origin, content, room_version): + if room_version not in KNOWN_ROOM_VERSIONS: + raise SynapseError( + 400, + "Homeserver does not support this room version", + Codes.UNSUPPORTED_ROOM_VERSION, + ) + format_ver = room_version_to_event_format(room_version) pdu = event_from_pdu_json(content, format_ver) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 899a5253d8..e65f8c63d3 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -97,6 +97,11 @@ class HttpPusher(object): pusherdict['pushkey'], ) + if self.data is None: + raise PusherConfigException( + "data can not be null for HTTP pusher" + ) + if 'url' not in self.data: raise PusherConfigException( "'url' required in data for HTTP pusher" diff --git a/synapse/push/pusher.py b/synapse/push/pusher.py index 368d5094be..b33f2a357b 100644 --- a/synapse/push/pusher.py +++ b/synapse/push/pusher.py @@ -56,7 +56,7 @@ class PusherFactory(object): f = self.pusher_types.get(kind, None) if not f: return None - logger.info("creating %s pusher for %r", kind, pusherdict) + logger.debug("creating %s pusher for %r", kind, pusherdict) return f(self.hs, pusherdict) def _create_email_pusher(self, _hs, pusherdict): diff --git a/synapse/push/pusherpool.py b/synapse/push/pusherpool.py index b2c92ce683..abf1a1a9c1 100644 --- a/synapse/push/pusherpool.py +++ b/synapse/push/pusherpool.py @@ -19,6 +19,7 @@ import logging from twisted.internet import defer from synapse.metrics.background_process_metrics import run_as_background_process +from synapse.push import PusherConfigException from synapse.push.pusher import PusherFactory logger = logging.getLogger(__name__) @@ -222,6 +223,15 @@ class PusherPool: """ try: p = self.pusher_factory.create_pusher(pusherdict) + except PusherConfigException as e: + logger.warning( + "Pusher incorrectly configured user=%s, appid=%s, pushkey=%s: %s", + pusherdict.get('user_name'), + pusherdict.get('app_id'), + pusherdict.get('pushkey'), + e, + ) + return except Exception: logger.exception("Couldn't start a pusher: caught Exception") return diff --git a/synapse/replication/slave/storage/_base.py b/synapse/replication/slave/storage/_base.py index 1353a32d00..817d1f67f9 100644 --- a/synapse/replication/slave/storage/_base.py +++ b/synapse/replication/slave/storage/_base.py @@ -59,12 +59,7 @@ class BaseSlavedStore(SQLBaseStore): members_changed = set(row.keys[1:]) self._invalidate_state_caches(room_id, members_changed) else: - try: - getattr(self, row.cache_func).invalidate(tuple(row.keys)) - except AttributeError: - # We probably haven't pulled in the cache in this worker, - # which is fine. - pass + self._attempt_to_invalidate_cache(row.cache_func, tuple(row.keys)) def _invalidate_cache_and_stream(self, txn, cache_func, keys): txn.call_after(cache_func.invalidate, keys) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 3d895da43c..5a80eef211 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -1342,15 +1342,43 @@ class SQLBaseStore(object): changed """ for member in members_changed: - self.get_rooms_for_user_with_stream_ordering.invalidate((member,)) + self._attempt_to_invalidate_cache( + "get_rooms_for_user_with_stream_ordering", (member,), + ) for host in set(get_domain_from_id(u) for u in members_changed): - self.is_host_joined.invalidate((room_id, host)) - self.was_host_joined.invalidate((room_id, host)) + self._attempt_to_invalidate_cache( + "is_host_joined", (room_id, host,), + ) + self._attempt_to_invalidate_cache( + "was_host_joined", (room_id, host,), + ) + + self._attempt_to_invalidate_cache( + "get_users_in_room", (room_id,), + ) + self._attempt_to_invalidate_cache( + "get_room_summary", (room_id,), + ) + self._attempt_to_invalidate_cache( + "get_current_state_ids", (room_id,), + ) + + def _attempt_to_invalidate_cache(self, cache_name, key): + """Attempts to invalidate the cache of the given name, ignoring if the + cache doesn't exist. Mainly used for invalidating caches on workers, + where they may not have the cache. - self.get_users_in_room.invalidate((room_id,)) - self.get_room_summary.invalidate((room_id,)) - self.get_current_state_ids.invalidate((room_id,)) + Args: + cache_name (str) + key (tuple) + """ + try: + getattr(self, cache_name).invalidate(key) + except AttributeError: + # We probably haven't pulled in the cache in this worker, + # which is fine. + pass def _send_invalidation_to_replication(self, txn, cache_name, keys): """Notifies replication that given cache has been invalidated. |