Copilot commented on code in PR #107:
URL: https://github.com/apache/activemq-nms-amqp/pull/107#discussion_r3255176794
##########
src/NMS.AMQP/Util/AmqpDestinationHelper.cs:
##########
@@ -91,7 +91,7 @@ public static IDestination GetReplyTo(AmqpNmsMessageFacade
message, IAmqpConnect
object typeAnnotation =
message.GetMessageAnnotation(SymbolUtil.JMSX_OPT_REPLY_TO);
if (typeAnnotation != null)
{
- byte type = Convert.ToByte(typeAnnotation);
+ sbyte type = Convert.ToSByte(typeAnnotation);
string name = StripPrefixIfNecessary(replyTo, connection,
type);
return CreateDestination(name, type);
}
Review Comment:
Convert.ToSByte(typeAnnotation) can throw on unexpected annotation
types/ranges (e.g., AMQP ubyte > 127). To avoid a message decode failure from
malformed / non-conforming peers, consider handling conversion failures
gracefully and falling back to the non-annotated path.
##########
src/NMS.AMQP/Util/AmqpDestinationHelper.cs:
##########
@@ -73,7 +73,7 @@ public static IDestination
GetDestination(AmqpNmsMessageFacade message, IAmqpCon
object typeAnnotation =
message.GetMessageAnnotation(SymbolUtil.JMSX_OPT_DEST);
if (typeAnnotation != null)
{
- byte type = Convert.ToByte(typeAnnotation);
+ sbyte type = Convert.ToSByte(typeAnnotation);
string name = StripPrefixIfNecessary(to, connection, type);
return CreateDestination(name, type);
}
Review Comment:
Convert.ToSByte(typeAnnotation) can throw (e.g., if the broker/client
encodes the annotation as an AMQP ubyte/byte outside [-128,127] or a
non-IConvertible type). That would cause destination parsing to fail the whole
decode path. Consider a safe conversion (pattern-match sbyte/byte/int, use
Try/Catch or a TryParse-style helper) and fall back to treating the annotation
as missing / using the consumerDestination when conversion fails.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
