Fix quality factor handling (NRE in MediaTypeWithQualityHeaderValueComparer)

[kevinchalet]

When returning negotiable content, an exception is thrown in MediaTypeWithQualityHeaderValueComparer.CompareBasedOnQualityFactor if the Accept header contains a quality factor:

GET http://localhost:49376/ HTTP/1.1
Accept: text/html, application/xhtml+xml;q=0.9
Host: localhost:49376

System.NullReferenceException: La référence d'objet n'est pas définie à une instance d'un objet.
à Microsoft.AspNet.Mvc.MediaTypeWithQualityHeaderValueComparer.CompareBasedOnQualityFactor(MediaTypeWithQualityHeaderValue mediaType1, MediaTypeWithQualityHeaderValue mediaType2)
à Microsoft.AspNet.Mvc.MediaTypeWithQualityHeaderValueComparer.Compare(MediaTypeWithQualityHeaderValue mediaType1, MediaTypeWithQualityHeaderValue mediaType2)
à System.Linq.EnumerableSorter`2.CompareKeys(Int32 index1, Int32 index2)
à System.Linq.EnumerableSorter`1.QuickSort(Int32[] map, Int32 left, Int32 right)
à System.Linq.EnumerableSorter`1.Sort(TElement[] elements, Int32 count)
à System.Linq.OrderedEnumerable`1.<GetEnumerator>d__0.MoveNext()
à System.Linq.Buffer`1..ctor(IEnumerable`1 source)
à System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
à Microsoft.AspNet.Mvc.ObjectResult.SortMediaTypeWithQualityHeaderValues(IEnumerable`1 headerValues)
à Microsoft.AspNet.Mvc.ObjectResult.SelectFormatter(OutputFormatterContext formatterContext, IEnumerable`1 formatters)
à Microsoft.AspNet.Mvc.JsonResult.SelectFormatter(OutputFormatterContext formatterContext)
à Microsoft.AspNet.Mvc.JsonResult.<ExecuteResultAsync>d__1.MoveNext()

[harshgMSFT]

@PinpointTownes , Can you please share the app I am unable to repro your scenario with

GET http://localhost:42750/Binding/Action HTTP/1.1
Host: localhost:42750
Connection: keep-alive
Cache-Control: max-age=0
Accept: text/html, application/xhtml+xml;q=0.9
Accept-Encoding: gzip,deflate,sdch
Accept-Language: en-US,en;q=0.8

[kevinchalet]

@harshgMSFT sure: https://github.com/PinpointTownes/Issue1141 😄

[kevinchalet]

[harshgMSFT]

Still doesnt repro: Could you tell me what is the version you are running against ?
I ran it against "Microsoft.AspNet.Mvc" 6.0.0-beta1-11196

[kevinchalet]

The same one:

[DefaultLoaderEngine]: LoadFile(C:\Users\ID8874\.kpm\packages\Microsoft.AspNet.Mvc.Core\6.0.0-beta1-11196\lib\aspnet50\Microsoft.AspNet.Mvc.Core.dll)

[kevinchalet]

A few clues:

[kevinchalet]

Okay, issue found: MediaTypeWithQualityHeaderValue.Parse calls double.TryParse without specifying a format provider (which is terribly baaaaad 😄). Because the default format provider (NumberFormatInfo.CurrentInfo) relies on Thread.CurrentThread.CurrentCulture, it fails parsing 0.9 as a valid double value on machines/sessions configured with the French locale. This is also why it worked on your machine, where you're probably using an English-derived culture.

https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNet.Mvc.HeaderValueAbstractions/MediaTypeWithQualityHeaderValue.cs#L25

Using double.TryParse(qualityStringValue, NumberStyles.AllowLeadingWhite | NumberStyles.AllowTrailingWhite | NumberStyles.AllowLeadingSign | NumberStyles.AllowDecimalPoint | NumberStyles.AllowThousands | NumberStyles.AllowExponent, NumberFormatInfo.InvariantInfo, out quality) will fix this issue (you should check whether these number styles, used by the shorter TryParse overload, are valid in this case and exclude the ones that are not applicable to a quality factor)

I'd also add a null check in HeaderParsingHelpers.GetAcceptHeaders to ensure that only non-null values are returned. You could also consider returning an header with a default quality factor or throwing an exception when double.TryParse fails to parse the factor rather than returning null.

[rynowak]

@PinpointTownes thanks for your help investigating this.

There's another double.TryParse at StringWithQualityHeaderValue@30 that should also be corrected.

[harshgMSFT]

@PinpointTownes thanks for your help in getting to the bottom of this, @rynowak yes we should fix that as well.

[kevinchalet]

@rynowak @harshgMSFT thanks 🎉

[sornaks]

Checked in. Please see - 0d8a736

[kevinchalet]

@sornaks it works like a charm, thanks sir! 👍