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

Add header configuration helper to simplify header setting #1293

Merged
merged 5 commits into from
Jun 25, 2021

Conversation

spinscale
Copy link
Contributor

This introduces a Headers class with a few pre-defined header names to
have a DSL to easily configure headers. This has been created with the
OWASP security headers in mind, but can be used for anything.

Closes #1246

This introduces a `Headers` class with a few pre-defined header names to
have a DSL to easily configure headers. This has been created with the
OWASP security headers in mind, but can be used for anything.

Closes javalin#1246
@spinscale
Copy link
Contributor Author

sorry for any kotlins embarassments :-) happy to reiterate on this

@spinscale spinscale changed the title Add header configuration helper to simpliy header setting Add header configuration helper to simplify header setting Jun 25, 2021
@tipsy
Copy link
Member

tipsy commented Jun 25, 2021

Great work @spinscale, I left a few comments.

@spinscale
Copy link
Contributor Author

one more thing to discuss might be to make this as an app.before handler, because that would allow handlers to overwrite these headers... I don't expect we need to protect the user, by not allowing to set those within handlers.

@tipsy
Copy link
Member

tipsy commented Jun 25, 2021

I think app.before is better, and I also think .globalHeaders is a better function name than just headers.

@spinscale
Copy link
Contributor Author

renamed to globalHeaders, changed to app.before and removed the null check

@tipsy
Copy link
Member

tipsy commented Jun 25, 2021

Looks good from my end! Do you want to make more changes, or are you ready to merge?

@spinscale
Copy link
Contributor Author

ready to merge!

@tipsy tipsy merged commit 1c7557f into javalin:master Jun 25, 2021
@tipsy
Copy link
Member

tipsy commented Jun 25, 2021

Perfect, thank you very much!

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.

Security headers: Simplify setting of headers, possibly via plugin
2 participants