Repository navigation
Should HttpHeaders remove newlines from values when they're parsed? #25319
Description
Activity
Well, the newlines are not removed from custom headers because not-known headers does not contain a parser that removes the linear white space after the CRLF, that is why in your example dotnet/corefx#27727 (comment) the spaces are not removed.
Also, on not-known headers, the parsed value is stored as a string, rather than a List as in the Known Headers, that is also why you see all the white space, because, what is printed is the entire header value, with HTs, SPs and LFs,
request.Headers.GetValues("FooBar")actually just returns one value.That said, it is unclear for me what's the expected here, should we add logic to parse the header value of not-known headers to a List or should we assign the
GenericHeaderParserto not-known headers or create a brand newHttpHeaderParser?I have some thoughts about this.
First, is the current behavior of the KnownHeaders (parsing multiline input into a List) something we inherited from .NET Framework? Or was this behavior invented in .NET Core, perhaps with SocketsHttpHandler?
And adding to the questions of .NET Framework behavior, .NET Framework has other behavior when certain config file settings are done, i.e. useUnsafeHeaderParsing config attribute
<system.net> <settings> <httpWebRequest useUnsafeHeaderParsing="true" /> </settings> </system.net>
This .NET Framework config entry affects HttpClient as well (which isn't obvious).
As per an offline discussion with @jozkee
I was thinking that the correct approach would be to replace any folded line (CRLF 1 SP) with a single space when headers are parsed back since that's what the RFC 7230 says that the server should do when receiving them.
I tend to agree that the desired behavior in general should be normalization in this fashion (replacing CRLF with a single space). I don't understand at all why we (potentially) invented a behavior of allowing multiple lines to be parsed into a "list". It doesn't seem like RFC aligned behavior.
I also want to understand if this issue is just about the API surface of what
TryAddWithoutValidationis doing, or is it about the parsing behavior of received server responses from the wire. Perhaps it is both of these?At a minimum, we should align the behaviors of
TryAddWithoutValidationso that it behaves the same for both "KnownHeaders" and "CustomHeaders".I think I have an approach.
After more investigation I found that the newLines are not removed from the Headers, either Known or Custom, when they do not define an HttpHeaderParser, in example, this behavior also replicates with Accept-Patch header.
Right now, we only validate that there are no errors in the line using
ContainsInvalidNewLineand add the value right after, but we are not actually doing any replacement of folding lines.My suggestion would be that when ParseSingleRawHeaderValue or ParseMultipleRawHeaderValues checks for
descriptor.Parserwe may replace ContainsInvalidNewLine with the following:if (TryGetValueReplaceWhitespace(rawValue, out string formattedValue)) { AddValue(info, formattedValue, StoreLocation.Parsed); } ... //value: rawValue //formattedValue: the value without line foldings. private static bool TryGetValueReplaceWhitespace(string value, out string formattedValue) { formattedValue= string.Empty; int current = 0; int newLineIndex = 0; while (current < value.Length) { char c = value[current]; if (c == '\r') { int char10Index = current + 1; if (char10Index < value.Length && value[char10Index] == '\n') { //Position after \n current = char10Index + 1; int whitespaceLength = HttpRuleParser.GetWhitespaceLength(value, current); //There should be at least 1 SP|HT after New Line. if (whitespaceLength == 0) return false; //sustract 2 for CRLF. formattedValue += value.Substring(newLineIndex, current - newLineIndex - 2) + ' '; current += whitespaceLength; newLineIndex = current; } } current++; } formattedValue += value.Substring(newLineIndex, current - newLineIndex); return true; }This way we may narrow the scope to only headers that does not use an HttpHeaderParser, fix the identified scenarios where line folding is not being handled and keep aside any other behavior of any header other that the ones mentioned.
First, is the current behavior of the KnownHeaders (parsing multiline input into a List) something we inherited from .NET Framework?
I was wrong here, the header value parses to a list when there is a separator
',', not because of a line folding.And adding to the questions of .NET Framework behavior, .NET Framework has other behavior when certain config file settings are done, i.e. useUnsafeHeaderParsing config attribute
I tested it and it was not affecting the parsing at all.
At a minimum, we should align the behaviors of TryAddWithoutValidation so that it behaves the same for both "KnownHeaders" and "CustomHeaders".
I think above proposal would comply with this.
- Reacted by Karel Zikmund
- ghost locked as resolved and limited conversation to collaborators
on Dec 18, 2020
See discussion at dotnet/corefx#27727 (comment). HttpHeaders isn't removing newlines adding in header values with TryAddWithoutValidation when that value is parsed.