dotnet / wcf

This repo contains the client-oriented WCF libraries that enable applications built on .NET Core to communicate with WCF services.
MIT License
1.72k stars 558 forks source link

UnixDomainSocketBinding ignoring set ReaderQuotas #5660

Open Arzbergerr opened 1 month ago

Arzbergerr commented 1 month ago

Describe the bug When using the UnixDomainSocketBinding for the WCF Client the set ReaderQuotas are ignored which may lead to an exception: "[...] The maximum array length quota (16384) has been exceeded while reading XML data. This quota may be increased by changing the MaxArrayLength property on the XmlDictionaryReaderQuotas object used when creating the XML reader."

I have set the mentioned MaxArrayLength to a higher value which works well on NetNamedPipeBinding but not on UnixDomainSocketBinding.

To Reproduce If needed i can try to create a small sample project the next days but i add informations after my investigations in Additional context below.

Expected behavior The set ReaderQuotas of the UnixDomainSocketBinding to be considered like in other Bindings like NetNamedPipeBinding

Additional context After some investigation of this behavior i found that the NetNamedPipeBinding add's the created BinaryMessageEncodingBindingElement of the _encoding field when CreateBindingElements is called.

public override BindingElementCollection CreateBindingElements()
        {   // return collection of BindingElements
            BindingElementCollection bindingElements = new BindingElementCollection();
            // order of BindingElements is important
            // add encoding
            bindingElements.Add(_encoding);
            // add transport security
            WindowsStreamSecurityBindingElement transportSecurity = CreateTransportSecurity();
            if (transportSecurity != null)
            {
                bindingElements.Add(transportSecurity);
            }
            // add transport (named pipes)
            bindingElements.Add(_namedPipe);

            return bindingElements.Clone();
        }

While the UnixDomainSocketBinding does not add the BinaryMessageEncodingBindingElement of the _encoding field:

public override BindingElementCollection CreateBindingElements()
        {
            // return collection of BindingElements
            BindingElementCollection bindingElements = new BindingElementCollection();
            BindingElement transportSecurity = CreateTransportSecurity();
            if (transportSecurity != null)
            {
                bindingElements.Add(transportSecurity);
            }
            _transport.ExtendedProtectionPolicy = _security.Transport.ExtendedProtectionPolicy;
            bindingElements.Add(_transport);

            return bindingElements.Clone();
        }

As workaround i created my own binding which derives form UnixDomainSocketBinding and overrides the CreateBindingElements. In my override i get the value of _encoding via reflection and add it at first position of the BindingElementCollection (like also done at NetNamedPipeBinding). With that binding the ReaderQuotas are considered and the Data get read correctly.

mconnew commented 1 month ago

You could also create a CustomBinding from the binding and add a BinaryMessageEncodingBindingElement copying the reader quotas across. That way you don't need to use reflection or create your own class.

Arzbergerr commented 1 month ago

Thanks for your info, i also had the CustomBinding as an option in mind but did not try it out yesterday at the moment i wrote the issue. Now i also tried that version with using the CustomBinding instead of the own class with reflection 'hack'.

This also works but it is important to not only add the element because that would result in another exception because the UnixDomainSocketTransportBindingElement is expected to be the last element in the BindingElementCollection.

So if the BinaryMessageEncodingBindingElement is not added but inserted this also works fine (e.g. below):

CustomBinding binding = new CustomBinding(udsBinding);
BinaryMessageEncodingBindingElement messageEncodingElement = new BinaryMessageEncodingBindingElement();
udsBinding.ReaderQuotas.CopyTo(messageEncodingElement.ReaderQuotas);

binding.Elements.Insert(0, messageEncodingElement);

The udsBinding as the already finished setup bindig of UnixDomainSocketBinding and using later this created CustomBinding works as well.

mconnew commented 1 month ago

I would do this in anticipation of a fix:

CustomBinding binding = new CustomBinding(udsBinding);
if (binding.Elements.Find<BinaryMessageEncodingBindingElement>() == null)
{
    BinaryMessageEncodingBindingElement messageEncodingElement = new BinaryMessageEncodingBindingElement();
    udsBinding.ReaderQuotas.CopyTo(messageEncodingElement.ReaderQuotas);
    binding.Elements.Insert(0, messageEncodingElement);
}
mconnew commented 1 month ago

@birojnayak, can you take a look. We should have a test which validates the reader quotas get applied. I think the easiest way to test that is to have an api where you send and receive a string. The MaxStringContentLength reader quota limits how long the string can be, the default is 8192.

Arzbergerr commented 1 month ago

I would do this in anticipation of a fix:

CustomBinding binding = new CustomBinding(udsBinding); if (binding.Elements.Find() == null) { BinaryMessageEncodingBindingElement messageEncodingElement = new BinaryMessageEncodingBindingElement(); udsBinding.ReaderQuotas.CopyTo(messageEncodingElement.ReaderQuotas); binding.Elements.Insert(0, messageEncodingElement); }

Yes, this way it would be also safe when the BinaryMessageEncodingBindingElement will be also considered by the UnixDomainSocketBinding. In my case i decided to get an exception if it is fixed so i can reconsider it and remove the workaround to make the binding preparation of UnixDomainSocketBinding more like other bindings which also may be used depending on the mode/configuration we use. This is sure the better way to stick with this solution, thank you very much.