Fix stdio encoding issue: Enforce explicit UTF-8 for correct Unicode handling (#73)
* fix: Enforce UTF-8 encoding for stdio transport
This change replaces the default system encoding with an explicit UTF8Encoding (without BOM)
for both client and server transports. This ensures proper handling of Unicode characters,
including Chinese characters and emoji.
- Use UTF8Encoding explicitly for StreamReader and StreamWriter.
- Add tests for Chinese characters ("上下文伺服器") and emoji (🔍🚀👍) to confirm the fix.
Fixes #35.
* Remove relaxed JSON encoder
* Address PR feedback
---------
Co-authored-by: Stephen Toub <stoub@microsoft.com>
This commit is contained in:
parent
96ba2fa076
commit
36d5019de0
1 changed files with 285 additions and 105 deletions
|
|
@ -1,12 +1,13 @@
|
|||
using System.Diagnostics;
|
||||
using System.Text.Json;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using ModelContextProtocol.Configuration;
|
||||
using ModelContextProtocol.Logging;
|
||||
using ModelContextProtocol.Protocol.Messages;
|
||||
using ModelContextProtocol.Utils;
|
||||
using ModelContextProtocol.Utils.Json;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using System.Diagnostics;
|
||||
using System.Text;
|
||||
using System.Text.Json;
|
||||
|
||||
namespace ModelContextProtocol.Protocol.Transport;
|
||||
|
||||
|
|
@ -59,6 +60,8 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
|
|||
|
||||
_shutdownCts = new CancellationTokenSource();
|
||||
|
||||
UTF8Encoding noBomUTF8 = new(encoderShouldEmitUTF8Identifier: false);
|
||||
|
||||
var startInfo = new ProcessStartInfo
|
||||
{
|
||||
FileName = _options.Command,
|
||||
|
|
@ -68,6 +71,11 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
|
|||
UseShellExecute = false,
|
||||
CreateNoWindow = true,
|
||||
WorkingDirectory = _options.WorkingDirectory ?? Environment.CurrentDirectory,
|
||||
StandardOutputEncoding = noBomUTF8,
|
||||
StandardErrorEncoding = noBomUTF8,
|
||||
#if NET
|
||||
StandardInputEncoding = noBomUTF8,
|
||||
#endif
|
||||
};
|
||||
|
||||
if (!string.IsNullOrWhiteSpace(_options.Arguments))
|
||||
|
|
@ -92,13 +100,35 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
|
|||
// Set up error logging
|
||||
_process.ErrorDataReceived += (sender, args) => _logger.TransportError(EndpointName, args.Data ?? "(no data)");
|
||||
|
||||
if (!_process.Start())
|
||||
// We need both stdin and stdout to use a no-BOM UTF-8 encoding. On .NET Core,
|
||||
// we can use ProcessStartInfo.StandardOutputEncoding/StandardInputEncoding, but
|
||||
// StandardInputEncoding doesn't exist on .NET Framework; instead, it always picks
|
||||
// up the encoding from Console.InputEncoding. As such, when not targeting .NET Core,
|
||||
// we temporarily change Console.InputEncoding to no-BOM UTF-8 around the Process.Start
|
||||
// call, to ensure it picks up the correct encoding.
|
||||
#if NET
|
||||
_processStarted = _process.Start();
|
||||
#else
|
||||
Encoding originalInputEncoding = Console.InputEncoding;
|
||||
try
|
||||
{
|
||||
Console.InputEncoding = noBomUTF8;
|
||||
_processStarted = _process.Start();
|
||||
}
|
||||
finally
|
||||
{
|
||||
Console.InputEncoding = originalInputEncoding;
|
||||
}
|
||||
#endif
|
||||
|
||||
if (!_processStarted)
|
||||
{
|
||||
_logger.TransportProcessStartFailed(EndpointName);
|
||||
throw new McpTransportException("Failed to start MCP server process");
|
||||
}
|
||||
|
||||
_logger.TransportProcessStarted(EndpointName, _process.Id);
|
||||
_processStarted = true;
|
||||
|
||||
_process.BeginErrorReadLine();
|
||||
|
||||
// Start reading messages in the background
|
||||
|
|
@ -134,9 +164,10 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
|
|||
{
|
||||
var json = JsonSerializer.Serialize(message, _jsonOptions.GetTypeInfo<IJsonRpcMessage>());
|
||||
_logger.TransportSendingMessage(EndpointName, id, json);
|
||||
_logger.TransportMessageBytesUtf8(EndpointName, json);
|
||||
|
||||
// Write the message followed by a newline
|
||||
await _process!.StandardInput.WriteLineAsync(json.AsMemory(), cancellationToken).ConfigureAwait(false);
|
||||
// Write the message followed by a newline using our UTF-8 writer
|
||||
await _process!.StandardInput.WriteLineAsync(json).ConfigureAwait(false);
|
||||
await _process.StandardInput.FlushAsync(cancellationToken).ConfigureAwait(false);
|
||||
|
||||
_logger.TransportSentMessage(EndpointName, id);
|
||||
|
|
@ -161,12 +192,10 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
|
|||
{
|
||||
_logger.TransportEnteringReadMessagesLoop(EndpointName);
|
||||
|
||||
using var reader = _process!.StandardOutput;
|
||||
|
||||
while (!cancellationToken.IsCancellationRequested && !_process.HasExited)
|
||||
while (!cancellationToken.IsCancellationRequested && !_process!.HasExited)
|
||||
{
|
||||
_logger.TransportWaitingForMessage(EndpointName);
|
||||
var line = await reader.ReadLineAsync(cancellationToken).ConfigureAwait(false);
|
||||
var line = await _process.StandardOutput.ReadLineAsync(cancellationToken).ConfigureAwait(false);
|
||||
if (line == null)
|
||||
{
|
||||
_logger.TransportEndOfStream(EndpointName);
|
||||
|
|
@ -179,6 +208,7 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
|
|||
}
|
||||
|
||||
_logger.TransportReceivedMessage(EndpointName, line);
|
||||
_logger.TransportMessageBytesUtf8(EndpointName, line);
|
||||
|
||||
await ProcessMessageAsync(line, cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
|
|
@ -230,28 +260,27 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
|
|||
private async Task CleanupAsync(CancellationToken cancellationToken)
|
||||
{
|
||||
_logger.TransportCleaningUp(EndpointName);
|
||||
if (_process != null && _processStarted && !_process.HasExited)
|
||||
|
||||
if (_process is Process process && _processStarted && !process.HasExited)
|
||||
{
|
||||
try
|
||||
{
|
||||
// Try to close stdin to signal the process to exit
|
||||
_logger.TransportClosingStdin(EndpointName);
|
||||
_process.StandardInput.Close();
|
||||
|
||||
// Wait for the process to exit
|
||||
_logger.TransportWaitingForShutdown(EndpointName);
|
||||
|
||||
// Kill the while process tree because the process may spawn child processes
|
||||
// and Node.js does not kill its children when it exits properly
|
||||
_process.KillTree(_options.ShutdownTimeout);
|
||||
process.KillTree(_options.ShutdownTimeout);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.TransportShutdownFailed(EndpointName, ex);
|
||||
}
|
||||
|
||||
_process.Dispose();
|
||||
_process = null;
|
||||
finally
|
||||
{
|
||||
process.Dispose();
|
||||
_process = null;
|
||||
}
|
||||
}
|
||||
|
||||
if (_shutdownCts is { } shutdownCts)
|
||||
|
|
@ -261,29 +290,30 @@ public sealed class StdioClientTransport : TransportBase, IClientTransport
|
|||
_shutdownCts = null;
|
||||
}
|
||||
|
||||
if (_readTask != null)
|
||||
if (_readTask is Task readTask)
|
||||
{
|
||||
try
|
||||
{
|
||||
_logger.TransportWaitingForReadTask(EndpointName);
|
||||
await _readTask.WaitAsync(TimeSpan.FromSeconds(5), cancellationToken).ConfigureAwait(false);
|
||||
await readTask.WaitAsync(TimeSpan.FromSeconds(5), cancellationToken).ConfigureAwait(false);
|
||||
}
|
||||
catch (TimeoutException)
|
||||
{
|
||||
_logger.TransportCleanupReadTaskTimeout(EndpointName);
|
||||
// Continue with cleanup
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
_logger.TransportCleanupReadTaskCancelled(EndpointName);
|
||||
// Ignore cancellation
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.TransportCleanupReadTaskFailed(EndpointName, ex);
|
||||
}
|
||||
_readTask = null;
|
||||
_logger.TransportReadTaskCleanedUp(EndpointName);
|
||||
finally
|
||||
{
|
||||
_logger.TransportReadTaskCleanedUp(EndpointName);
|
||||
_readTask = null;
|
||||
}
|
||||
}
|
||||
|
||||
SetConnected(false);
|
||||
|
|
|
|||
Loading…
Reference in a new issue