forked from MirrorHub/synapse
Comments and interface cleanup for on_receive_pdu
Add some informative comments about what's going on here. Also, `sent_to_us_directly` and `get_missing` were doing the same thing (apart from in `_handle_queued_pdus`, which looks like a bug), so let's get rid of `get_missing` and use `sent_to_us_directly` consistently.
This commit is contained in:
Richard van der Hoff
parent
1f3f5fcf52
commit
703de4ec13
2 changed files with 47 additions and 24 deletions
|
|
@ -618,7 +618,7 @@ class FederationServer(FederationBase):
|
|||
)
|
||||
|
||||
yield self.handler.on_receive_pdu(
|
||||
origin, pdu, get_missing=True, sent_to_us_directly=True,
|
||||
origin, pdu, sent_to_us_directly=True,
|
||||
)
|
||||
|
||||
def __str__(self):
|
||||
|
|
|
|||
|
|
@ -136,7 +136,7 @@ class FederationHandler(BaseHandler):
|
|||
|
||||
@defer.inlineCallbacks
|
||||
def on_receive_pdu(
|
||||
self, origin, pdu, get_missing=True, sent_to_us_directly=False,
|
||||
self, origin, pdu, sent_to_us_directly=False,
|
||||
):
|
||||
""" Process a PDU received via a federation /send/ transaction, or
|
||||
via backfill of missing prev_events
|
||||
|
|
@ -145,7 +145,8 @@ class FederationHandler(BaseHandler):
|
|||
origin (str): server which initiated the /send/ transaction. Will
|
||||
be used to fetch missing events or state.
|
||||
pdu (FrozenEvent): received PDU
|
||||
get_missing (bool): True if we should fetch missing prev_events
|
||||
sent_to_us_directly (bool): True if this event was pushed to us; False if
|
||||
we pulled it as the result of a missing prev_event.
|
||||
|
||||
Returns (Deferred): completes with None
|
||||
"""
|
||||
|
|
@ -250,7 +251,7 @@ class FederationHandler(BaseHandler):
|
|||
pdu.internal_metadata.outlier = True
|
||||
elif min_depth and pdu.depth > min_depth:
|
||||
missing_prevs = prevs - seen
|
||||
if get_missing and missing_prevs:
|
||||
if sent_to_us_directly and missing_prevs:
|
||||
# If we're missing stuff, ensure we only fetch stuff one
|
||||
# at a time.
|
||||
logger.info(
|
||||
|
|
@ -282,24 +283,46 @@ class FederationHandler(BaseHandler):
|
|||
room_id, event_id, len(missing_prevs), shortstr(missing_prevs),
|
||||
)
|
||||
|
||||
if sent_to_us_directly and prevs - seen:
|
||||
# If they have sent it to us directly, and the server
|
||||
# isn't telling us about the auth events that it's
|
||||
# made a message referencing, we explode
|
||||
logger.warn(
|
||||
"[%s %s] Failed to fetch %d prev events: rejecting",
|
||||
room_id, event_id, len(prevs - seen),
|
||||
)
|
||||
raise FederationError(
|
||||
"ERROR",
|
||||
403,
|
||||
(
|
||||
"Your server isn't divulging details about prev_events "
|
||||
"referenced in this event."
|
||||
),
|
||||
affected=pdu.event_id,
|
||||
)
|
||||
elif prevs - seen:
|
||||
if prevs - seen:
|
||||
# We've still not been able to get all of the prev_events for this event.
|
||||
#
|
||||
# In this case, we need to fall back to asking another server in the
|
||||
# federation for the state at this event. That's ok provided we then
|
||||
# resolve the state against other bits of the DAG before using it (which
|
||||
# will ensure that you can't just take over a room by sending an event,
|
||||
# withholding its prev_events, and declaring yourself to be an admin in
|
||||
# the subsequent state request).
|
||||
#
|
||||
# Now, if we're pulling this event as a missing prev_event, then clearly
|
||||
# this event is not going to become the only forward-extremity and we are
|
||||
# guaranteed to resolve its state against our existing forward
|
||||
# extremities, so that should be fine.
|
||||
#
|
||||
# On the other hand, if this event was pushed to us, it is possible for
|
||||
# it to become the only forward-extremity in the room, and we would then
|
||||
# trust its state to be the state for the whole room. This is very bad.
|
||||
# Further, if the event was pushed to us, there is no excuse for us not to
|
||||
# have all the prev_events. We therefore reject any such events.
|
||||
#
|
||||
# XXX this really feels like it could/should be merged with the above,
|
||||
# but there is an interaction with min_depth that I'm not really
|
||||
# following.
|
||||
|
||||
if sent_to_us_directly:
|
||||
logger.warn(
|
||||
"[%s %s] Failed to fetch %d prev events: rejecting",
|
||||
room_id, event_id, len(prevs - seen),
|
||||
)
|
||||
raise FederationError(
|
||||
"ERROR",
|
||||
403,
|
||||
(
|
||||
"Your server isn't divulging details about prev_events "
|
||||
"referenced in this event."
|
||||
),
|
||||
affected=pdu.event_id,
|
||||
)
|
||||
|
||||
# Calculate the state of the previous events, and
|
||||
# de-conflict them to find the current state.
|
||||
state_groups = []
|
||||
|
|
@ -464,7 +487,7 @@ class FederationHandler(BaseHandler):
|
|||
yield self.on_receive_pdu(
|
||||
origin,
|
||||
ev,
|
||||
get_missing=False
|
||||
sent_to_us_directly=False,
|
||||
)
|
||||
except FederationError as e:
|
||||
if e.code == 403:
|
||||
|
|
@ -1112,7 +1135,7 @@ class FederationHandler(BaseHandler):
|
|||
try:
|
||||
logger.info("Processing queued PDU %s which was received "
|
||||
"while we were joining %s", p.event_id, p.room_id)
|
||||
yield self.on_receive_pdu(origin, p)
|
||||
yield self.on_receive_pdu(origin, p, sent_to_us_directly=True)
|
||||
except Exception as e:
|
||||
logger.warn(
|
||||
"Error handling queued PDU %s from %s: %s",
|
||||
|
|
|
|||
Loading…
Reference in a new issue