Skip to content
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

Validate visual parent on attaching to the tree #17049

Merged
merged 2 commits into from
Sep 18, 2024
Merged

Conversation

maxkatz6
Copy link
Member

As reported, it is possible to bypass "protected" method rules and manually raise AttachedToVisualTree event, without control being attached to the parent. It results in NRE.
While we shouldn't allow manual raise of AttachedToVisualTree event, we also should output more clear validation errors instead of NRE.

cc @jmacato and @MrJul

@maxkatz6 maxkatz6 added enhancement backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Sep 18, 2024
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0051934-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Copy link
Member

@MrJul MrJul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a real exception message is always better than an NRE.

I don't think we should really lock down anything here, as every system can be abused. (For example all routed events can be raised from anywhere and that can wreak havoc if misused.)

However, we could easily add a comment "Avalonia infrastructure - shouldn't be called by user code", as a warning to some methods.

@MrJul MrJul added this pull request to the merge queue Sep 18, 2024
Merged via the queue into master with commit 0aac1e4 Sep 18, 2024
11 checks passed
@MrJul MrJul deleted the visual-parent-validation branch September 18, 2024 08:42
@maxkatz6
Copy link
Member Author

@MrJul it should be doable with existing Avalonia.Analyzer, but the question is which methods to mark. [NotUserCallableAttibute] on every such method doesn't sound nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants