Spam messages shouldn't be sent to clients

[turt2live]

This is apparently desired behaviour according to the following, however that feels like a mistake. It should probably just not send it to users/appservices.

synapse/synapse/events/spamcheck.py

Lines 34 to 36 in f009df2

	If the server considers an event spammy, then it will be rejected if
	sent by a local user. If it is sent by a user on another server, then
	users receive a blank event.

[turt2live]

for perspective, this is what is seen in clients ([e] TravisR being a remote spammer):

[neilisfragile]

I've tagged it as a feature even though it is not really (more a change from current expected behaviour) - agreed that blocking all makes most sense UX wise

[richvdh]

I'm a bit surprised this is still the case. The comment is still present at https://github.com/matrix-org/synapse/blob/0171fa5/synapse/events/spamcheck.py#L248 implying it is, though.

[H-Shay]

Looking at the code starting here

synapse/synapse/federation/federation_base.py

Line 101 in 0326888

result = await self.spam_checker.check_event_for_spam(pdu)

it appears that we redact and soft-fail spammy events received over federation. My understanding of the spec is that soft-failed events are most likely not passed on to clients (although I think that's an implementation detail?). If this is the case I can change the misleading comment.

[turt2live]

it is an implementation detail, somewhat surprisingly. Spec has this to say:

However, servers may choose not to send such events on to their clients, so that end users won’t actually see the events.

[richvdh]

Pretty sure that the implementation in synapse does not pass on soft-failed events.

The thing is, that's always been the case, so I wonder what has changed, if anything, between the time that comment was written and now. It's possible that we didn't soft-fail incoming spam events at the time?

[H-Shay]

Well it looks like up until about 8 months ago Synapse just redacted the events and sent them on. The code to soft-fail the spammy events was added 8 months ago (by Rich, I might add 🙂 ). Either way the comment is definitely wrong and I can fix it, and I think that means we can close this issue as well?

[richvdh]

hurrah, sounds great!

do you have a link to that change, out of interest?

[H-Shay]

I think this is what you are asking for? https://github.com/matrix-org/synapse/pull/10263/files.

[richvdh]

yes it is, thank you!