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

Adds http_config.response_headers to the UI headers plus tests #7369

Merged
merged 1 commit into from
Mar 3, 2020

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Mar 2, 2020

This PR uses the already existing http_config.response_headers user config for setting HTTP headers when serving the UI. Use case here is for setting headers like X-Frame-Options for the UI.

Fixes #5712

Couple of notes:

  1. Do we want to reuse this config setting, or would a new one fit better? My thoughts here are it ok to reuse.
  2. Codewise I started out here with an anonymous function after the check for whether the UI is enabled, meaning the serveHandlerWithHandlers wrapper function is only created if we are running the UI. I'm not sure how important these things are in the codebase, and in the end I hoisted this function out so it lives with the rest of the functions in this file.

Submitting in Draft for the moment as I'd like to get some feedback on process for working on the go codebase here before I submit proper.

@johncowen johncowen requested a review from a team March 2, 2020 10:18
@johncowen johncowen marked this pull request as ready for review March 3, 2020 13:11
Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

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

👍

@johncowen johncowen merged commit e83fb18 into master Mar 3, 2020
@johncowen johncowen deleted the feature/ui-headers branch March 3, 2020 13:18
@johncowen johncowen added this to the 1.7.2 milestone Mar 12, 2020
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.

UI HTTP Headers
2 participants