Particular / NServiceBus.Transport.Msmq

MSMQ transport for NServiceBus
https://docs.particular.net/nservicebus/msmq/
Other
5 stars 11 forks source link

Do not emit BOM when serializing message headers #710

Closed andreasohlund closed 1 month ago

andreasohlund commented 1 month ago

Versions affected

The following MSMQ transport versions will not deserialize MSMQ headers correctly when the data contains a BOM:

These versions were using Encoding.UTF8.GetString to get the data to deserialize via `XmlReader which can't handle a BOM.

NServiceBus.Transport.Msmq version 1.1.0 contains a change to be more liberal in receiving header data that have trailing bytes. That change also made the transport in being liberal in receiving headers with or without a BOM:

andreasohlund commented 1 month ago

@bording This shows that we emit the BOM in v3 which previous versions didn't do. Not sure why I fail to reproduce this via SC though

andreasohlund commented 1 month ago

The error being reported is

024-05-22 10:17:39,385 ERROR NServiceBus.Transports.Msmq.MsmqDequeueStrategy - Message '5431bf03-4ca7-4517-89ff-87f5ab0ef349\395827641' is corrupt and will be moved to the configured error queue.
System.InvalidOperationException: There is an error in XML document (1, 1). ---> System.Xml.XmlException: Data at the root level is invalid. Line 1, position 1.
at System.Xml.XmlTextReaderImpl.Throw(Exception e)
at System.Xml.XmlTextReaderImpl.ParseRootLevelWhitespace()
at System.Xml.XmlTextReaderImpl.ParseDocumentContent()
at System.Xml.XmlReader.MoveToContent()
at Microsoft.Xml.Serialization.GeneratedAssembly.XmlSerializationReaderList1.Read3_ArrayOfHeaderInfo()
--- End of inner exception stack trace ---
at System.Xml.Serialization.XmlSerializer.Deserialize(XmlReader xmlReader, String encodingStyle, XmlDeserializationEvents events)
at NServiceBus.MsmqUtilities.DeserializeMessageHeaders(Message m)
at NServiceBus.MsmqUtilities.Convert(Message m)
at NServiceBus.Transports.Msmq.MsmqDequeueStrategy.Action(TransactionSettings transactionSettings, TransactionOptions transactionOptions, MsmqUnitOfWork unitOfWork, MessageQueue receiveQueue, MessageQueue errorQueue, CircuitBreaker circuitBreaker, CriticalError criticalError, AutoResetEvent peekResetEvent, TimeSpan receiveTimeout, SemaphoreSlim throttlingSemaphore, Func`2 tryProcessMessage, Action`2 endProcessMessage)
andreasohlund commented 1 month ago

@saschanm just to double check:

Valid messages has the XML header:

<?xml version="1.0"?>...

while failing ones (coming from ServiceControl) have:

???<?xml version="1.0"?>...

is that correct?

On my box (using queue explorer I see (note the encoding attribute)

<?xml version="1.0" encoding="utf-8"?>...

ramonsmits commented 1 month ago

As mentioned in our call. The code should be liberal in what is receives and strict in what it sends:

saschanm commented 1 month ago

@saschanm just to double check:

Valid messages has the XML header:

<?xml version="1.0"?>...

while failing ones (coming from ServiceControl) have:

???<?xml version="1.0"?>...

is that correct?

On my box (using queue explorer I see (note the encoding attribute)

<?xml version="1.0" encoding="utf-8"?>...

The failing ones have the BOM (visible as ??? only in Text/ASCII mode in Queue Explorer) and also the encoding attribute (utf-8)

andreasohlund commented 1 month ago

I've figured it out thanks to @saschanm

Header deserialization in v1 vs v1.2 is different and us emitting the BOM breaks v1.0 but not v1.2. I've added a test that shows this.

This also breaks older NServiceBus v5 endpoints since they also use the same method

https://github.com/Particular/NServiceBus/blob/5.2.26/src/NServiceBus.Core/Transports/Msmq/MsmqUtilities.cs#L188

andreasohlund commented 1 month ago

Note: This problem will not manifest it self in endpoints since v3 is only used by ServiceControl and the TransportBridge

andreasohlund commented 1 month ago

I have now reproduced this end to end with SC 5.2 and a v7 endpoint using the 1.0.2 version of the transport, retrying a failure leads to the endpoint throwing:

2024-05-24 09:50:35.097 WARN  Message '7e47b130-25e6-4b43-baf0-2339b773687a\105607' has corrupted headers
System.InvalidOperationException: There is an error in XML document (1, 1). ---> System.Xml.XmlException: Data at the root level is invalid. Line 1, position 1.
   at System.Xml.XmlTextReaderImpl.Throw(Exception e)
   at System.Xml.XmlTextReaderImpl.ParseRootLevelWhitespace()
   at System.Xml.XmlTextReaderImpl.ParseDocumentContent()
   at System.Xml.XmlReader.MoveToContent()
   at Microsoft.Xml.Serialization.GeneratedAssembly.XmlSerializationReaderList1.Read3_ArrayOfHeaderInfo()
   --- End of inner exception stack trace ---
   at System.Xml.Serialization.XmlSerializer.Deserialize(XmlReader xmlReader, String encodingStyle, XmlDeserializationEvents events)
   at NServiceBus.Transport.Msmq.MsmqUtilities.DeserializeMessageHeaders(Message m)
   at NServiceBus.Transport.Msmq.MsmqUtilities.ExtractHeaders(Message msmqMessage)
   at NServiceBus.Transport.Msmq.ReceiveStrategy.TryExtractHeaders(Message message, Dictionary`2& headers)
andreasohlund commented 1 month ago

@danielmarbach caching added to both writer and reader settings

@bording updated tests based on our discussion yesterday

andreasohlund commented 1 month ago

@saschanm we'll start the process to release new versions of ServiceControl and the TransportBridge now. I'll let you know via the support case once released