Catch small buffer Exception Beacon

[bart-manusvr]

Catches exception that occurs when a UDP message larger than 255 appears on the beacon's port and ignores it.

Reproduce by sending a message on a given UDP port when the beacon is listening on it that is larger than 255 bytes.

FATAL { 2021-05-13 16:20:31,295 [NetMQActor-4640-3863] } - Fatal unhandled exception thrown, runtime terminating=True System.Net.Sockets.SocketException (0x80004005): A message sent on a datagram socket was larger than the internal message buffer or some other network limit, or the buffer used to receive a datagram into was smaller than the datagram itself at System.Net.Sockets.Socket.ReceiveFrom(Byte[] buffer, Int32 offset, Int32 size, SocketFlags socketFlags, EndPoint& remoteEP) at NetMQ.NetMQBeacon.Shim.ReceiveUdpFrame(String& peerName) at NetMQ.NetMQBeacon.Shim.OnUdpReady(Socket socket) at NetMQ.NetMQPoller.RunPoller() at NetMQ.NetMQPoller.Run(SynchronizationContext syncContext) at NetMQ.NetMQBeacon.Shim.Run(PairSocket shim) at NetMQ.NetMQActor.RunShim() at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx) at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx) at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state) at System.Threading.ThreadHelper.ThreadStart()

[codecov[bot]]

Codecov Report

Merging #977 (55644ed) into master (0e0c470) will increase coverage by 0.06%. The diff coverage is 46.15%.

:exclamation: Current head 55644ed differs from pull request most recent head eb1d7e2. Consider uploading reports for the commit eb1d7e2 to get more accurate results

@@            Coverage Diff             @@
##           master     #977      +/-   ##
==========================================
+ Coverage   65.92%   65.99%   +0.06%     
==========================================
Files         148      148              
Lines        9059     9068       +9     
Branches     1495     1444      -51     
==========================================
+ Hits         5972     5984      +12     
- Misses       2472     2481       +9     
+ Partials      615      603      -12     

Impacted Files	Coverage Δ
src/NetMQ/NetMQBeacon.cs	67.04% <46.15%> (-2.42%)	:arrow_down:
src/NetMQ/Core/Patterns/Dish.cs	50.42% <0.00%> (-3.42%)	:arrow_down:
src/NetMQ/Core/SessionBase.cs	70.89% <0.00%> (-1.50%)	:arrow_down:
src/NetMQ/Core/SocketBase.cs	70.84% <0.00%> (-0.36%)	:arrow_down:
src/NetMQ/OutgoingSocketExtensions.cs	76.71% <0.00%> (+0.68%)	:arrow_up:
src/NetMQ/AsyncReceiveExtensions.cs	40.59% <0.00%> (+0.99%)	:arrow_up:
src/NetMQ/Core/Patterns/Router.cs	79.14% <0.00%> (+1.06%)	:arrow_up:
src/NetMQ/ReceivingSocketExtensions.cs	48.86% <0.00%> (+1.13%)	:arrow_up:
src/NetMQ/Core/Patterns/Server.cs	61.01% <0.00%> (+1.69%)	:arrow_up:
src/NetMQ/Core/Transports/Tcp/TcpAddress.cs	57.57% <0.00%> (+3.03%)	:arrow_up:
... and 3 more

[drewnoakes]

Does it make sense to return a frame of zero length?

[bart-manusvr]

Returning a null instead would require more parts of the code to be rewritten. Would setting a negative length be more appropriate and not break other things?

[bart-manusvr]

I was wondering if there is a chance of this being merged in soon, and if there is a release coming? The company I work for is running into issues with customers where they have large messages appearing on a UDP port which causes the whole application to crash. If there is anything that needs to be changed in this pull request in order to get it merged, please notify me so I can fix it as soon as possible!

[drewnoakes]

I took a bit more of a look at this and want to check something with you.

My understanding of the problem is that your beacon is receiving a message that is not intended for it, and that message is larger than the allocated buffer size (256), which previously threw an exception.

If that's the case, then my understanding of the change in this PR is that when such a message is received, a zero-length frame is sent down the pipe, within the shim. I suspect this is not great behaviour.

Could you change ReceiveUdpFrame to bool TryReceiveUdpFrame(out NetMQFrame frame, out string peerName) instead, and have it return false when such an oversized message is received, which would trigger OnUdpReady to return early.

@somdoron has historically taken care of package updates.

[bart-manusvr]

This should be more according to what might be desired?

[drewnoakes]

Thanks very much!

@somdoron what will it take to get a release out? Or is there a feed for CI build packages?

[bart-manusvr]

Has there been any new information regarding the release? We are looking towards releasing our product sometime soon, and would like this to be in as well.

[bart-manusvr]

@drewnoakes @somdoron Is there any update coming, because we are really depending on this fix. Please respond.

[drewnoakes]

I'm on a phone right now, but I believe you can source a package from the CI build feed rather than NuGet.org, to unblock yourself.

@somdoron has historically made the NuGet.org releases.

[alex-manusvr]

We poked around but couldn't find it anywhere - is there a place we should be looking?

[drewnoakes]

I can't see a feed for CI builds either. @somdoron is the pipeline set up to produce prerelease packages if we push a tag?