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 X-Frame-Options #582

Merged
merged 7 commits into from Jun 22, 2017

Conversation

Projects
None yet
3 participants
@garrensmith
Member

garrensmith commented Jun 6, 2017

Overview

Adds X-Frame-Options support to help protect against clickjacking.
X-Frame-Options is configurable via the config and allows for DENY,
SAMEORIGIN and ALLOW-FROM. See Mozilla Docs for a nice introduction to it.

Testing recommendations

Checklist

  • Code is written and works correctly;
  • Changes are covered by tests;
  • Documentation reflects the changes;

@garrensmith garrensmith changed the title from [WIP] Add X-Frame-Options to Add X-Frame-Options Jun 8, 2017

@garrensmith garrensmith requested a review from iilyak Jun 8, 2017

garrensmith added some commits Jun 6, 2017

Add X-Frame-Options
Adds X-Frame-Options support to help protect against clickjacking.
X-Frame-Options is configurable via the config and allows for DENY,
SAMEORIGIN and ALLOW-FROM

garrensmith added some commits Jun 14, 2017

split_list(S) ->
re:split(S, "\\s*,\\s*", [trim, {return, list}]).

This comment has been minimized.

@tonysun83

tonysun83 Jun 20, 2017

Contributor

no new line

@tonysun83

tonysun83 Jun 20, 2017

Contributor

no new line

This comment has been minimized.

@garrensmith

garrensmith Jun 21, 2017

Member

Do you mean that this function should be:

split_list(S) -> re:split(S, "\\s*,\\s*", [trim, {return, list}]).
@garrensmith

garrensmith Jun 21, 2017

Member

Do you mean that this function should be:

split_list(S) -> re:split(S, "\\s*,\\s*", [trim, {return, list}]).

This comment has been minimized.

@tonysun83

tonysun83 Jun 21, 2017

Contributor

U fixed this, basically there was no new line at the end of the file

@tonysun83

tonysun83 Jun 21, 2017

Contributor

U fixed this, basically there was no new line at the end of the file

deny_with_different_host(_) ->
Headers = chttpd_xframe_options:header(mock_request(), [], config_diffent_specific_hosts()),
?_assertEqual([{"X-Frame-Options", "DENY"}], Headers).

This comment has been minimized.

@tonysun83

tonysun83 Jun 20, 2017

Contributor

end of file new no new line

@tonysun83

tonysun83 Jun 20, 2017

Contributor

end of file new no new line

; same_origin = true
; Settings hosts will return X-Frame-Options: ALLOW-FROM https://example.com/
; List of hosts separated by a comma. * means accept all
; hosts =

This comment has been minimized.

@tonysun83

tonysun83 Jun 21, 2017

Contributor

what are the consequences of having typos here besides the embedder not being able to put the site in frames and complaining to us?

@tonysun83

tonysun83 Jun 21, 2017

Contributor

what are the consequences of having typos here besides the embedder not being able to put the site in frames and complaining to us?

This comment has been minimized.

@garrensmith

garrensmith Jun 21, 2017

Member

I think its the same as for cors or anything else. The typo's will give incorrect results and mean that they might have a situation where a page can be or cannot be embedded

@garrensmith

garrensmith Jun 21, 2017

Member

I think its the same as for cors or anything else. The typo's will give incorrect results and mean that they might have a situation where a page can be or cannot be embedded

@garrensmith

This comment has been minimized.

Show comment
Hide comment
@garrensmith

garrensmith Jun 21, 2017

Member

@tonysun83 thanks for the review. I've made all the fixes you recommended

Member

garrensmith commented Jun 21, 2017

@tonysun83 thanks for the review. I've made all the fixes you recommended

@tonysun83

This comment has been minimized.

Show comment
Hide comment
@tonysun83

tonysun83 Jun 22, 2017

Contributor

+1 after squash

Contributor

tonysun83 commented Jun 22, 2017

+1 after squash

@garrensmith garrensmith merged commit 2f539e0 into apache:master Jun 22, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment