Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Legacy spam checkers for are broken #10234

Closed
erikjohnston opened this issue Jun 23, 2021 · 0 comments · Fixed by #10238
Closed

Legacy spam checkers for are broken #10234

erikjohnston opened this issue Jun 23, 2021 · 0 comments · Fixed by #10238
Labels
S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Release-Blocker Must be resolved before making a release

Comments

@erikjohnston
Copy link
Member

Stack trace:

TypeError: wrapper() missing 1 required positional argument: 'auth_provider_id'
  File "synapse/http/server.py", line 258, in _async_render_wrapper
    callback_return = await self._async_render(request)
  File "synapse/http/server.py", line 446, in _async_render
    callback_return = await raw_callback_return
  File "synapse/rest/client/v2_alpha/_base.py", line 96, in wrapped
    return await orig(*args, **kwargs)
  File "synapse/rest/client/v2_alpha/register.py", line 420, in on_POST
    ret = await self._do_guest_registration(body, address=client_addr)
  File "synapse/rest/client/v2_alpha/register.py", line 723, in _do_guest_registration
    make_guest=True, address=address
  File "synapse/handlers/register.py", line 214, in register_user
    auth_provider_id=auth_provider_id,
  File "synapse/events/spamcheck.py", line 370, in check_registration_for_spam
    behaviour = await (
  File "synapse/events/spamcheck.py", line 148, in run
    
  File "synapse/events/spamcheck.py", line 133, in wrapper
    username,

(Sentry link: https://sentry.matrix.org/sentry/synapse-matrixorg/issues/223977/?query=is%3Aunresolved)

I think this is here:

return f(
email_threepid,
username,
request_info,
)
f = wrapper

where we set f = wrapper, so when wrapper(..) calls f(..) it is actually just calling itself.

Patch:

diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py
index 45ec96dfc..5fe41854b 100644
--- a/synapse/events/spamcheck.py
+++ b/synapse/events/spamcheck.py
@@ -109,6 +109,7 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"):
             if f is None:
                 return None
 
+            wrapped_func = f
             if f.__name__ == "check_registration_for_spam":
                 checker_args = inspect.signature(f)
                 if len(checker_args.parameters) == 3:
@@ -133,7 +134,7 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"):
                             request_info,
                         )
 
-                    f = wrapper
+                    wrapped_func = wrapper
                 elif len(checker_args.parameters) != 4:
                     raise RuntimeError(
                         "Bad signature for callback check_registration_for_spam",
@@ -143,9 +144,9 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"):
                 # We've already made sure f is not None above, but mypy doesn't do well
                 # across function boundaries so we need to tell it f is definitely not
                 # None.
-                assert f is not None
+                assert wrapped_func is not None
 
-                return maybe_awaitable(f(*args, **kwargs))
+                return maybe_awaitable(wrapped_func(*args, **kwargs))
 
             return run
 
@erikjohnston erikjohnston added S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Release-Blocker Must be resolved before making a release labels Jun 23, 2021
@babolivier babolivier linked a pull request Jun 23, 2021 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Release-Blocker Must be resolved before making a release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant