jstedfast/MailKit

ImapClient.ConnectAsync throws NullReferenceException with OTEL enabled

georg-jung opened this issue · 2 comments

georg-jung commented

Describe the bug

ImapClient.ConnectAsync throws NullReferenceException if it's activity source MailKit.Net.ImapClient is actually used with OTEL.

ImapClient.ConnectAsync calls engine.StartNetworkOperation, which in turn uses engine's Uri property before it is set. It would be set some LoC later:

MailKit/MailKit/Net/Imap/AsyncImapClient.cs

Lines 830 to 835 in c39a209

using var operation = engine.StartNetworkOperation (NetworkOperationKind.Connect);
try {
Stream network;
engine.Uri = uri;

The NetworkOperation ctor tries to access Uri's properties, which fails because the reference is null:

MailKit/MailKit/Net/NetworkOperation.cs

Lines 66 to 69 in c39a209

if (activity is not null) {
activity.AddTag ("url.scheme", uri.Scheme);
activity.AddTag ("server.address", uri.Host);
activity.AddTag ("server.port", uri.Port);

I think this just happens if telemetry is enabled and .NET 6+ is used (I didn't test this). If telemetry is not enabled, the activity will be null and thus the faulty code path is not taken.

Looking at the code, the same probably applies to the non-async Connect.

Further looking at the code, the same probably also applies to Pop3Client:

MailKit/MailKit/Net/Pop3/Pop3Client.cs

Lines 1395 to 1398 in c39a209

using var operation = engine.StartNetworkOperation (NetworkOperationKind.Connect);
try {
engine.Uri = uri;

I didn't repro these other cases.

I don't think this applies to SmtpClient, as it does not have a seperate engine which's Uri property is used. Instead, in this class it's private uri field is properly set before it is used (note there is no var in out uri here):

MailKit/MailKit/Net/Smtp/SmtpClient.cs

Lines 1669 to 1671 in c39a209

ComputeDefaultValues (host, ref port, ref options, out uri, out var starttls);
using var operation = StartNetworkOperation (NetworkOperationKind.Connect);

Platform (please complete the following information):

  • OS: Linux
  • .NET Runtime: CoreCLR
  • MailKit Version: MailKitLite 4.7.0

Exception

System.NullReferenceException: Object reference not set to an instance of an object.
   at MailKit.Net.NetworkOperation..ctor(NetworkOperationKind kind, Uri uri, Activity activity, ClientMetrics metrics)
   at MailKit.Net.NetworkOperation.Start(NetworkOperationKind kind, Uri uri, ActivitySource source, ClientMetrics metrics)
   at MailKit.Net.Imap.ImapEngine.StartNetworkOperation(NetworkOperationKind kind)
   at MailKit.Net.Imap.ImapClient.ConnectAsync(String host, Int32 port, SecureSocketOptions options, CancellationToken cancellationToken)
   at Program.<<Main>$>g__ConnectAsync|0_0() in /__w/MailKitTelemetryNre/MailKitTelemetryNre/Program.cs:line [12](https://github.com/georg-jung/MailKitTelemetryNre/actions/runs/9776775182/job/26990023687#step:4:13)
   at Program.<Main>$(String[] args) in /__w/MailKitTelemetryNre/MailKitTelemetryNre/Program.cs:line 28
   at Program.<Main>(String[] args)

To Reproduce
Full repro in GitHub Actions: https://github.com/georg-jung/MailKitTelemetryNre/actions/runs/9776775182/job/26990023687

Otherwise, see my repro repo MailKitTelemetryNre or (expecting you have a reachable IMAP server at hostname greenmail and port 3143):

using System.Diagnostics;
using MailKit.Net.Imap;
using OpenTelemetry;
using OpenTelemetry.Trace;

async Task ConnectAsync()
{
    using var c = new ImapClient();
    await c.ConnectAsync("greenmail", 3143, false);
}

await ConnectAsync(); // this works
Console.WriteLine("Connected to greenmail");

using var tracerProvider = Sdk.CreateTracerProviderBuilder()
    .AddSource("demo")
    .AddSource(MailKit.Telemetry.ImapClient.ActivitySourceName)
    .AddConsoleExporter()
    .Build();

var s = new ActivitySource("demo");
s.StartActivity("test");
Console.WriteLine("Activity started");

await ConnectAsync(); // this doesn't
Console.WriteLine("Connected to greenmail");

Expected behavior
No NRE is thrown if ImapClient or Pop3Client are used with OTEL traces enabled.

Code Snippets
Included above.

Protocol Logs
n/a

jstedfast commented

Oof! Thanks for the bug report!

jstedfast commented

Released v4.7.1 with a fix for this.

  • ❤️2