Skip to content

[Security] Resolve event bubbling of logout + new events in a compiler pass #37368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Jun 20, 2020

Q A
Branch? 5.1
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #37292
License MIT
Doc PR -

This PR proposes to create a compiler pass that registers listeners on the main event_dispatcher on the firewall-specific event dispatcher during compile time. This allows to still specify listener priorities while listening on a bubbled-up event (instead of a fix moment where the event bubbling occurs). It probably also improves performance, as it doesn't use duplicated event dispatching logic to provide event bubbling.

Nothing changes on the user side. I proposed this as a bugfix, as it fixes the bug mentioned in #37292 (not being able to use listener priorities). I did remove a class, which was introduced in 5.1 and is very internal. I think it's safe, but we can also keep it and remove in master.

@wouterj wouterj force-pushed the issue-37292/event-bubbling-in-container-pass branch 2 times, most recently from 0d0e51f to 7649d0f Compare June 20, 2020 22:55
* This removes duplicate event dispatching logic on event bubbling, which
  probably improves performance.
* It allows to still specify listener priorities while listening on a
  bubbled-up event (instead of a fix moment where the event bubbling occurs)
@wouterj wouterj force-pushed the issue-37292/event-bubbling-in-container-pass branch from 7649d0f to f962c26 Compare June 20, 2020 23:27
@fabpot
Copy link
Member

fabpot commented Jun 23, 2020

Thank you @wouterj.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants