Bug 21717 - System.ServiceModel.Channel.WebMessageEncoder exceptions causing WCF service to crash
Summary: System.ServiceModel.Channel.WebMessageEncoder exceptions causing WCF service ...
Alias: None
Product: Class Libraries
Classification: Mono
Component: WCF assemblies ()
Version: unspecified
Hardware: All All
: --- normal
Target Milestone: Future Release
Assignee: Bugzilla
Depends on:
Reported: 2014-07-30 18:00 UTC by metalize
Modified: 2017-10-12 13:44 UTC (History)
4 users (show)

Is this bug a regression?: ---
Last known good build:

Test program to cause the described bug. (602 bytes, application/octet-stream)
2014-07-30 18:00 UTC, metalize

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.

Please create a new report for Bug 21717 on GitHub or Developer Community if you have new information to add and do not yet see a matching new report.

If the latest results still closely match this report, you can use the original description:

  • Export the original title and description: GitHub Markdown or Developer Community HTML
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments

Related Links:

Description metalize 2014-07-30 18:00:47 UTC
Created attachment 7540 [details]
Test program to cause the described bug.

Description of Problem:
System.ServiceModel.Channel.WebMessageEncoder throws exceptions when either the HTTP Post Content-Type header is missing or Content-Type is not a valid Content-Type.

Steps to reproduce the problem:
1. For the Content-Type ArgumentNullException bug run the attached code and then also run:
curl -d "data" -H "Content-Type:"
2. For the Content-Type FormatException bug run the attached code and then also run:
 curl -d "blah" -H "Content-Type: invalid"

Actual Results:
An uncaught exception from WCF. The WCF service stops running.

Expected Results:
Not an uncaught exception from WCF. The WCF service should continue running.

How often does this happen? 
Every POST without a Content-Type or with an invalid Content-Type.

Additional Information:
Current mono code:
public override bool IsContentTypeSupported (string contentType)
    if (contentType == null)
        throw new ArgumentNullException ("contentType");
    return true; // anything is accepted.

A possible fixed version:
public override bool IsContentTypeSupported (string contentType)
    if (String.IsNullOrEmpty(contentType))
        //Not supported if it's not a valid string
        return false;

        //Not supported if it's not a valid content type
        new System.Net.Mime.ContentType(contentType);
    catch (FormatException)
        return false;

    //everything else is supported
    return true;
Comment 1 Martin Baulig 2014-08-04 16:49:53 UTC
No point in fixing error messages in a class that's not implemented.

Message encoding in WCF is supported yet and large parts of the implementation are missing.
Comment 2 metalize 2014-08-04 16:59:54 UTC
It's not fixing error messages.

The service actually crashes. I'm pretty sure it's not supposed to do that. The .NET implementation does not crash on the same input.

With a small code change similar to the sample code posted, the WCF service won't crash on bad input.
Comment 3 Martin Baulig 2014-08-04 17:24:47 UTC
The problem is simply, large parts of WCF are not implemented.  This class in particular has not seen a single change in the last four years and there are a lot of FIXME's and not implemented parts in it which all come from the initial implementation in 2008.

From looking at the parts that are implemented and what I remember about message encoders in WCF, this class is supposed to convert the message.

I'm not sure what's supposed to happen when the Content-Type: header is missing - probably auto-detect it somehow?  But that doesn't seem to be implemented.

Your suggested change also doesn't look right: it would accept every Content-Type that's "valid" according to the System.Net.Mime API, but that's far more types than this class could actually handle.

And for "Content-Type: invalid" - I would assume we're supposed to crash ... ?
Comment 4 metalize 2014-08-04 17:31:21 UTC
I'm not saying my change is the correct implementation. It's just one way I was able to modify the mono source so that my WCF service will no longer crash if missing or unsupported Content-Type occurs. So, no, I'm not saying my changes are the way to go.

I am saying that the current behavior of the WCF service crashing is undesirable and definitely a bug. Whether or not the full WCF has been implemented or not, the current mono code allows a user with a web browser to crash my WCF service.
Comment 5 Martin Baulig 2014-08-04 17:41:27 UTC
Sure it's better than it was before, but let's not replace one broken thing with another not quite so broken thing, but take the chance of making it at least somewhat right.

So let's first split this into two separate problems:

1.) No Content-Type provided.  If that's crashing your app, then I think we should just pick some reasonable default type - either by removing that null check or doing something else, I don't really care.

2.) Invalid Content-Type - if I see this correctly, then this class only supports a few known types: "application/xml", "text/xml", "application/json", "text/json", "application/octet-stream".  If your app uses something else and that works, then add it.  But instead of using that System.Net.Mime API, let's just whitelist those vew supported types, how does this sound?

If possible, could you please also provide a pull request with these changes; assign to me (baulig on github) and I'll merge it :-)