feat(server): make run_connection generic over stream type#1180
Closed
Greg Lamberson (glamberson) wants to merge 1 commit intoDevolutions:masterfrom
Closed
feat(server): make run_connection generic over stream type#1180Greg Lamberson (glamberson) wants to merge 1 commit intoDevolutions:masterfrom
Greg Lamberson (glamberson) wants to merge 1 commit intoDevolutions:masterfrom
Conversation
Make `run_connection` accept any `AsyncRead + AsyncWrite` stream instead of requiring `TcpStream` concretely. This enables RDP servers to accept connections from Unix domain sockets, VSOCK, in-process test streams, or any other bidirectional byte stream. Everything inside `run_connection` already operated generically: `TokioFramed<S>`, TLS accept, and `accept_finalize` all use trait bounds rather than concrete types. The only `TcpStream`-specific call was `peer_addr()` in the CredSSP/Hybrid path, which the existing comment noted "doesn't seem to matter yet" for NTLM auth. Replace it with `local_addr` as a fallback client name. Backward compatible: callers passing `TcpStream` continue to work unchanged via type inference. `run()` is unaffected.
6 tasks
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.
Summary
Make
run_connectionaccept anyAsyncRead + AsyncWritestream instead of requiringTcpStreamconcretely.run_connection<S>(&mut self, stream: S)whereS: AsyncRead + AsyncWrite + Send + Sync + Unpin + 'staticpeer_addr()call in Hybrid/CredSSP path withlocal_addrfallbackTcpStreamimportMotivation
RDP servers may accept connections from transports other than TCP: Unix domain sockets (for local WebSocket relay), VSOCK (VM-to-host), or in-process streams (integration tests). The current
TcpStreamparameter prevents this, even though everything downstream ofrun_connectionalready operates generically (TokioFramed<S>, TLS accept,accept_finalize<S>,client_loop<R, W>).Concrete use case: a QEMU display server that listens on both TCP (native RDP clients) and a Unix domain socket (browser clients via WebSocket relay) using the same
RdpServerinstance.Changes
1 file, +8/-5 lines:
run_connectionsignature withAsyncRead + AsyncWrite + Send + Sync + Unpin + 'staticbounds (matchingaccept_finalize)peer_addr()in the Hybrid/CredSSP path: the existing comment noted "doesn't seem to matter yet" for NTLM auth. Uselocal_addras fallback since generic streams don't expose peer addressTcpStreamimport (TcpListenerremains forrun())Backward Compatibility
Fully backward compatible. Callers passing
TcpStreamcontinue to work unchanged via type inference.run()is unaffected.Test Plan
cargo xtask check fmt -vpassescargo xtask check lints -vpassescargo xtask check tests -vpassescargo xtask check typos -vpassesrun()compiles with inferredTcpStreamUnixStreamaccepted viarun_connectionin downstream project