fix(SyslogUdp): allow configuring the max UDP datagram length - #2049
Open
nabsei wants to merge 1 commit into
Open
fix(SyslogUdp): allow configuring the max UDP datagram length#2049nabsei wants to merge 1 commit into
nabsei wants to merge 1 commit into
Conversation
UdpSocket::DATAGRAM_MAX_LENGTH (65023 bytes) is the largest a UDP payload can theoretically be, but a datagram that size gets fragmented at the IP level, and many routers/firewalls drop fragmented UDP packets outright. That silently truncates or loses log messages that exceed the path MTU on the way to the syslog receiver, even though nothing on the sending side reports an error. Add an optional $maxLength constructor parameter to UdpSocket and thread it through SyslogUdpHandler, so a value that fits within the path MTU (eg. 1024) can be used to avoid fragmentation. The default is unchanged, so this is fully backwards compatible. Fixes Seldaek#1826 Co-authored-by: Claude <noreply@anthropic.com>
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
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.
What
UdpSocket::DATAGRAM_MAX_LENGTH(65023 bytes) is the largest a UDP payload can theoretically be, but a datagram that size gets fragmented at the IP level, and many routers/firewalls drop fragmented UDP packets outright. That silently truncates or loses log messages that exceed the path MTU on the way to the syslog receiver, even though nothing on the sending side reports an error.Why this approach
An existing test (
testLongMessagesAreTruncated) explicitly locks in the current 65023-byte behavior, and some setups over trusted/local networks may rely on it, so I didn't want to just lower the default and risk a behavior change / BC break. Instead, this adds an optional$maxLengthconstructor parameter toUdpSocket, threaded throughSyslogUdpHandler's constructor (the entry point most users actually use), so a value that fits within the path MTU (eg. 1024) can be opted into. The default is unchanged.Verification
$maxLengthtruncates at the new length, the default is preserved when not specified, andSyslogUdpHandlercorrectly threads the value through to its internalUdpSocket(verified via reflection, since$socketisn't otherwise inspectable from outside).JsonFormatterTest/NormalizerFormatterTest/ErrorHandlerTest(confirmed identical on unmodifiedmain), plus 30 skips for missing optional extensions (mongodb, redis, etc.) unrelated to this change.phpstan analyseis clean on the changed files.Honest caveat: I could not verify the actual real-world symptom (fragmented UDP getting dropped by a router/firewall) with a live network reproduction — that's not something reproducible over loopback, which doesn't share the same MTU/fragmentation-handling constraints as a real network path. Verification here is limited to code review and the unit tests described above.
Related issue
Fixes #1826
Disclosure: this PR was prepared with the assistance of Claude Code (Anthropic). The investigation, fix, and verification steps described above were done and checked by me.