dotnet / docs

This repository contains .NET Documentation.
https://learn.microsoft.com/dotnet
Creative Commons Attribution 4.0 International
4.25k stars 5.89k forks source link

WCF StreamBodyWriter 4.0-4.5 retargeting change #13940

Closed KalleOlaviNiemitalo closed 9 months ago

KalleOlaviNiemitalo commented 5 years ago

Please add the following to the list of migration issues.

If an implementation of System.ServiceModel.Channels.BodyWriter.OnWriteBodyContents(XmlDictionaryWriter writer) intends to output a stream, it must call writer.WriteStartElement("Binary"), writer.WriteBase64(buffer, offset, count), and perhaps eventually writer.WriteEndElement. So the message body is conceptually XML <Binary>…base64…</Binary>, although WCF normally manages to skip the Base64 encoding and decoding.

The System.ServiceModel.Channels.StreamBodyWriter class handles this conceptual Base64 encoding. Applications can define a derived class that overrides StreamBodyWriter.OnWriteBodyContents(Stream stream) and writes the data to the stream. In .NET Framework 4.0 however, StreamBodyWriter does not call writer.WriteStartElement("Binary") even though it is required by System.ServiceModel.Channels.XmlByteStreamWriter. The derived class thus has to override StreamBodyWriter.OnWriteBodyContents(XmlDictionaryWriter writer), like so:

protected override void OnWriteBodyContents(System.Xml.XmlDictionaryWriter writer)
{
    writer.WriteStartElement("Binary");
    base.OnWriteBodyContents(writer);
    writer.WriteEndElement();
}

If such an application targeting .NET Framework 4.0 is then run on .NET Framework 4.5 or later, StreamBodyWriter activates a quirk so that the application still works. However, if the application is retargeted to .NET Framework 4.5 or later, WCF discards the data written to the stream. No exception is thrown. This happens because System.ServiceModel.Channels.StreamBodyWriter.XmlWriterBackedStream.Write(byte[] buffer, int offset, int count) sees that writer.WriteState is WriteState.Element, rather than WriteState.Content or WriteState.Start.

Thus, when the application is being retargeted to .NET Framework 4.5 or later, the override of OnWriteBodyContents(XmlDictionaryWriter) should be removed. Alternatively, I think the following code will work regardless of whether the application targets .NET Framework 4.0, 4.5, or a later version.

protected override void OnWriteBodyContents(System.Xml.XmlDictionaryWriter writer)
{
    writer.WriteStartElement("Binary");
    writer.WriteString(""); // Changes writer.WriteState from WriteState.Element to WriteState.Content.
    base.OnWriteBodyContents(writer);
    writer.WriteEndElement();
}

Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.


Associated WorkItem - 199399

KalleOlaviNiemitalo commented 5 years ago

A related Stack Overflow question: Breaking change in class XmlWriterBackedStream with KB2901983

It refers to KB2901983: The Microsoft .NET Framework 4.5.2 for Windows and the corresponding language packs are available.

So, perhaps the behaviour was changed in .NET Framework 4.5.2. The target version threshold for the compatibility quirk is 4.5 rather than 4.5.2, however.

rpetrusha commented 5 years ago

@mconnew, can you confirm that this should be added to the compatibility documentation?

mconnew commented 5 years ago

The behavior change is correct. I'll need to spend some time validating the exact details of the code change required in customer code, but from memory I believe it's correct. I'll update next week whether it's good as is or if any changes are needed.

mconnew commented 5 years ago

Here's the information that needs to be added. I tried to keep it to just the facts and keep it simple:

When deriving from the class System.ServiceModel.Channels.BodyWriter and the implementation of OnWriteBodyContents(XmlDictionaryWriter writer) is being used to write binary output, some changes may need to be made. The write state should be checked and if it's WriterState.Start, emit the Binary wrapping XML element.

protected override void OnWriteBodyContents(XmlDictionaryWriter writer)
{
    bool wroteStartElement = false;
    if (writer.WriteState == WriteState.Start)
    {
        writer.WriteStartElement("Binary", string.Empty);
        wroteStartElement = true;
    }
    writer.WriteBase64(buffer, offset, count);
    if (wroteStartElement)
    {
        writer.WriteEndElement();
    }
}

When deriving from the class System.ServiceModel.Channels.StreamBodyWriter, if the method OnWriteBodyContents(XmlDictionaryWriter writer) has been overridden then some changes may be required. When targeting .NET 4.0, it was necessary to explicitly write the Binary element when overriding this method. This is no longer needed and doing so when targeting .NET 4.5 will cause the body to not be written.

KalleOlaviNiemitalo commented 5 years ago

When deriving from the class System.ServiceModel.Channels.BodyWriter […] The write state should be checked

I suspect this part of the migration advice is not necessary and Base64 implementations of BodyWriter.OnWriteBodyContents can unconditionally write the Binary element.

I searched for uses of IsApplicationTargeting45 or TargetsAtLeast_Desktop_V4_5 as of microsoft/referencesource 4.6.2-29-ge5627972, and I do not think any of those affects how BodyWriter.OnWriteBodyContents is called when StreamBodyWriter is not involved.

When deriving from the class System.ServiceModel.Channels.StreamBodyWriter […]

This part is OK.

mconnew commented 5 years ago

@KalleOlaviNiemitalo, if you create a test app in your verification, could you share it here?

KalleOlaviNiemitalo commented 5 years ago

@mconnew, I don't currently have a test app that I could share, and I'm not sure I will create one.

Earlier, I tested that the writer.WriteString("") workaround fixes the problem, when targeting .NET Framework 4.5.2 and running on .NET Framework 4.8. This was server-side WCF with WebHttpBinding, transferMode="Buffered", WebInvokeAttribute.ResponseFormat = WebMessageFormat.Json, and an IErrorHandler.ProvideFault implementation that added new WebBodyFormatMessageProperty(WebContentFormat.Raw) to the fault Message. This scenario was OK with unconditionally writing the wrapper element.

I am looking at Reference Source. System.ServiceModel.Channels.StreamBodyWriter overrides OnCreateBufferedCopy(int maxBufferSize). If an app derives a class directly from System.ServiceModel.Channels.BodyWriter and does not override OnCreateBufferedCopy, then the state will be WriterState.Element when BodyWriter.OnCreateBufferedCopy calls OnWriteBodyContents. If the app's implementation of OnWriteBodyContents was written the way you advised, it then omits the wrapper element because the state is not WriteState.Start, so there will be no Binary wrapper element in the XmlBuffer (only the a wrapper element added by BodyWriter.OnCreateBufferedCopy). Later, BufferedBodyWriter.OnWriteBodyContents will copy the data from the XmlBuffer to the other XmlDictionaryWriter; again without the Binary wrapper element, even if the write state was WriteState.Start now. I think this is a scenario that shows OnWriteBodyContents really needs to unconditionally write the wrapper. However, I did not test it yet.

dotnet-bot commented 3 years ago

This issue has been closed as part of the issue backlog grooming process outlined in #22351.

That automated process may have closed some issues that should be addressed. If you think this is one of them, reopen it with a comment explaining why. Tag the @dotnet/docs team for visibility.

Youssef1313 commented 3 years ago

@gewarren Should this be reopened?