Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TCP minor refactor #22

Open
colin-anderson opened this issue Feb 19, 2021 · 1 comment
Open

TCP minor refactor #22

colin-anderson opened this issue Feb 19, 2021 · 1 comment
Labels
refactor Refactor or tidy-up

Comments

@colin-anderson
Copy link
Contributor

In relation to PRs #16 & #19, SyslogTcpClient now contains a fair chunk of lower level code that deals with the vagaries of older .NET versions' sketchy TCP API support. It's possible that more code like this will be required in future, to support edge cases on different platforms, so to help keep SyslogTcpClient maintainable and approachable, some refactoring is needed.

  1. In relation to Call to AuthenticateAsClientAsync() Hangs When Server Doesn't Support TLS #18 and accompanying fix Implement Timeout for AuthenticateAsClientAsync #19, we should move this timeout wrapper to an SslStream extension method. That extension method should use the out-of-the-box AuthenticateAsClientAsync (with CancellationToken support) for .NET Core 2.1+ and .NET 5+, and fall back to the timeout wrapper only for net462 and netstandard2.0.

    This keeps us "standard" where possible, and will also result in simplifing SyslogTcpSink, keeping code at a higher level where it doesn't have to deal with the vagaries of older .NET versions' sketchy support.

  2. In relation to Server Listening on TCP IPv6 Does Not Receive Messages #13 and accompanying fix Support TCP IPv6 #16, we should also consider simplifying platform-dependant configuration of TcpClient for dual-mode IPv4/IPv6 support and buidling hostAddresses just once in the ctor (so it is always a single endpoint on non-Windows platforms, but may be a list on Windows). Probably just move the logic to a SyslogTcpClient private method:

    • If config.Host is already an IP address, just use it as-is. Otherwise:
    • On Windows: resolve 1..n IPs with Dns.GetHostAddresses, ensuring both IPv4 and IPv6 addresses are possible
    • On Linux: resolve only 1 IP with Dns.GetHostAddresses, ensuring both IPv4 and IPv6 addresses are possible (at present, if keep-alives are enabled on Linux, it will only resolve IPv4 IPs - now we support IPv6 connections, we should permit resolution of IPv6 addresses here too)
@colin-anderson colin-anderson added the refactor Refactor or tidy-up label Feb 19, 2021
@jakenuts
Copy link
Contributor

If it's relevant, in AWS Lambda IPV6 is not available so the initialization that hardcodes that protocol prevents the library from being used. Would be cool if the protocol was configurable or had a fallback if the "Address family not supported by protocol" exception is thrown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor or tidy-up
Projects
None yet
Development

No branches or pull requests

2 participants