Kestrel Content-Length handling changes#43103
Conversation
| // with a status code of 1xx (Informational) or 204 (No Content). | ||
| return false; | ||
| } | ||
| else if (Method == HttpMethod.Connect && Is2xxCode(StatusCode)) |
There was a problem hiding this comment.
nit: single if with additional || (Method == HttpMethod.Connect && Is2xxCode(StatusCode)) ?
There was a problem hiding this comment.
Only if Method == HttpMethod.Connect && Is2xxCode(StatusCode) is moved to a local Is2xxConnect() local function or something. I don't like nested parens in conditions.
There was a problem hiding this comment.
Yea, I chose to do it this way because of readability (and it makes it clear which comments belong to each part). Will keep it as-is unless there's strong objection.
HaoK
left a comment
There was a problem hiding this comment.
Looks good except some minor nits
| // with a status code of 1xx (Informational) or 204 (No Content). | ||
| return false; | ||
| } | ||
| else if (Method == HttpMethod.Connect && Is2xxCode(StatusCode)) |
There was a problem hiding this comment.
Only if Method == HttpMethod.Connect && Is2xxCode(StatusCode) is moved to a local Is2xxConnect() local function or something. I don't like nested parens in conditions.
| static bool Is1xxCode(int code) => code >= StatusCodes.Status100Continue && code < StatusCodes.Status200OK; | ||
| static bool Is2xxCode(int code) => code >= StatusCodes.Status200OK && code < StatusCodes.Status300MultipleChoices; |
There was a problem hiding this comment.
| static bool Is1xxCode(int code) => code >= StatusCodes.Status100Continue && code < StatusCodes.Status200OK; | |
| static bool Is2xxCode(int code) => code >= StatusCodes.Status200OK && code < StatusCodes.Status300MultipleChoices; | |
| static bool Is1xxCode(int code) => code is >= StatusCodes.Status100Continue and < StatusCodes.Status200OK; | |
| static bool Is2xxCode(int code) => code is >= StatusCodes.Status200OK and < StatusCodes.Status300MultipleChoices; |
Cf. dotnet/roslyn#60534 -- Roslyn can produce optimized code with the pattern matching by using an uint-cast that saves one comparison at runtime. Hopefully this will be implemented soon 😉
This includes two changes to how we handle Content-Length on responses:
Fixes #42956