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

Security: Disable JSONP by default #6795

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@kimchy
Member

kimchy commented Jul 9, 2014

By default, disable the option to use JSONP in our REST layer

Disable JSONP by default
By default, disable the option to use JSONP in our REST layer
closes #6795

@kimchy kimchy added review labels Jul 9, 2014

@@ -140,7 +140,7 @@ public RestFilterChain filterChain(RestFilter executionFilter) {
public void dispatchRequest(final RestRequest request, final RestChannel channel) {
// If JSONP is disabled and someone sends a callback parameter we should bail out before querying
if (!settings.getAsBoolean("http.jsonp.enable", true) && request.hasParam("callback")){
if (!settings.getAsBoolean("http.jsonp.enable", false) && request.hasParam("callback")){

This comment has been minimized.

@dakrone

dakrone Jul 9, 2014

Member

Can the "http.jsonp.enable" setting be moved to a constant?

@dakrone

This comment has been minimized.

Member

dakrone commented Jul 9, 2014

LGTM, left a minor comment.

@dakrone

This comment has been minimized.

Member

dakrone commented Jul 9, 2014

Actually, docs/reference/api-conventions.asciidoc needs to be updated as well to reflect this change.

@kimchy

This comment has been minimized.

Member

kimchy commented Jul 9, 2014

@dakrone pushed fixes to relevant comments

@dakrone

This comment has been minimized.

Member

dakrone commented Jul 9, 2014

LGTM, +1

@@ -241,17 +241,19 @@ document indexed.
[float]
=== JSONP
By default JSONP resposes are enabled. All REST APIs accept a `callback` parameter
resulting in a http://en.wikipedia.org/wiki/JSONP[JSONP] result. You can disable

This comment has been minimized.

@s1monw

s1monw Jul 9, 2014

Contributor

we should put a note that this was changed in 1.3

This comment has been minimized.

@kimchy

kimchy Jul 9, 2014

Member

aye!, will fix

@kimchy

This comment has been minimized.

Member

kimchy commented Jul 9, 2014

@s1monw fixed doc

@s1monw

This comment has been minimized.

Contributor

s1monw commented Jul 9, 2014

LGTM

@s1monw s1monw removed the review label Jul 9, 2014

@kimchy kimchy closed this in 8910e09 Jul 9, 2014

kimchy added a commit that referenced this pull request Jul 9, 2014

Disable JSONP by default
By default, disable the option to use JSONP in our REST layer
closes #6795

@kimchy kimchy deleted the kimchy:disable_jsonp branch Jul 9, 2014

@clintongormley clintongormley added feature and removed feature labels Jul 16, 2014

@clintongormley clintongormley changed the title from Disable JSONP by default to Security: Disable JSONP by default Jul 16, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment