-
Notifications
You must be signed in to change notification settings - Fork 38.5k
CORS support in 4.2 - default or configure to OFF [SPR-14049] #18621
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
Comments
Sébastien Deleuze commented I have answered directly on the Stackoverflow question: http://stackoverflow.com/a/35984705/1092077. |
ruben malchow commented hi sebastien, thanks for your response. although i can (kind of) follow zeroflagL's argument about not mixing CORS and "actual" OPTIONS request, i still think it is weird to return two completely separate responses. one more issue with this still exist, though: there is no simple way to turn it off even if i'm really certain i don't want it, and most importantly, the default is on AND to allow everything (if i'm not mistaken?). this for sure is a bit of an issue, because it means that changing my maven dependency from 4.1.x to 4.2.x opens up my application to ALL cross origin requests. this, for sure, is not right. why would you not check for the annotation and/or the existence of a CorsHandler in the context before triggering the logic? this seems to me to be an approach that would lead to more reasonable results, would it not? thanks .rm |
Rossen Stoyanchev commented The short answer is you can't handle pre-flight requests in an |
ruben malchow commented hello rossen, partly, yes. i've been thinking about the pros and cons of the built-int CORS support, and I think now that both positions are valid. *but I would still insist that changing the "default" from doing nothing to, basically "allow everything" is not right. it just should not do this without developer interaction. also, a little bit of background about our specific situation: we have requests that should contain an API key. this identifies a configuration that is stored in a database. depending on this configuration, requests are allowed or not allowed. with the current situation, it is not easy to inject something that can deal with it. the solution we had before was to do basic checks on the request in an interceptor ("is this request allowed at all") and if this goes through OK, we hand setting the specific "allowed methods" header off to the controller. we have many conditions that govern permissions on collections as well as specific elements of the collections, so we chose not try to make it abstract, so basically every controller knows how to decide access ... and we use the exact same logic to set the CORS headers that we use to decide wether or not to allow the "actual" request. also, because preflight request do not contain credentials (and there might be different session active concurrently), we are including the session ID in the URL, and we're not using cookies - so our responses are actually correct, including the current authentication. without this last bit, i can follow your approach, but i would think having an easier approach to configuring the CORS processing would be nice. for example, it would be awesome to be able to simply switch it off (as defined in #18266) or have an easier way to set the CorsProcessor. what we do now is |
Sébastien Deleuze commented CORS processing is enabled by default, but no remote origin is allowed by default. You have to configure it explicitly with |
ruben malchow commented i have a project here: https://github.com/rmalchow/spring-boot-options i am switching "security" off ("-Dsecurity.basic.enabled=false"), because i don't want to implement anything there. other than that, no configuration whatsoever. the result, however, is a response to an (explicit) OPTIONS call that contains ALL methods in the header. this is NOT a preflight request, i create and explicit JSON with the OPTIONS method call to that URL. Request: OPTIONS /test/controller HTTP/1.1
Host: 127.0.0.1:8080
Accept: application/json, text/plain, */*
Origin: http://127.0.0.1:8080
Content-Type: application/json Response: HTTP/1.1 200 OK
Server: Apache-Coyote/1.1
Allow: GET, HEAD, POST, PUT, DELETE, TRACE, OPTIONS, PATCH
Content-Length: 0
Date: Wed, 13 Apr 2016 00:42:17 GMT my controller: package springboot.optionstest.controllers;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RestController;
@RestController
@RequestMapping("/test")
public class OptionsTestController {
@RequestMapping(value="/controller",method=RequestMethod.GET)
public String x() {
return "hase";
}
} and my angular code for the AJAX call: getOptions : function() {
var def = $q.defer();
var req = {
method: 'OPTIONS',
url: '/test/controller',
headers: {
'Content-Type': 'application/json'
},
data: { },
}
$http(req).
success(function(data, status, headers, config) {
console.log("OPTIONS success: "+req.url+" --- "+headers('Allow'));
def.resolve(headers('Allow'));
}).
error(function(data, status, headers, config) {
console.log("OPTIONS error: "+status);
def.reject('error retrieving options ('+status+')');
});
return def.promise;
} |
ruben malchow commented just added an example project to show the overly permissive handling of CORS headers. |
Sébastien Deleuze commented Thanks I will have a look. |
Sébastien Deleuze commented I made a test with your sample application, but as answered on Stackoverflow, I think what you see is regular handling of Let's continue the discussion on Stackoverflow if you have other questions if that's fine for you. |
ruben malchow opened SPR-14049 and commented
hi,
i have a stackoverflow question here:
http://stackoverflow.com/questions/35982266/how-can-i-convince-spring-4-2-to-pass-options-request-through-to-the-controller
(which says it all, i guess?)
Issue Links:
The text was updated successfully, but these errors were encountered: