Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added global HTTP response headers configuration. Resolves #3788. #4596

Merged
merged 1 commit into from Jul 26, 2019

Conversation

kashimiz
Copy link
Contributor

@kashimiz kashimiz commented Jun 19, 2019

Resolves #3788

Global HTTP headers are configured at the root host.json level, ex.:

{
  "version": "2.0",
  "customHeaders":
  {
    "X-Content-Type-Options": "nosniff"
  }
}

I chose the name customHeaders for its familiarity to ASP.NET Core developers, being used for the same purpose in web.config.

Copy link
Member

@brettsam brettsam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment about where this setting should live in the host.json -- I think @soninaren may have to change the hsts as well. It seems like these should live under extensions -> http.


public CustomHeadersMiddleware(IOptions<ScriptJobHostOptions> hostOptions)
{
RequestDelegate contextNext = async context =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming this pattern is in place here because of what was done in the HSTS middleware, but this middleware doesn't have the same requirements, so you should be able to simplify this and make it a bit more efficient.

Something as simple as this should just work:

 public class CustomHeadersMiddleware : IJobHostHttpMiddleware
    {
        private readonly ScriptJobHostOptions _hostOptions;

        public CustomHeadersMiddleware(IOptions<ScriptJobHostOptions> hostOptions)
        {
            _hostOptions = hostOptions.Value;
        }

        public async Task Invoke(HttpContext context, RequestDelegate next)
        {
            await next(context);

            foreach (var header in _hostOptions.CustomHeaders)
            {
                context.Response.Headers[header.Key] = header.Value;
            }
        }
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do. More for my own curiosity than anything else, and to better understand this part of the codebase, what were the requirements for the HSTS middleware that necessitated the pattern Naren used?

Copy link
Member

@fabiocav fabiocav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit comment (to avoid an additional file change), but this is good to go! :shipit:

@fabiocav
Copy link
Member

@kashimiz are you still waiting on anything to merge this?

@kashimiz kashimiz merged commit bb5b80e into Azure:dev Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add global HTTP header rules
4 participants