Best way to do CORS preflight #72
Comments
|
Hi, I'm in the same situation as you, have you found a solution? |
|
I think I found a workaround for this If instead of using So this: becomes this: You have to do that for each handler you want to bypass CORS for though. |
|
You could also extend the PsychicResponse class and add the custom stuff if
you have a lot of handlers.
…On Tue, May 28, 2024, 16:54 staviq ***@***.***> wrote:
I think I found a workaround for this
If instead of using request->reply() you construct a PsychicResponse and
send that, you can add custom headers to the reply, and bypass CORS.
So this:
return request->reply(200, "application/json", jsonBuffer.c_str()); });
becomes this:
PsychicResponse response(request);
response.setCode(200);
response.setContentType("application/json");
response.setContent(jsonBuffer.c_str());
response.addHeader("Access-Control-Allow-Origin", "*");
return response.send();
You have to do that for each handler you want to bypass CORS for though.
—
Reply to this email directly, view it on GitHub
<#72 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABEHSUG4UY5YBGU22GSHYDZETVJHAVCNFSM6AAAAABCVWXHDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZWGA4DOOJVGY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
But how do you handle preflight requests? For the
But there is a preflight request which fails |
|
I have found the solution: #102 (comment) Basically you have to add these default headers DefaultHeaders::Instance().addHeader("Access-Control-Allow-Origin", "*");
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, Accept");And a wildcard handler for the server.on("/*", HTTP_OPTIONS, [](PsychicRequest *request) {
return request->reply(200);
}); |
|
Nevermind. If it is in place, the web app works which is not on the same host, but if the esp32 is the server it does not work. |
|
A middleware would be the best option where we could manipulate the headers also. |
|
Hey all, I'm not really up to date on CORS. Can someone send me a tutorial on how the client/server need to communicate? It seems like you need to check for the headers and respond with other headers in a callback no? |
Basically, whenever your browser loads a page, every image, link, css file, and so on, gets verified against the url of that page itself. If you include an image in your html, and that image is hosted somewhere else ( different host/domain ) the browser will block this and refuse to load the image. This is generally a good thing, and prevents cross site exploits etc. Except, if you are testing your own web page, via localhost, the browser will block pretty much everything outside that particular html file. That's the main problem. CORS ( in the browser ) is in fully restrictive mode if you are trying to test things locally. The only reasonable, and the simplest way to get around this, is to make the server serve the html page, together with a custom http header ( mentioned in other comments here ) The only thing you need to fiddle with and modify, is the request handler on the server side, and you just need to convince it to add another http header, beside the default ones. No extra parsing, no overriding, just one more http header, so the browser won't use cors when talking with your own server. |
|
The problem is that there is a preflight request for every request you make on the client side. Your server should send a response to this preflight request without content and a 200 response. It is an HTTP-OPTIONS request |
This is what the arduino server is doing. The same thing would need to be implemented in Psychic. That's not complicated, just a global handler on OPTIONS and add the headers back, Ideally, expose that in a configuration. (https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request) I remember that well I was developping js libraries years ago around ajax (when browsers still were not all supporting cors and some like IE were problematic). What I don't see from Arduino WS is the |
|
Yeah, thats why I said it would be a good idea to implement a way to use express.js style middlewares. You could attach a global middleware where you listen to all OPTIONS requests, also you could use these for authentication on specific routes. You can have one authentication function and attach it to any route you want. In node express js, a lot of things is done this way. For example CORS. |
|
Okay, so I see what you guys mean now about the CORS. I was experimenting with it, and it seems like sometimes the browser will skip the OPTIONS request and look for the headers in the GET request. I came up with this kludge that seems to work: //this is for SUPER ULTRA PERMISSIVE CORS requests
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Origin", "*");
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, Accept");
DefaultHeaders::Instance().addHeader("Access-Control-Max-Age", "86400");
server.on("/*", HTTP_OPTIONS, [](PsychicRequest* request) {
PsychicResponse response(request);
response.addHeader("Access-Control-Allow-Origin", "*");
response.addHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
response.addHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, Accept");
response.addHeader("Access-Control-Max-Age", "86400");
response.setCode(200);
return response.send();
});PS. You will need to run it with the v2-dev branch, since I discovered a bug with setting headers multiple times, fixed now. Moving forward, its clear we need some kind of endpoint->middleware() functionality to enable this sort of logic. I'm still trying to think how we can pull that off without breaking compatibility too much. |
|
A handy way to test this is using the "CORS Unblock" browser extension (for Chrome and Edge). It basically interrupts the headers and mangles them, which is what we're doing in code. The site shows which access-control headers they are setting. Just interesting for comparison I guess. |
|
I've been thinking about this a bit, and I think we could add an easy temporary fix that wont break anything without going full-on middleware that will require fairly large breaking changes. It seems that most of the things people want to are working with headers in a filter(). Its a bit of a hack, but adding a request->addResponseHeader() would allow you to do that without changing the API. we could then scan those headers in the PsychicResponse object and copy them over. The request object gets passed to every new Response, so it should work pretty nicely. Thoughts? |
|
Well, if the response headers are passed to every endpoint then its a middleware :D |
|
@hoeken @hitecSmartHome : note sure We only need to answer to CORS preflight request (OPTIONS) with 4 headers including a max age to avoid the browser to resend the request 5 sec later.
This should be enough: server.on("/*", HTTP_OPTIONS, [](PsychicRequest* request) {
PsychicResponse response(request);
response.addHeader("Access-Control-Allow-Origin", "*");
response.addHeader("Access-Control-Allow-Methods", "*");
response.addHeader("Access-Control-Allow-Headers", "*");
response.addHeader("Access-Control-Max-Age", "86400");
response.setCode(200);
return response.send();
});Also not that to allow everything, Also, pay attention that whatever solution we provide, it must be customisable by the user, because it really depends on the application. That is why I was saying earlier:
To be, the code above, which only ned to be documented, would be enough. If we really want to help users, we could do a Refs: |
|
That is true. Would be good to be able to add an OPTIONS endpoint |
|
But it should work for websockets and events too :/ |
|
@mathieucarbou @hitecSmartHome okay, i looked into this a bit more. i see a way forward.
bool PERMISSIVE_CORS(PsychicRequest* request)
{
if (request->hasHeader("Origin")) {
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Origin", "*");
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, Accept");
DefaultHeaders::Instance().addHeader("Access-Control-Max-Age", "86400");
}
return true;
}
For the solution, I think we should do 2 things:
I think it will be pretty straightforward to implement and allow some pretty flexible processing that verges on middleware. |
|
Here's a simple html page that will trigger CORS with a GET: <!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Fetch Image Example</title>
<style>
img {
max-width: 100%;
height: auto;
}
</style>
</head>
<body>
<h1>Image Fetch Example</h1>
<div id="image-container"></div>
<script>
// URL of the image
const imageUrl = 'http://psychic.local/alien.png';
// Function to fetch and display the image
async function fetchAndDisplayImage(url) {
try {
const response = await fetch(url);
if (!response.ok) {
throw new Error(`Network response was not ok: ${response.statusText}`);
}
const blob = await response.blob();
const imageUrl = URL.createObjectURL(blob);
const imgElement = document.createElement('img');
imgElement.src = imageUrl;
document.getElementById('image-container').appendChild(imgElement);
} catch (error) {
console.error('Error fetching the image:', error);
}
}
// Call the function
fetchAndDisplayImage(imageUrl);
</script>
</body>
</html> |
|
then in my server setup there is this: PsychicStaticFileHandler* handler = server.serveStatic("/", LittleFS, "/www/");
handler->setFilter(PERMISSIVE_CORS);
// this will send CORS headers on options requests
server.on("*", HTTP_OPTIONS, [](PsychicRequest* request) { return request->reply(200); })
->setFilter(PERMISSIVE_CORS);we really need the chain filtering, since the StaticFileHandler gets used with |
You are essentially describing the FilterChain and RequestFlter design I talked about in Discord... IMO, one way or another, this design is required and has a lot of benefits, one being to avoid duplicating auth (username passowrd) on each endpoints for example. Only one RequestFilter for authc or CORS can be created and held by the user, and added to many endpoints. A FilterChain is a sequence of several RequestFilters, a RequestFilter takes as parameter a request and a pre-built response (to be able to act on it, add headers ,etc) and has the ability to stop the chain and commit the response eagerly. The FilterChain ends by giving back control to the handler which has the responsibility to handle app specific code linked to the request. |
|
@mathieucarbou now that i've studied this a bit, i understand it a lot better. i would like to see this added for sure. to do it properly will require some breaking changes. As an intermediary step, i've added the filter chaining to both PsychicHandler and PsychicServer. It was pretty easy and is working nicely. Here is how the code that handles CORS is currently setup: bool PERMISSIVE_CORS(PsychicRequest* request)
{
if (request->hasHeader("Origin")) {
request->addResponseHeader("Access-Control-Allow-Origin", "*");
request->addResponseHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
request->addResponseHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, Accept");
request->addResponseHeader("Access-Control-Max-Age", "86400");
}
return true;
}
// this will send CORS headers on every request that contains the Origin: header
server.setFilter(PERMISSIVE_CORS);
// this will respond to CORS requests (note: the global server filter will automatically add the CORS headers)
server.on("*", HTTP_OPTIONS, [](PsychicRequest* request) { return request->reply(200); });
To do it properly, I think there are 2 big challenges:
|
I don't think so: I think it might be possible to overload and support both in a union, like I did in my previous PR. Either you use a callback with the response, or the other one, or none. IF we really want to keep those. Like already said, I do not ee any value of the current Filter implementation. This should be something like
I saw. Personally I am not fan of this part and I think this is a a bullet that we shoot ourselves in the foot. This gives too much flexibility where it might not be required and keeping backward compat' will be a hell if extension si allowed. IMO the only way to derive a response should be from a request, like with the Through a filter chain, the initial response could be created as equivalent as The response object needs to be flexible enough to support different types of response and I think this is doable, whether the answer is bytes, chars, json, message pack, or a stream. |
|
We're starting to get a bit off-track from the CORS discussion. I created issue #161 to discuss how to best implement the middleware functionality. |
What is the recommended way to perform a CORS preflight test? I want to avoid adding a dedicated OPTIONS endpoint to any POST endpoint I have.
I tried a special CORSHandler, but it never got triggered.
And I tried to tap into the not found mechanism, but this resulted in a core panic.
The text was updated successfully, but these errors were encountered: