sanic-cors middleware not always executed alongside other middleware

[j616]

The latest release (2.0.0) seems to have introduced a breaking change where CORs headers are no longer included in responses to HEAD requests. This seems to be the case for both the Sanic-Ext and default usage. I have made sure I've disabled Sanic-Ext CORS in Sanic-Ext mode, and included HEAD in the methods section of the CORS_OPTIONS. Behaviour is the same in each case. I get valid responses for both GET and HEAD requests on a route. But the GET response has the cors headers, and the HEAD response doesn't. It does, however, include the content-type.

I can't see any obvious reason for this in the changes. Perhaps there's some difference in default behaviour between default behaviour of SanicPlugin and Extension? Or am I missing some obvious change to the library interface?

Thanks in advance.

[j616]

Poking the examples with Postman, I am getting the CORS headers as expected. This might be an issue on our end.

[j616]

Have now managed to confirm this is an interaction with another bit of middleware on our end. Closing.

[ashleysommer]

Thanks, I think this is caused by the change from removing the usage of sanic-plugin-framework, to now using native Sanic middlware injection. Now Sanic-CORS has to inject its middleware into the a specific location in the app's middlware queue, and I'm worried it might not always get it right (though it did pass all my testing).

Are you able to share more information about how your other middlware was interacting with CORS headers and causing this bug you saw?

[j616]

We have a few middlewares but the one this interfered with was really basic. It sets the body on head requests to b''. It used to use the return middlewhere code path so triggered the respond early behaviour. This was fine previously because the middlewares get triggered in reverse order of which they are registered and this one is registered first. sanic-cors is set up somewhere in the middle for us. But I'm guessing our assumptions about order registered vs executed don't work for the updated sanic-cors. This was fixed for us by making our head middleware just modify the response rather than returning.

[ashleysommer]

@j616 Thanks for your info. Are you using Sanic-CORS on an App or on a Blueprint?

If you're using it on an App, then Sanic-CORS activates on before_server_start event, and it inserts its middleware right at the end of the resonse_middleware queue. It should always be run last. That is, unless you have something else that activates using before_server_start or after_server_start that adds/changes the middleware order after that.

[j616]

On an app. We don't append to the queue directly, though. We use something along the lines of app.middleware('response')(header_response_middleware) for each middleware just after app creation. So that'll happen before the server starts. So if you insert on before_server_start rather than immediately on the call to CORS(app) then you'll always end up at the end of the list. Because we were using the the respond early behaviour, your middleware would never get executed because its at the end of the queue. I suspect the sanic-plugin-framework approach either registered when CORS(app) was called or just bypassed the middleware behaviour entirely, which is why it worked previously?

[ashleysommer]

Yes, with sanic-plugins-framework (and sanic-plugin-toolkit) it did bypass the middleware completely. It was able to run all plugin middleware on a separate queue, that got processed after all of the App's own middleware. And it would run on whichever the App's response middleware output, even if it responded early with a new Response, the plugin's middleware could still modify it.

Unfortunately its no longer possible to use the approaches of Sanic-Plugin-Toolkit with Sanic v21.12+, hence the big changes in Sanic-CORS v2.0.

[j616]

Yep. Makes sense. Anyway. As said, if anyone else runs into it, our solution was to modify the existing response rather than returning early. I can imagine there being a case where that wouldn't be possible. But it's probably best leaving theoretical issues as theoretical unless they actually impact someone. I guess the alternative would be to have a flag when instantiating sanic-cors that makes it register immediately with a caveat of "here be dragons. aim to put it as late in the chain as possible".

I'll edit the title of this issue to reflect the real problem so its easier for others to find.