[core] Deprecate PacketGuard, refactor C2S by sruon · Pull Request #9796 · LandSandBoat/server

sruon · 2026-04-15T08:48:42Z

I affirm:

I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
I have read and understood the Contributing Guide and the Code of Conduct.
I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

Size checks are enforced for all packets in the handlers and automatically determined at compile time from structures (replaces packet_system.cpp hand-rolled sizes)
- Uses a slightly modified macro for packets with VLA
Adds missing padding to a handful of packets
Introduce a flag-driven BlockedState mechanism for packet validation
- Replaces PacketGuard cutscene check and a whole lot of existing validation methods
Rate limit values loaded from settings and handled by PacketSystem
- Replaces PacketGuard rate limits
PacketParser global array is no longer accessible
- Use OnIncomingPacket in your modules
PacketValidation now takes PChar since it's used by several validators
PacketValidation now short-circuits on all checks
Aligned on this-> everywhere

Steps to test these changes

Need to test size some more and uplift several modules:

AH announcement
Renamer (may have to drop that one unless we want to make the concept of custom packets first class)

github-actions · 2026-04-15T08:48:53Z

✨ Hello and thanks for the PR! ✨

🤖: This is a friendly automated reminder that the maintainers won't look at your PR until you've properly completed all of the checkboxes in the pre-filled template.

zach2good

Glorious!

zach2good · 2026-04-15T09:07:58Z

+    -- Packets not listed here have no rate limit.
+    -- You should rate limit any packet that a player can send at will that results in
+    -- an immediate database hit or generates logs or results in file or network io.
+    PACKET_RATE_LIMITS =


I would probably push this to it's own settings file, or into network - your choice (or do neither)

zach2good · 2026-04-15T09:08:56Z

+#include "magic_enum/magic_enum_containers.hpp"
+
+using namespace magic_enum::bitwise_operators;


Do these have to live down here? Could/should we have a general enums.h header which self-includes the other actual enum files, and then does this include voodoo at the end?

I'll leave it like that for the time being but I think enums.h is a good idea for a future refactor.

zach2good · 2026-04-15T09:09:19Z

    // Not implemented.
-    return PacketValidator();
+    return PacketValidator(PChar)
+        .blockedBy({ BlockedState::InEvent });


zach2good · 2026-04-15T09:09:56Z

-    return PacketValidator()
+    return PacketValidator(PChar)
        .oneOf<GP_CLI_COMMAND_ACTION_ACTIONID>(this->ActionID)
        .custom([&](PacketValidator& pv)


If you put the lambda on a new line, you can redule the indentation a bit:

.custom( [](...

zach2good · 2026-04-15T09:10:06Z

-                                .isNotFishing(PChar) // Note: It is possible to attack while fishing on retail and is disabled here on purpose.
-                                .isNotPreventedAction(PChar);
+                            // Note: It is possible to attack while fishing on retail and is disabled here on purpose.
+                            pv.blockedBy({ BlockedState::Healing, BlockedState::Sitting, BlockedState::Crafting, BlockedState::Fishing, BlockedState::PreventAction });


zach2good · 2026-04-15T09:10:58Z

        SmallPD_Type = (ref<uint16>(SmallPD_ptr, 0) & 0x1FF);

-        if (PacketSize[SmallPD_Type] == SmallPD_Size || PacketSize[SmallPD_Type] == 0) // Tests incoming packets for the correct size prior to processing
+        if ((ref<uint16>(SmallPD_ptr, 2) <= PSession->client_packet_id) || (ref<uint16>(SmallPD_ptr, 2) > SmallPD_Code))


Loving the reduction in this file 👍

zach2good · 2026-04-15T09:14:05Z

+    }
+
+    auto       timeNow            = timer::now();
+    const auto lastPacketReceived = PChar->m_PacketRecievedTimestamps.emplace(packetId, timeNow);


Use structured bindings here

zach2good · 2026-04-15T09:14:53Z


-void PacketParserInitialize()
+// cppcheck-suppress templateRecursion
+consteval auto buildPacketHandlers() -> std::array<PacketHandler, 512>


- Size checks are enforced for all packets in the handlers and automatically determined from structures - Adds missing padding to a handful of packets - Introduce a flag-driven BlockedState mechanism for packet validation - Rate limit values loaded from settings and handled by PacketSystem - PacketHandlers global array is no longer accessible

sruon · 2026-04-16T02:59:41Z

Dropped 'renamer' module entirely. Don't know anyone using it and it doesn't fit in with the enforced validation mechanisms.
- I can probably support fake packets as a first class concept but not sure it's worth the hassle right now
Uplifted ah_announcement module to new OnIncomingPacket model. Tested.
Dropped rate limit settings from the lua and hardcoded them into the class itself for the time being.
- Settings class need a little bit of rework to support arbitrary tables which I will do separately.
Retested packets where I changed paddings
Retested rate limit (/jump spam)
Retested packets blocked during cutscene
Added a cppcheck exception because it's not happy about the 100x template calls in packet_system

Should be good to go after another review

sruon force-pushed the packet-guard-deprecation branch from 82bcc7f to af90c7f Compare April 15, 2026 08:52

zach2good approved these changes Apr 15, 2026

View reviewed changes

sruon force-pushed the packet-guard-deprecation branch from af90c7f to 8267b4f Compare April 16, 2026 02:51

sruon marked this pull request as ready for review April 16, 2026 02:58

Xaver-DaRed reviewed Apr 16, 2026

View reviewed changes

Comment thread src/map/enums/blocked_state.h

Xaver-DaRed merged commit a3f2e28 into LandSandBoat:base Apr 16, 2026
10 checks passed

		#include "magic_enum/magic_enum_containers.hpp"

		using namespace magic_enum::bitwise_operators;

Conversation

sruon commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this pull request do?

Steps to test these changes

Uh oh!

github-actions Bot commented Apr 15, 2026

Uh oh!

zach2good left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sruon commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sruon commented Apr 15, 2026 •

edited

Loading

sruon commented Apr 16, 2026 •

edited

Loading