Add Content-Type as default header that is set #186
Conversation
This was a default value that it appears some internal teams had relied on and caused problems when they were migrating. This change expands an existing middleware to handle adding any default response headers
mjrousos
left a comment
mjrousos
left a comment
There was a problem hiding this comment.
LGTM. One question about whether it makes sense to set these headers before or after subsequent middleware run, but either way likely works.
| { | ||
| foreach (var (header, value) in _defaultHeaders) | ||
| { | ||
| if (!context.Response.Headers.ContainsKey(header)) |
There was a problem hiding this comment.
Since we only set headers that aren't already present, should we call _next first so that whatever endpoint handles the request has an opportunity to write headers and we'll only set the default ones if they're still unset after that?
There was a problem hiding this comment.
We can't really do that because after _next(context) the response will have started writing. The alternative would be to register a Response.OnStarting delegate that would handle it there. Let me think about the implications of either.
There was a problem hiding this comment.
That's a good point. I'm ok with keeping this model if it's simpler.
This was a default value that it appears some internal teams had relied on and caused problems when they were migrating. This change expands an existing middleware to handle adding any default response headers
This was a default value that it appears some internal teams had relied on and caused problems when they were migrating. This change expands an existing middleware to handle adding any default response headers