fix: malformed packet crash #889
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This prevents possible bugs from forgetting to remove the player from the player reigstry when calling shutdown. It also removes the player from the positions map.
Using entity.destruct to disconnect players leads to an abort because, when the server receives PlayerDisconnect from the proxy, the server will try to set PendingRemove on the already-destructed entity, causing an abort. Sending DisconnectS2c was removed because it is not possible to send packets to disconnected players.
The channel is closed which makes the recv loop end. Therefore, sending a shutdown message is not needed.
Benchmark Results for generalComparing to 4c402d0 |
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #889 +/- ##
==========================================
- Coverage 20.85% 20.85% -0.01%
==========================================
Files 161 161
Lines 16867 16864 -3
Branches 468 463 -5
==========================================
- Hits 3518 3517 -1
+ Misses 13285 13283 -2
Partials 64 64
🚀 New features to boost your workflow:
|
andrewgazelka
requested changes
Apr 27, 2025
| #[derive(Component, Debug)] | ||
| pub struct PendingRemove { | ||
| pub reason: String, | ||
| _private: u8, |
There was a problem hiding this comment.
what is the point of this _private field? why can't this just be a ZST
There was a problem hiding this comment.
Flecs doesn't allow zero sized components. The _private field also prevents other modules from constructing PendingRemove since it should only be sent by the PlayerDisconnect proxy message.
There was a problem hiding this comment.
Flecs doesn't allow zero sized components. The _private field also prevents other modules from constructing PendingRemove since it should only be sent by the PlayerDisconnect proxy message.
when you use .with I believe flecs does. I have definitely used ZSTs before.
There was a problem hiding this comment.
If I change it to
#[derive(Component, Debug)]
pub struct PendingRemove;I get the following error:
error[E0277]: the size of type `PendingRemove` should not be zero, should not be a tag.
--> crates/hyperion/src/ingress/mod.rs:336:47
|
336 | world.entity_from_id(*id).set(PendingRemove);
| --- ^^^^^^^^^^^^^ Supports only non-empty components
| |
| required by a bound introduced by this call
|
= help: the trait `flecs_ecs::core::DataComponent` is not implemented for `PendingRemove`
= help: the following other types implement trait `flecs_ecs::core::DataComponent`:
&T
&mut T
(T, U)
ActiveAnimation
AdditionalHearts
AirSupply
AppId
ArrowsInEntity
and 137 others
note: required by a bound in `flecs_ecs::core::entity_view::entity_view_mut::<impl flecs_ecs::core::EntityView<'a>>::set`
--> /home/remote-dev/.cargo/git/checkouts/flecs-rust-182003fcd2303c9b/f2762ad/flecs_ecs/src/core/entity_view/entity_view_mut.rs:860:33
|
860 | pub fn set<T: ComponentId + DataComponent>(self, component: T) -> Self {
| ^^^^^^^^^^^^^ required by this bound in `flecs_ecs::core::entity_view::entity_view_mut::<impl EntityView<'a>>::set`
Benchmark Results for generalComparing to 4c402d0 |
Benchmark Results for generalComparing to 4c402d0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a client sends a malformed packet, the server will crash because it used entity.destruct to disconnect players for malformed packets which leads to an abort because, when the server receives PlayerDisconnect from the proxy, the server will try to set PendingRemove on the already-destructed entity.
This PR fixes these errors on player disconnects.