Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
51 changes: 50 additions & 1 deletion src/NetMQ.Tests/MechanismTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
using System.Text;
using System.Reflection;
using System.Runtime.Serialization;
using System.Text;
using NetMQ.Core;
using NetMQ.Core.Mechanisms;
using NetMQ.Core.Transports;
using Xunit;

namespace NetMQ.Tests
Expand Down Expand Up @@ -43,6 +47,51 @@ public void IsCommandShouldReturnFalseForInvalidCommand()
Assert.False(mechanism.IsCommand("READY", ref msg));
}

[Fact]
public void MechanismReadyShouldHandleNullPeerIdentityWhenRecvIdentityIsEnabled()
{
#pragma warning disable SYSLIB0050
var streamEngine = (StreamEngine)FormatterServices.GetUninitializedObject(typeof(StreamEngine));
#pragma warning restore SYSLIB0050
var options = new Options { RecvIdentity = true, HeartbeatInterval = 0 };
var session = new RecordingSession();
var mechanism = new NullMechanism(session, options) { PeerIdentity = null };

SetPrivateField(streamEngine, "m_options", options);
SetPrivateField(streamEngine, "m_session", session);
SetPrivateField(streamEngine, "m_mechanism", mechanism);

var method = typeof(StreamEngine).GetMethod("MechanismReady", BindingFlags.Instance | BindingFlags.NonPublic);
Assert.NotNull(method);

var exception = Record.Exception(() => method!.Invoke(streamEngine, null));
Assert.Null(exception);
Assert.Equal(0, session.LastPushedMessageSize);
}

private static void SetPrivateField(object instance, string fieldName, object value)
{
var field = instance.GetType().GetField(fieldName, BindingFlags.Instance | BindingFlags.NonPublic);
Assert.NotNull(field);
field!.SetValue(instance, value);
}

private sealed class RecordingSession : SessionBase
{
public int LastPushedMessageSize { get; private set; } = -1;

public RecordingSession()
: base(new IOThread(new Ctx(), 0), false, null!, new Options(), null!)
{
}

public override PushMsgResult PushMsg(ref Msg msg)
{
LastPushedMessageSize = msg.Size;
return PushMsgResult.Ok;
}
}

// this test was used to validate the behavior prior to changing the validation logic in Mechanism.IsCommand
// [Fact]
// public void IsCommandShouldThrowWhenLengthByteExceedsSize()
Expand Down
12 changes: 9 additions & 3 deletions src/NetMQ/Core/Transports/StreamEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1199,8 +1199,14 @@ void MechanismReady ()

if (m_options.RecvIdentity) {
Msg identity = new Msg();
identity.InitPool(m_mechanism.PeerIdentity.Length);
identity.Put(m_mechanism.PeerIdentity, 0, m_mechanism.PeerIdentity.Length);
byte[] peerIdentity = m_mechanism.PeerIdentity;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot is this code not null annotated? If this can be null, then the old code should ideally have flagged a warning. Is it feasible to annotate it as part of this work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. I updated this with focused nullable annotations in StreamEngine.MechanismReady by enabling nullable context for that method and changing peerIdentity to byte[]?, so the potential null is explicit in the code path (f841e0c). Mechanism.PeerIdentity was already nullable (byte[]?).

if (peerIdentity == null)
identity.InitEmpty();
else
{
identity.InitPool(peerIdentity.Length);
identity.Put(peerIdentity, 0, peerIdentity.Length);
}
var pushResult = m_session.PushMsg(ref identity);
if (pushResult == PushMsgResult.Full) {
// If the write is failing at this stage with
Expand Down Expand Up @@ -1384,4 +1390,4 @@ public void TimerEvent(int id)

}
}
}
}