Skip to content

feat(SocketClientOptions): add EnableLog parameter #6371

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 7, 2025
Merged

feat(SocketClientOptions): add EnableLog parameter #6371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
43b7b88
feat(SocketClientOptions): add EnableLog parameter
ArgoZhang Jul 7, 2025
e4a1be1
test: 增加单元测试
ArgoZhang Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/BootstrapBlazor/Services/TcpSocket/SocketClientOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,9 @@ public class SocketClientOptions
/// </summary>
/// <remarks>This property specifies the local network endpoint that the socket client will bind to when establishing a connection.</remarks>
public IPEndPoint LocalEndPoint { get; set; } = new IPEndPoint(IPAddress.Any, 0);

/// <summary>
/// Gets or sets a value indicating whether logging is enabled. Default value is false.
/// </summary>
public bool EnableLog { get; set; }
}
10 changes: 10 additions & 0 deletions src/BootstrapBlazor/Services/TcpSocket/TcpSocketClientBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ public virtual async ValueTask<bool> SendAsync(ReadOnlyMemory<byte> data, Cancel
{
Log(LogLevel.Error, ex, $"TCP Socket send failed from {_localEndPoint} to {_remoteEndPoint}");
}

if (options.EnableLog)
{
Log(LogLevel.Information, null, $"Sending data from {_localEndPoint} to {_remoteEndPoint}, Data Length: {data.Length} Data Content: {BitConverter.ToString(data.ToArray())} Result: {ret}");
Comment on lines +169 to +171
Copy link
Contributor

@sourcery-ai sourcery-ai bot Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Logging raw data content may expose sensitive information.

Redact or limit logged data content, or make logging configurable to avoid exposing sensitive information.

Copy link
Contributor

@sourcery-ai sourcery-ai bot Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (performance): Converting ReadOnlyMemory to array may impact performance.

ToArray() allocates and copies data, which can be costly for large or frequent logs. Consider limiting logged data size or using a more efficient method.

Suggested implementation:

        if (options.EnableLog)
        {
            const int MaxLogBytes = 64;
            int logLength = Math.Min(data.Length, MaxLogBytes);
            string dataContent;
            if (logLength > 0)
            {
                var span = data.Span.Slice(0, logLength);
                dataContent = BitConverter.ToString(span.ToArray());
                if (data.Length > MaxLogBytes)
                {
                    dataContent += "...(truncated)";
                }
            }
            else
            {
                dataContent = string.Empty;
            }
            Log(LogLevel.Information, null, $"Sending data from {_localEndPoint} to {_remoteEndPoint}, Data Length: {data.Length} Data Content: {dataContent} Result: {ret}");
        }
        return ret;
        if (options.EnableLog)
        {
            const int MaxLogBytes = 64;
            int logLength = Math.Min(len, MaxLogBytes);
            string dataContent;
            if (logLength > 0)
            {
                var span = buffer.Span.Slice(0, logLength);
                dataContent = BitConverter.ToString(span.ToArray());
                if (len > MaxLogBytes)
                {
                    dataContent += "...(truncated)";
                }
            }
            else
            {
                dataContent = string.Empty;
            }
            Log(LogLevel.Information, null, $"Receiving data from {_localEndPoint} to {_remoteEndPoint}, Data Length: {len} Data Content: {dataContent}");
        }

}
return ret;
}

Expand Down Expand Up @@ -258,6 +263,11 @@ private async ValueTask<int> ReceiveCoreAsync(ISocketClientProvider client, Memo
{
Log(LogLevel.Error, ex, $"TCP Socket receive failed from {_localEndPoint} to {_remoteEndPoint}");
}

if (options.EnableLog)
{
Log(LogLevel.Information, null, $"Receiving data from {_localEndPoint} to {_remoteEndPoint}, Data Length: {len} Data Content: {BitConverter.ToString(buffer.ToArray())}");
}
return len;
Comment on lines +267 to 271
Copy link
Contributor

@sourcery-ai sourcery-ai bot Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (performance): Repeated ToArray() calls on buffer may be inefficient.

For large or frequently used buffers, this conversion can impact performance. Consider logging a subset of the buffer or a more efficient format.

Suggested change
if (options.EnableLog)
{
Log(LogLevel.Information, null, $"Receiving data from {_localEndPoint} to {_remoteEndPoint}, Data Length: {len} Data Content: {BitConverter.ToString(buffer.ToArray())}");
}
return len;
if (options.EnableLog)
{
const int maxLogBytes = 32;
var logBytes = buffer.Count > maxLogBytes ? buffer.Slice(0, maxLogBytes).ToArray() : buffer.ToArray();
var dataContent = BitConverter.ToString(logBytes);
if (buffer.Count > maxLogBytes)
{
dataContent += $"... (truncated, total {buffer.Count} bytes)";
}
Log(LogLevel.Information, null, $"Receiving data from {_localEndPoint} to {_remoteEndPoint}, Data Length: {len} Data Content: {dataContent}");
}
return len;

}

Expand Down
6 changes: 5 additions & 1 deletion test/UnitTest/Services/TcpSocketFactoryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,11 @@ private static ITcpSocketClient CreateClient(Action<ServiceCollection>? builder

var provider = sc.BuildServiceProvider();
var factory = provider.GetRequiredService<ITcpSocketFactory>();
var client = factory.GetOrCreate("test", op => op.LocalEndPoint = Utility.ConvertToIpEndPoint("localhost", 0));
var client = factory.GetOrCreate("test", op =>
{
op.LocalEndPoint = Utility.ConvertToIpEndPoint("localhost", 0);
op.EnableLog = true;
Comment on lines +646 to +649
Copy link
Contributor

@sourcery-ai sourcery-ai bot Jul 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): No test coverage for EnableLog=false scenario.

Please add a test to cover the default EnableLog=false case and confirm that logging is not triggered.

Suggested implementation:

        var provider = sc.BuildServiceProvider();
        var factory = provider.GetRequiredService<ITcpSocketFactory>();
        var client = factory.GetOrCreate("test", op =>
        {
            op.LocalEndPoint = Utility.ConvertToIpEndPoint("localhost", 0);
            op.EnableLog = true;
        });
        return client;
    }

    [Fact]
    public void GetOrCreate_DefaultEnableLogFalse_DoesNotTriggerLogging()
    {
        // Arrange
        var sc = new ServiceCollection();
        // Register ITcpSocketFactory and any required dependencies, including a mock logger if needed
        var mockLogger = new Mock<ILogger>();
        sc.AddSingleton<ILogger>(mockLogger.Object);
        sc.AddSingleton<ITcpSocketFactory, TcpSocketFactory>();
        var provider = sc.BuildServiceProvider();
        var factory = provider.GetRequiredService<ITcpSocketFactory>();

        // Act
        var client = factory.GetOrCreate("test-default", op =>
        {
            op.LocalEndPoint = Utility.ConvertToIpEndPoint("localhost", 0);
            // Do not set EnableLog, should default to false
        });

        // Assert
        // Check that logging was not triggered
        mockLogger.Verify(
            logger => logger.Log(
                It.IsAny<LogLevel>(),
                It.IsAny<EventId>(),
                It.IsAny<It.IsAnyType>(),
                It.IsAny<Exception>(),
                (Func<It.IsAnyType, Exception, string>)It.IsAny<object>()),
            Times.Never
        );
    }
  • If your project uses a different logging abstraction or does not use dependency injection for the logger, you may need to adjust the mocking and registration code accordingly.
  • If TcpSocketFactory does not use ILogger, or logging is handled differently, adapt the assertion to match your actual logging mechanism.
  • Ensure you have the necessary using directives at the top of the file: 
    using Xunit;
using Moq;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
  • If you do not have Moq installed, add it to your test project.

});
return client;
}

Expand Down
Loading