villebro commented on code in PR #43689:
URL: https://github.com/apache/superset/pull/43689#discussion_r3888520705
##########
superset/tasks/async_queries.py:
##########
@@ -125,6 +125,14 @@ def on_unsubscribe(
prefix = f"{principal}:"
return not any(existing.startswith(prefix) for existing in remaining)
+ def routing_channels(self, task: "CoreTask") -> list[str] | None:
+ # The consumer entries are exactly the per-tab realtime routing keys
+ # (`"<principal>:<tab_id>"` -> `realtime:<principal>:<tab_id>`), so a
+ # task-status message reaches only the tabs watching this task. Empty
->
+ # None so a chart task with no recorded tab (all detached, or a no-tab
+ # caller) falls back to principal-grain fanout instead of dropping it.
+ return self._consumers(task) or None
Review Comment:
Good catch — fixed in 78ec946530. `on_unsubscribe` now clears **all** of the
principal's tab entries on the principal-grain (no-`tab_id`) unsubscribe path,
so a later status transition can no longer route to that principal's tab
channels after it has left; the per-tab (`client_ref`) path is unchanged. Added
a regression test
(`test_consumer_policy_no_client_ref_clears_that_principals_tab_entries`).
Note on scope: even before the fix this was stale delivery to the *same*
principal's own JWT-bound socket, not a cross-principal leak (a `user:5:<tab>`
key only ever routes to user 5's socket) — but delivering to a principal that
explicitly unsubscribed is still wrong, so the fix stands.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]