Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Charset wrapped with "" in Content-Type header rises issue in model binding #5349

Closed
d0pare opened this issue Oct 3, 2016 · 5 comments
Closed
Assignees
Milestone

Comments

@d0pare
Copy link

d0pare commented Oct 3, 2016

My application works with 3rd party web service which sends request with

Content-Type: text/html; charset="utf-8" instead of Content-Type: text/html; charset=utf-8

In MVC's model binding phase I get such ArgumentException

'"utf-8"' is not a supported encoding name. For information on defining a custom encoding, see the documentation for the Encoding.RegisterProvider method.

I am not sure this issue is related to Kestrel.
Please help.

Call stack:

 mscorlib.dll!System.Globalization.EncodingTable.internalGetCodePageFromName(string name) + 0x175 bytes 
 mscorlib.dll!System.Globalization.EncodingTable.GetCodePageFromName(string name) + 0x7e bytes  
 mscorlib.dll!System.Text.Encoding.GetEncoding(string name) + 0x1d bytes    
 Microsoft.AspNetCore.Mvc.Core.dll!Microsoft.AspNetCore.Mvc.Formatters.MediaType.GetEncodingFromCharset(Microsoft.Extensions.Primitives.StringSegment charset) + 0x60 bytes 
 Microsoft.AspNetCore.Mvc.Core.dll!Microsoft.AspNetCore.Mvc.Formatters.MediaType.Encoding.get() + 0x42 bytes    
 Microsoft.AspNetCore.Mvc.Core.dll!Microsoft.AspNetCore.Mvc.Formatters.MediaType.GetEncoding(Microsoft.Extensions.Primitives.StringSegment mediaType) + 0x3d bytes  
 Microsoft.AspNetCore.Mvc.Core.dll!Microsoft.AspNetCore.Mvc.Formatters.MediaType.GetEncoding(string mediaType) + 0x4f bytes 
 Microsoft.AspNetCore.Mvc.Core.dll!Microsoft.AspNetCore.Mvc.Formatters.TextInputFormatter.SelectCharacterEncoding(Microsoft.AspNetCore.Mvc.Formatters.InputFormatterContext context) + 0x73 bytes   
 Microsoft.AspNetCore.Mvc.Core.dll!Microsoft.AspNetCore.Mvc.Formatters.TextInputFormatter.ReadRequestBodyAsync(Microsoft.AspNetCore.Mvc.Formatters.InputFormatterContext context) + 0x3f bytes  
 Microsoft.AspNetCore.Mvc.Core.dll!Microsoft.AspNetCore.Mvc.Formatters.InputFormatter.ReadAsync(Microsoft.AspNetCore.Mvc.Formatters.InputFormatterContext context) + 0xa8 bytes 
 Microsoft.AspNetCore.Mvc.Core.dll!Microsoft.AspNetCore.Mvc.ModelBinding.Binders.BodyModelBinder.BindModelAsync() + 0x34f bytes 
...

Application is running on net451.

dependencies:

"dependencies": {
    "Microsoft.DiaSymReader.Native": "1.4.0",

    "Microsoft.AspNetCore.Mvc": "1.0.0",
    "Microsoft.AspNetCore.Mvc.TagHelpers": "1.0.0",
    "Microsoft.AspNetCore.Authentication.Cookies": "1.0.0",
    "Microsoft.AspNetCore.Diagnostics": "1.0.0",
    "Microsoft.AspNetCore.Server.Kestrel": "1.0.0",
    "Microsoft.AspNetCore.StaticFiles": "1.0.0",
    "Microsoft.AspNetCore.Server.IISIntegration": "1.0.0",
    "Microsoft.Extensions.Configuration.Binder": "1.0.0",
    "Microsoft.Extensions.Configuration.Json": "1.0.0",
    "Microsoft.Extensions.Configuration.UserSecrets": "1.0.0",
    "Microsoft.Extensions.Configuration.EnvironmentVariables": "1.0.0",
    "Microsoft.Extensions.Options.ConfigurationExtensions": "1.0.0",
    "Microsoft.Extensions.Logging": "1.0.0",
    "Microsoft.Extensions.Logging.Console": "1.0.0",
    "Microsoft.Extensions.Logging.Debug": "1.0.0",
    "Newtonsoft.Json": "9.0.1",
    "log4net": "2.0.5"
  }

dotnet --info output:

.NET Command Line Tools (1.0.0-preview2-003131)

Product Information:
 Version:            1.0.0-preview2-003131
 Commit SHA-1 hash:  635cf40e58

Runtime Environment:
 OS Name:     Windows
 OS Version:  6.2.9200
 OS Platform: Windows
 RID:         win8-x64
@danroth27
Copy link
Member

@dopare Did you mean to close this?

@danroth27
Copy link
Member

@dopare Can you also share with us which particular service is sending this header so that we can get a feel for the customer impact?

@danroth27 danroth27 reopened this Oct 3, 2016
@danroth27 danroth27 added this to the Backlog milestone Oct 3, 2016
@d0pare
Copy link
Author

d0pare commented Oct 3, 2016

This exception is actually catched by the MVC.
That was my bad Visual Studio was breaking on it, that's why I have closed this issue.

@danroth27 third party service is http://medical.logicnets.com/ designer.

@dougbu
Copy link
Member

dougbu commented Oct 9, 2016

@danroth27, @Eilon MVC does catch an ArgumentException while parsing Content-Type: text/json; charset="utf-8". It then treats the header as if it were invalid. Tests succeed only if the defaults are close enough to what the client requested.

Similar problems exist for any quoted parameter MVC receives e.g. MVC ignores Accept headers containing text/json; charset="utf-8".

The bug here is that MVC creates different MediaTypes for text/json; charset=utf-8 and text/json; charset="utf-8". Since the two should be 💯% equivalent (according to RFC 2045), the rest of the system is not designed to handle a MediaType.Charset value that includes quotation marks. That is, quotation marks aren't stripped out or ignored later.

FYI the ArgumentException comes from Encoding.GetEncoding(string), not MVC itself. But it would not throw anything if MediaType parsed quoted parameter values correctly.

@dougbu dougbu reopened this Oct 9, 2016
@Eilon Eilon modified the milestones: 1.1.0, Backlog Oct 10, 2016
dougbu added a commit that referenced this issue Oct 15, 2016
- #5349
- fix or add comments about other parsing errors and inconsistencies
 - `MediaType` did not skip whitespace before the type
 - look for `// ???` comments for questions about the inconsistencies

nits:
- use `+=`
- `<code>` -> `<c>` since the former is not for use within a paragraph
- split tests up to remove `bool expectedResult` parameters
dougbu added a commit that referenced this issue Oct 17, 2016
- #5349
- fix or add comments about other parsing errors and inconsistencies
 - `MediaType` did not skip whitespace before the type
 - look for `// ???` comments for questions about the inconsistencies

nits:
- use `+=`
- `<code>` -> `<c>` since the former is not for use within a paragraph
- split tests up to remove `bool expectedResult` parameters
dougbu added a commit that referenced this issue Oct 21, 2016
- #5349
- fix or add comments about other parsing errors and inconsistencies
 - `MediaType` did not skip whitespace before the type
 - look for `// ???` comments for questions about the inconsistencies

nits:
- use `+=`
- `<code>` -> `<c>` since the former is not for use within a paragraph
- split tests up to remove `bool expectedResult` parameters
dougbu added a commit that referenced this issue Oct 24, 2016
- #5349
- fix or add comments about other parsing errors and inconsistencies
 - `MediaType` did not skip whitespace before the type

nits:
- use `+=`
- `<code>` -> `<c>` since the former is not for use within a paragraph
- split tests up to remove `bool expectedResult` parameters
@dougbu
Copy link
Member

dougbu commented Oct 24, 2016

3fdcaec

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants