ViaHeaderValue throws on TryParse

[MihaZupan]

var headers = new HttpResponseMessage().Headers;
headers.TryAddWithoutValidation("Via", "1.1 foo.bar, foo");

foreach (var h in headers) { } // Throws

System.ArgumentException: The value cannot be null or empty. (Parameter 'protocolVersion')
   at System.Net.Http.Headers.HeaderUtilities.CheckValidToken(String value, String parameterName)
   at System.Net.Http.Headers.ViaHeaderValue..ctor(String protocolVersion, String receivedBy, String protocolName, String comment)
   at System.Net.Http.Headers.ViaHeaderValue.GetViaLength(String input, Int32 startIndex, Object& parsedValue)
   at System.Net.Http.Headers.GenericHeaderParser.GetParsedValueLength(String value, Int32 startIndex, Object storeValue, Object& parsedValue)
   at System.Net.Http.Headers.BaseHeaderParser.TryParseValue(String value, Object storeValue, Int32& index, Object& parsedValue)
   at System.Net.Http.Headers.HttpHeaders.TryParseAndAddRawHeaderValue(HeaderDescriptor descriptor, HeaderStoreItemInfo info, String value, Boolean addWhenInvalid)
   at System.Net.Http.Headers.HttpHeaders.ParseSingleRawHeaderValue(HeaderDescriptor descriptor, HeaderStoreItemInfo info)
   at System.Net.Http.Headers.HttpHeaders.ParseRawHeaderValues(HeaderDescriptor descriptor, HeaderStoreItemInfo info, Boolean removeEmptyHeader)
   at System.Net.Http.Headers.HttpHeaders.GetEnumeratorCore()+MoveNext()

TryParse should never throw. In this case, it throws from the ViaHeaderValue ctor as the protocolVersion was not parsed. I assume this debug assert would trigger here as well.

This would silently succeed without throwing (but still storing a null value) before 5.0 (#47205).

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

var headers = new HttpResponseMessage().Headers;
headers.TryAddWithoutValidation("Via", "1.1 foo.bar, foo");

foreach (var h in headers) { } // Throws

System.ArgumentException: The value cannot be null or empty. (Parameter 'protocolVersion')
   at System.Net.Http.Headers.HeaderUtilities.CheckValidToken(String value, String parameterName)
   at System.Net.Http.Headers.ViaHeaderValue..ctor(String protocolVersion, String receivedBy, String protocolName, String comment)
   at System.Net.Http.Headers.ViaHeaderValue.GetViaLength(String input, Int32 startIndex, Object& parsedValue)
   at System.Net.Http.Headers.GenericHeaderParser.GetParsedValueLength(String value, Int32 startIndex, Object storeValue, Object& parsedValue)
   at System.Net.Http.Headers.BaseHeaderParser.TryParseValue(String value, Object storeValue, Int32& index, Object& parsedValue)
   at System.Net.Http.Headers.HttpHeaders.TryParseAndAddRawHeaderValue(HeaderDescriptor descriptor, HeaderStoreItemInfo info, String value, Boolean addWhenInvalid)
   at System.Net.Http.Headers.HttpHeaders.ParseSingleRawHeaderValue(HeaderDescriptor descriptor, HeaderStoreItemInfo info)
   at System.Net.Http.Headers.HttpHeaders.ParseRawHeaderValues(HeaderDescriptor descriptor, HeaderStoreItemInfo info, Boolean removeEmptyHeader)
   at System.Net.Http.Headers.HttpHeaders.GetEnumeratorCore()+MoveNext()

TryParse should never throw. In this case, it throws from the ViaHeaderValue ctor as the protocolVersion was not parsed. I assume this debug assert would trigger here as well.

This would silently succeed without throwing (but still storing a null value) before 5.0 (#47205).

Author:	MihaZupan
Assignees:	-
Labels:	bug, area-System.Net.Http
Milestone:	-

[karelz]

Triage: Regression against 5.0 -- throwing is much worse (from TryParse than invalid output).
It was reported by 1 customer.
Unless it is costly, we should fix it in 7.0.