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
[Bug]fix prefilght cors OPTIONS request record in the action visits #19030
Conversation
when options header and method is options do not record in the database.
update function
update tests
adjust code only trigger on option request
remove server
add check request method
drop prefight request
I'm not really deep into such CORS stuff, but I was wondering if we shouldn't send a proper answer to a CORS request. |
@sgiehl right that makes sense, will update that. |
update reset
@sgiehl I am trying to figure out how to do a response as 204, but I notice there is a set of requests, not just one, if I respond 204 in the foreach, will that affect another request? |
1 similar comment
This comment was marked as duplicate.
This comment was marked as duplicate.
@peterhashair You may need to handle that at an earlier stage. As it's not relevant if the preflight request might contain one or more tracking requests, you should do that before the request is actually parsed. Guess here it might suite: Lines 130 to 136 in 078909e
|
return 204 on prefight
accept cors
@sgiehl it seems like the prefight works, just wondering should I use the site url store in the data for the header('Access-Control-Allow-Methods: GET, POST');
header('Access-Control-Allow-Headers: *');
header('Access-Control-Allow-Origin: *');
Common::sendResponseCode(204); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sgiehl it seems like the prefight works, just wondering should I use the site url store in the data for the Access-Control-Allow-Origin, otherwise there will be security issue I guess.
Actually I guess the *
might be fine. We can't really know where a tracking request might come from. Using all configured sites might also not be correct, as people could also track unconfigured site urls.
I've now applied a couple of fixes and added a simple test to get this ready to merge... |
Description:
Fixes: #18812
Extend requests with
headers
andmethod
do not record preflight cors in the database.To reproduce the issue one needs to trigger prefight cors, eg add a request header more details can be found here
Review