Multiple Accept headers are pulled out as 1 item causing Contains to fail

[jchannon]

When a client sends in multiple values in Accept header doing a check on that header fails the Contains method.

Looking at the Accept header it is a StringValues type which implements the many ICollection,IList,IEnumerable etc interfaces and the value of the Accept header is 1 entry with a string separated by comma. Therefore a IList.Contains will fail because there isn't an entry that matches application/json when the entry is application/json, text/plain, */*

I would have expected 3 entries in the result of calling context.Request.Headers["Accept"] which would have made the check pass. For now I will have to use context.Request.Headers["Accept"].Any(h => h.Contains("application/json")) or context.Request.Headers["Accept"].ToString().Contains("application/json"))

Attached is a screenshot of my debug window which shows what I saw.

[Tratcher]

The indexer returns the raw values as received by the server. If you want a specific format you need to ask for it. E.g. context.Request.Headers.GetCommaSeparatedValues(HeaderNames.Accept);
https://github.com/aspnet/HttpAbstractions/blob/1.0.0/src/Microsoft.AspNetCore.Http.Abstractions/Extensions/HeaderDictionaryExtensions.cs#L40

[jchannon]

Why isn't there a context.Request.Headers.Accept that returns an array for example?

I'm also not sure why the decision was made to return it as a string as the indexer returns a type that implements numerous array interfaces so surely the expectation would be to expect multiple entries

[darrelmiller]

@jchannon I'm assuming the multiple values are supported because there may actually be more than one Accept header "line" in the request message. The System.Net.Http libraries took care of merging multiple header lines into a single array of media type ranges.

@Tratcher It is unfortunate that we require the end user to know how to standardized headers like Accept are delimited. How would someone split user agent products and comment tokens as they are space delimited but comments also allow spaces within them?

[Tratcher]

See https://github.com/aspnet/HttpAbstractions/blob/dev/src/Microsoft.AspNetCore.Http.Extensions/HeaderDictionaryTypeExtensions.cs#L16
https://github.com/aspnet/HttpAbstractions/blob/dev/src/Microsoft.AspNetCore.Http.Extensions/RequestHeaders.cs#L24

[jchannon]

Would using those extensions return multiple items in the array?

UPDATE GetTypedHeaders appears to make them an array with multiple entries

[khellang]

I think, once again, this problem boils down to "pay to play". If you don't want/need parsing of header values, the framework will allow you to get the raw values. The question is; should the defaults be the other way around, i.e. the framework optimizes for the 90%* that need separated values, but give the remaining 10%* an API that allows you to "drop down" to raw values? Anyway, I think the ship has sailed, long ago, for anyone to do much about it.

* numbers pulled out of my ass

[jchannon]

I think the default should return them as an array and merge them if the client sends multiple Accept headers. The API of the headers collection returns StringValues which is an array. If it was string then it would be understandable why I got them all as one string value but its not so therefore if the API is an array I expect multiple entries in the array

[Tratcher]

I see at least two problems with that expectation.

First, you're using the lowest level API and expecting it to have knowledge of every possible header format. If you were accessing an Accept property then yes it would make sense that it would know the format. The comments above about the user agent header underscore this well, and then there are custom headers. Making broad assumptions at the lowest level makes those assumeptions impossible to work around when they go wrong.

Second, comma splitting is only the first level of structure. Your Contains approach ignores the fact that each value is itself structured, possibly containing additional properties like q-values. Not to mention other issues like case normalization, wildcards, etc..

Even if we did this one thing for you here, you would still need a much deeper API to correctly evaluate the headers.

[khellang]

@Tratcher Because StringValues can represent zero, one or many string values, it's hard to know whether StringValues.Contains has "string contains" or "collection contains" semantics. Today, it's the latter, i.e. "any value is equal to", but if it has only one value, like text/html, application/json, application/xml, you'd expect Contains("application.json") to return true.

[darrelmiller]

The current solution seems like an ideal approach to me. The StringValues primitive is a bit weird, but it solves the problem of duplicate headers. It also allows a proxy to maintain arbitrary duplicate headers if it wants to.

Parsing the headers into semantically meaningful things should be a secondary and optional thing. This is one of the major advantages of these HttpAbstractions over the System.Net.Http stuff where you couldn't avoid paying the price of doing the semantic parsing of everything.

[jchannon]

One other thing I see as a bit odd is requiring a higher level package to do the behaviour I'd expect at the lowest level but it is what it is I guess

[darrelmiller]

@khellang I see the StringValues that comes from  request.Headers dictionary as being equivalent to this https://tools.ietf.org/html/rfc7230#section-3.2 where field-content may be zero, one or many values.

@jchannon The semantic parsing is still in the HttpAbstractions package.  The typed headers effectively provide the interpretation as per RFC 7321-5. I think the distinction between the raw header value and the parsed content is a valuable one.

[khellang]

@darrelmiller Exactly, but if you call request.Headers["Accept"].Contains("application/json") and the Accept header is text/html, application/json; q=0.8, application/xml; q=0.6, */*; q=0.4, what would you expect it to return? true or false? Today, that will always return false, no matter if the string's been split by commas or not.

EDIT: To clarify; my question is if this Contains implementation is "correct".

[jchannon]

@darrelmiller the typed headers are available in another package Microsoft.AspNetCore.Http.Extensions I believe

[darrelmiller]

@khellang Doing simple string matching on HTTP headers is asking to introduce subtle bugs. I'm not sure how you convince people not to do that though. If the Accept header was just application/json then the Contains() would work! I think having access to the unfolded headers is worth the extra parsing step. Especially because this is a the HttpAbstractions layer. Frameworks can do pretty stuff on top of this stuff if they like.

@jchannon My bad, I saw package and thought repo, not Nuget.