Skip to content

Commit

Permalink
Fix: bug #172 - Incorrect encoding conversion of just send 8 messages. (
Browse files Browse the repository at this point in the history
#173)

* Fix bug #172 by reading message data in a byte-wise manner rather than decoding to text and re-endcoding to bytes for the resultant message.
  • Loading branch information
rnwood authored Mar 7, 2024
1 parent 34e3e99 commit 2a07568
Show file tree
Hide file tree
Showing 13 changed files with 116 additions and 44 deletions.
17 changes: 9 additions & 8 deletions smtpserver/.gitignore
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
.vs
obj
bin
*.ncrunch*
packages
_ncrunch_*
*.user
/Rnwood.Smtp4dev/node_modules
.vs
obj
bin
*.ncrunch*
packages
_ncrunch_*
*.user
/Rnwood.Smtp4dev/node_modules
.idea
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

<PropertyGroup>
<VersionPrefix>3.0</VersionPrefix>
<TargetFrameworks>netcoreapp2.0;net462</TargetFrameworks>
<TargetFrameworks>net8.0;net462</TargetFrameworks>
<AssemblyName>Rnwood.SmtpServer.Tests</AssemblyName>
<OutputType>Library</OutputType>

<PackageId>Rnwood.SmtpServer.Tests</PackageId>
<GenerateRuntimeConfigurationFiles>true</GenerateRuntimeConfigurationFiles>
<RuntimeFrameworkVersion Condition=" '$(TargetFramework)' == 'netcoreapp2.0' ">2.0.0</RuntimeFrameworkVersion>
<RuntimeFrameworkVersion Condition=" '$(TargetFramework)' == 'net8.0' ">8.0.0</RuntimeFrameworkVersion>
<GenerateAssemblyTitleAttribute>false</GenerateAssemblyTitleAttribute>
<GenerateAssemblyDescriptionAttribute>false</GenerateAssemblyDescriptionAttribute>
<GenerateAssemblyConfigurationAttribute>false</GenerateAssemblyConfigurationAttribute>
Expand Down Expand Up @@ -35,7 +36,7 @@
<AdditionalFiles Include="../stylecop.json" />
</ItemGroup>

<ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'net462' ">
<Reference Include="mscorlib">
<HintPath>..\..\..\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.2\mscorlib.dll</HintPath>
</Reference>
Expand Down
16 changes: 14 additions & 2 deletions smtpserver/Rnwood.SmtpServer.Tests/SmtpStreamReaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ await Test("MAIL FROM <robért@rnwood.co.uk>\r\n", Encoding.GetEncoding("iso-885
"MAIL FROM <robért@rnwood.co.uk>"
});
}

[Fact]
public async Task ReadLine_WithAnsiChars()
{
await Test("MAIL FROM <quáestionem@rnwood.co.uk>\r\n", Encoding.GetEncoding("iso-8859-1"), new[]
{
"MAIL FROM <quáestionem@rnwood.co.uk>"
});
}

[Fact]
public async Task ReadLine_WithUtf8Chars()
Expand Down Expand Up @@ -54,12 +63,15 @@ private async Task Test(string data, Encoding encoding, string[] expectedLines)
List<string> receivedLines = new List<string>();

string receivedLine;

while ((receivedLine = await ssr.ReadLineAsync(cts.Token)) != null)
{
receivedLines.Add(receivedLine);
}

Assert.Equal(expectedLines, receivedLines.ToArray());

byte[] receivedBytes = encoding.GetBytes(string.Join("\r\n", receivedLines));
byte[] expectedBytes = encoding.GetBytes(string.Join("\r\n", expectedLines));
Assert.Equal(expectedBytes, receivedBytes);
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions smtpserver/Rnwood.SmtpServer.Tests/Verbs/DataVerbTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public async Task Data_AboveSizeLimit_Rejected()

string[] messageData = new string[] { new string('x', 11), "." };
int messageLine = 0;
mocks.Connection.Setup(c => c.ReadLine()).Returns(() => Task.FromResult(messageData[messageLine++]));
mocks.Connection.Setup(c => c.ReadLineBytes()).Returns(() => Task.FromResult(Encoding.ASCII.GetBytes(messageData[messageLine++])));

DataVerb verb = new DataVerb();
await verb.Process(mocks.Connection.Object, new SmtpCommand("DATA")).ConfigureAwait(false);
Expand Down Expand Up @@ -88,7 +88,7 @@ public async Task Data_ExactlySizeLimit_Accepted()

string[] messageData = new string[] { new string('x', 10), "." };
int messageLine = 0;
mocks.Connection.Setup(c => c.ReadLine()).Returns(() => Task.FromResult(messageData[messageLine++]));
mocks.Connection.Setup(c => c.ReadLineBytes()).Returns(() => Task.FromResult(Encoding.UTF8.GetBytes(messageData[messageLine++])));

DataVerb verb = new DataVerb();
await verb.Process(mocks.Connection.Object, new SmtpCommand("DATA")).ConfigureAwait(false);
Expand Down Expand Up @@ -127,7 +127,7 @@ public async Task Data_WithinSizeLimit_Accepted()

string[] messageData = new string[] { new string('x', 9), "." };
int messageLine = 0;
mocks.Connection.Setup(c => c.ReadLine()).Returns(() => Task.FromResult(messageData[messageLine++]));
mocks.Connection.Setup(c => c.ReadLineBytes()).Returns(() => Task.FromResult(Encoding.UTF8.GetBytes(messageData[messageLine++])));

DataVerb verb = new DataVerb();
await verb.Process(mocks.Connection.Object, new SmtpCommand("DATA")).ConfigureAwait(false);
Expand All @@ -151,7 +151,7 @@ private async Task TestGoodDataAsync(string[] messageData, string expectedData)
mocks.ServerBehaviour.Setup(b => b.GetMaximumMessageSize(It.IsAny<IConnection>())).ReturnsAsync((long?)null);

int messageLine = 0;
mocks.Connection.Setup(c => c.ReadLine()).Returns(() => Task.FromResult(messageData[messageLine++]));
mocks.Connection.Setup(c => c.ReadLineBytes()).Returns(() => Task.FromResult(Encoding.UTF8.GetBytes( messageData[messageLine++])));

DataVerb verb = new DataVerb();
await verb.Process(mocks.Connection.Object, new SmtpCommand("DATA")).ConfigureAwait(false);
Expand Down
5 changes: 5 additions & 0 deletions smtpserver/Rnwood.SmtpServer/Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ public async Task WriteResponse(SmtpResponse response)
await this.WriteLineAndFlush(response.ToString().TrimEnd()).ConfigureAwait(false);
}

public Task<byte[]> ReadLineBytes()
{
return this.ConnectionChannel.ReadLineBytes();
}

/// <summary>
/// Creates the a connection for the specified server and channel..
/// </summary>
Expand Down
2 changes: 2 additions & 0 deletions smtpserver/Rnwood.SmtpServer/IConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,7 @@ public interface IConnection
/// <param name="response">The response<see cref="SmtpResponse"/>.</param>
/// <returns>A <see cref="Task{T}"/> representing the async operation.</returns>
Task WriteResponse(SmtpResponse response);

Task<byte[]> ReadLineBytes();
}
}
2 changes: 2 additions & 0 deletions smtpserver/Rnwood.SmtpServer/IConnectionChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,7 @@ public interface IConnectionChannel : IDisposable
/// <param name="text">The text<see cref="string"/>.</param>
/// <returns>A <see cref="Task{T}"/> representing the async operation.</returns>
Task WriteLine(string text);

Task<byte[]> ReadLineBytes();
}
}
3 changes: 2 additions & 1 deletion smtpserver/Rnwood.SmtpServer/Rnwood.SmtpServer.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
<Copyright>Robert N. Wood</Copyright>
<VersionPrefix>3.0</VersionPrefix>
<Authors>Robert N. Wood &lt;rob@rnwood.co.uk&gt;</Authors>
<TargetFrameworks>netstandard2.0</TargetFrameworks>
<AssemblyName>Rnwood.SmtpServer</AssemblyName>
<OutputType>Library</OutputType>
<PackageId>Rnwood.SmtpServer</PackageId>
Expand All @@ -17,6 +16,8 @@
<GenerateAssemblyConfigurationAttribute>false</GenerateAssemblyConfigurationAttribute>
<GenerateFullPaths>true</GenerateFullPaths>
<SymbolPackageFormat>snupkg</SymbolPackageFormat>
<TargetFramework>netstandard2.0</TargetFramework>
<LangVersion>default</LangVersion>
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|netstandard2.0|AnyCPU'">
Expand Down
8 changes: 6 additions & 2 deletions smtpserver/Rnwood.SmtpServer/Rnwood.SmtpServer.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 20 additions & 6 deletions smtpserver/Rnwood.SmtpServer/SmtpStreamReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ public SmtpStreamReader(Stream stream, Encoding fallbackEncoding, bool leaveOpen
{
this.stream = stream;
this.leaveOpen = leaveOpen;
this.fallbackEncoding = Encoding.GetEncoding(fallbackEncoding.CodePage, new EncoderExceptionFallback(), new DecoderExceptionFallback());
this.fallbackEncoding = Encoding.GetEncoding(fallbackEncoding.CodePage, new EncoderExceptionFallback(),
new DecoderExceptionFallback());
}

/// <summary>Reads the a line from the stream which is terminated with a \n. The string will be decoded using UTF8 and falling back to the provided encoding if decoding fails.</summary>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>A Task representing the asynchronous operation.</returns>
public async Task<string> ReadLineAsync(CancellationToken cancellationToken)
public async Task<byte[]> ReadLineBytesAsync(CancellationToken cancellationToken)
{
this.lineBytes.Clear();

Expand All @@ -52,7 +52,7 @@ public async Task<string> ReadLineAsync(CancellationToken cancellationToken)

if (bufferByte == '\n')
{
return this.Decode(this.lineBytes.ToArray());
return this.lineBytes.ToArray();
}

if (bufferByte != '\r')
Expand All @@ -62,7 +62,8 @@ public async Task<string> ReadLineAsync(CancellationToken cancellationToken)
}

this.bufferPos = 0;
this.bufferLen = await this.stream.ReadAsync(this.buffer, 0, this.buffer.Length, cancellationToken).ConfigureAwait(false);
this.bufferLen = await this.stream.ReadAsync(this.buffer, 0, this.buffer.Length, cancellationToken)
.ConfigureAwait(false);

if (this.bufferLen < 1)
{
Expand All @@ -71,6 +72,14 @@ public async Task<string> ReadLineAsync(CancellationToken cancellationToken)
}
}

/// <summary>Reads the a line from the stream which is terminated with a \n. The string will be decoded using UTF8 and falling back to the provided encoding if decoding fails.</summary>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>A Task representing the asynchronous operation.</returns>
public async Task<string> ReadLineAsync(CancellationToken cancellationToken)
{
return this.Decode(await ReadLineBytesAsync(cancellationToken).ConfigureAwait(false));
}

/// <summary>Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.</summary>
public void Dispose()
{
Expand Down Expand Up @@ -99,6 +108,11 @@ protected virtual void Dispose(bool disposing)

private string Decode(byte[] lineBytes)
{
if (lineBytes == null)
{
return null;
}

try
{
return this.utf8Encoding.GetString(lineBytes);
Expand All @@ -109,4 +123,4 @@ private string Decode(byte[] lineBytes)
}
}
}
}
}
23 changes: 23 additions & 0 deletions smtpserver/Rnwood.SmtpServer/TcpClientConnectionChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,29 @@ public async Task WriteLine(string text)
}
}

public async Task<byte[]> ReadLineBytes()
{
try
{
using (CancellationTokenSource cts = new CancellationTokenSource(TimeSpan.FromSeconds(30)))
{
byte[] data = await this.reader.ReadLineBytesAsync(cts.Token).ConfigureAwait(false);

if (data == null)
{
throw new IOException("Reader returned null bytes");
}

return data;
}
}
catch (IOException e)
{
await this.Close().ConfigureAwait(false);
throw new ConnectionUnexpectedlyClosedException("Read failed", e);
}
}

/// <summary>
/// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
/// </summary>
Expand Down
35 changes: 21 additions & 14 deletions smtpserver/Rnwood.SmtpServer/Verbs/DataVerb.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// Licensed under the BSD license. See LICENSE.md file in the project root for full license information.
// </copyright>

using System.Linq;

namespace Rnwood.SmtpServer
{
using System;
Expand All @@ -16,6 +18,8 @@ namespace Rnwood.SmtpServer
/// </summary>
public class DataVerb : IVerb
{
private readonly byte[] CRLF_BYTES = Encoding.ASCII.GetBytes("\r\n");

/// <inheritdoc/>
public virtual async Task Process(IConnection connection, SmtpCommand command)
{
Expand All @@ -33,24 +37,27 @@ await connection.WriteResponse(new SmtpResponse(
StandardSmtpResponseCode.StartMailInputEndWithDot,
"End message with period")).ConfigureAwait(false);

using (SmtpStreamWriter writer = new SmtpStreamWriter(await connection.CurrentMessage.WriteData().ConfigureAwait(false), false))
using (Stream messageStream = await connection.CurrentMessage.WriteData().ConfigureAwait(false))
{
bool firstLine = true;
long messageSize = 0;

do
{
string line = await connection.ReadLine().ConfigureAwait(false);
byte[] data = await connection.ReadLineBytes().ConfigureAwait(false);

if (line != ".")
if (!Encoding.ASCII.GetBytes(".").SequenceEqual(data))
{
line = this.ProcessLine(line);
data = this.ProcessLine(data);

if (!firstLine)
{
writer.Write("\r\n");
messageSize += CRLF_BYTES.Length;
messageStream.Write(CRLF_BYTES, 0, CRLF_BYTES.Length);
}

writer.Write(line);
messageSize += data.Length;
messageStream.Write(data, 0, data.Length);
}
else
{
Expand All @@ -61,11 +68,11 @@ await connection.WriteResponse(new SmtpResponse(
}
while (true);

writer.Flush();
await messageStream.FlushAsync().ConfigureAwait(false);
long? maxMessageSize =
await connection.Server.Behaviour.GetMaximumMessageSize(connection).ConfigureAwait(false);

if (maxMessageSize.HasValue && writer.BaseStream.Length > maxMessageSize.Value)
if (maxMessageSize.HasValue && messageSize > maxMessageSize.Value)
{
await connection.WriteResponse(
new SmtpResponse(
Expand All @@ -74,7 +81,7 @@ await connection.WriteResponse(
}
else
{
writer.Dispose();
messageStream.Dispose();
await connection.Server.Behaviour.OnMessageCompleted(connection).ConfigureAwait(false);
await connection.WriteResponse(new SmtpResponse(StandardSmtpResponseCode.OK, "Mail accepted")).ConfigureAwait(false);
await connection.CommitMessage().ConfigureAwait(false);
Expand All @@ -85,17 +92,17 @@ await connection.WriteResponse(
/// <summary>
/// Processes a line of data from the client removing the escaping of the special end of message character.
/// </summary>
/// <param name="line">The line.</param>
/// <param name="data">The line.</param>
/// <returns>The line of data without escaping of the . character.</returns>
protected virtual string ProcessLine(string line)
protected virtual byte[] ProcessLine(byte[] data)
{
// Remove escaping of end of message character
if (line.StartsWith(".", StringComparison.Ordinal))
if (data.Length > 0 && data[0] == '.')
{
line = line.Remove(0, 1);
return data.Skip(1).ToArray();
}

return line;
return data;
}
}
}
8 changes: 4 additions & 4 deletions smtpserver/appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ version: 3.1.0-ci@{build}
image: Visual Studio 2019
environment:
matrix:
- TESTFRAMEWORK: netcoreapp2.0
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
- TESTFRAMEWORK: net8.0
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2022
PUBLISH: 1
- TESTFRAMEWORK: netcoreapp2.0
- TESTFRAMEWORK: net8.0
APPVEYOR_BUILD_WORKER_IMAGE: Ubuntu2004
PUBLISH: 0
- TESTFRAMEWORK: net462
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2019
APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2022
PUBLISH: 0
APPVEYORAPIKEY:
secure: f6ZDvXLvu8fryoFvnGfIUdpvlUC6Rft/qZKbWGh5CFw=
Expand Down

0 comments on commit 2a07568

Please sign in to comment.