Prevent Telnet clients reaching or stalling the application - #318
Draft
FreeAndNil wants to merge 3 commits into
Draft
Prevent Telnet clients reaching or stalling the application#318FreeAndNil wants to merge 3 commits into
FreeAndNil wants to merge 3 commits into
Conversation
- The appender streams the rendered log to any client that connects, with no authentication and no encryption, and defaulted to every interface on port 23. - It now defaults to 127.0.0.1. Watching from another machine is opt-in: set listenAddress to 0.0.0.0 or ::. Every documented example already used loopback. - SocketHandler(port, sendTimeoutMillis) defaults the same way. Deliberate default flip: a deployment relying on the implicit all-interfaces bind loses remote access until it sets listenAddress. audit da18b6f-f012
Three copies, two identical and one collecting into a joined string instead of a list. Now one internal helper beside SimpleTelnetClient and UdpMock, exposing Messages; the ANSI test asserts on that instead of its own Message property.
- Clients were written to serially under the appender lock, so one that stopped reading blocked every logging thread for sendTimeoutMillis. - Events are queued now, sendQueueSize 500, enqueueTimeoutMillis 50. - A full queue drops from the telnet stream, never from the log. - While the queue stays full, enqueueTimeoutMillis caps logging at 20 events/s; set it to 0 to drop immediately instead of waiting. audit da18b6f-f014
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
1 participant
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.
Two findings on
TelnetAppender, both about what a connected client can do tothe host application rather than to the log.
unauthenticated, and defaulted to every interface on port 23. It now defaults
to
127.0.0.1; remote access is opt-in vialistenAddress. Every documentedexample already used loopback.
stopped reading held up every thread that logs for
sendTimeoutMillis. Writesgo through a background thread now, bounded by
sendQueueSize(500), with alogging call waiting at most
enqueueTimeoutMillis(50) for room.Both change behaviour and need release notes:
listenAddressis set.queue stays full,
enqueueTimeoutMilliscaps logging at 20 events/s; set it to 0to drop immediately instead of waiting.
A third commit merges three copies of
RecordingErrorHandlerin the tests intoone helper, which is what the f014 test needed.