Incorrect middleware call order

[slavugan]

So I have an app with configured trusted host and CORS and some middlewares

app = Starlite(
    debug=True,
    cors_config=cors_conf,
    allowed_hosts=['127.0.0.1', 'localhost'],
    dependencies={'edb': Provide(init_edgedb)},
    on_startup=[logging_config.configure],
    on_shutdown=[disconnect_edgedb],
    route_handlers=[health_check, admin_router],
    middleware=[SomeMiddleware, SomeMiddleware2],
)

class SomeMiddleware(MiddlewareProtocol):
    def __init__(self, app: ASGIApp):
        self.app = app

    async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
        print("///call some middleware")
        await self.app(scope, receive, send)
        print("///some middleware AWAITED")

There are two problems here:

1. SomeMiddleware2 called before SomeMiddleware. Normally we expect that middlewares will be called in order they were passed.
2. cors_config and allowed_hosts settings can't be used with other middlewares, as the other middlewares will be called before host check which in most cases does not make sense. See log below, my middlewares called even for preflight OPTIONS request.

INFO - 2022-04-19 00:22:24,513 - uvicorn.error - on - Application startup complete.
///call some middleware2
///call some middleware
///call cors middleware
INFO:     127.0.0.1:38354 - "OPTIONS /api/a/brand HTTP/1.1" 200 OK
///cors middleware AWAITED OPTIONS
///some middleware AWAITED
///some middleware2 AWAITED
# here OPTIONS req finishes and POST req starts
///call some middleware2
///call some middleware
///call cors middleware
///call trusted host middleware
INFO:     127.0.0.1:38354 - "POST /api/a/brand HTTP/1.1" 201 Created
///trusted host middleware AWAITED
///cors middleware AWAITED
///some middleware AWAITED
///some middleware2 AWAITED

[Goldziher]

Middleware are always called, also in Starlette. Why shouldn't they run for an Options request?

As for the order of middlewares, this is the pertinent code:

    def build_middleware_stack(
        self,
        user_middleware: List[Union[Middleware, Type[BaseHTTPMiddleware], Type[MiddlewareProtocol]]],
        allowed_hosts: Optional[List[str]],
        cors_config: Optional[CORSConfig],
    ) -> ASGIApp:
        """
        Builds the middleware stack by passing middlewares in a specific order
        """
        current_app: ASGIApp = self.asgi_router
        if allowed_hosts:
            current_app = TrustedHostMiddleware(app=current_app, allowed_hosts=allowed_hosts)
        if cors_config:
            current_app = CORSMiddleware(app=current_app, **cors_config.dict())
        for middleware in user_middleware:
            if isinstance(middleware, Middleware):
                current_app = middleware.cls(app=current_app, **middleware.options)
            else:
                current_app = middleware(app=current_app)
        return current_app

[slavugan]

See my log, for Options request TrustedHostMiddleware is not called, because CORSMiddleware returns response.
For example what is the sense to run auth middleware for Options request or before host check?

[slavugan]

Look how Starlette calls middlewares

///server error middleware called
///cors middleware called
INFO:     127.0.0.1:53186 - "OPTIONS /api/a/brands HTTP/1.1" 200 OK
# here OPTIONS ends and POST starts
///server error middleware called
///cors middleware called
///trusted host middleware called///
////some middleware called
////some middleware2 called
///exception middleware called
INFO:     127.0.0.1:53186 - "POST /api/a/brands HTTP/1.1" 405 Method Not Allowed

and Starlette app

async def brand(request):
    return JSONResponse({'hello': 'brand'})

class SomeMiddleware(BaseHTTPMiddleware):
    async def dispatch(self, request, call_next):
        print('////some middleware called')
        return await call_next(request)
        print('////some middleware awaited')

middlewares = [
    Middleware(CORSMiddleware, allow_origins=['http://localhost:3000'], allow_methods=('GET', 'POST')),
    Middleware(TrustedHostMiddleware, allowed_hosts=['localhost']),
    Middleware(SomeMiddleware),
    Middleware(SomeMiddleware2),
]

app = Starlette(
    debug=True,
    middleware=middlewares,
    routes=[
        Route('/api/a/brands', brand),
    ]
)

Also Starlette provide ExceptionMiddleware where I can catch validation errors and not break CORSMiddleware

[Goldziher]

Lets keep this thread ordered.

1. You are saying the order of middlewares is incorrect, and the logging you posted above appears to support this. The simplest course of action I can suggest is that you submit a PR. Either with a fix for the issue if you identify it, or with a failing test case that demonstrates this, use the @xfail decorator so the test actually passes.
2. The starlette app you created has a different order of middlewares than the one baked into Starlite. Starlite, as you can see in the code snippet I posted previously, places trusted hosts before the CORSMiddleware. There is nothing stopping you from using the Starlette CORSMiddleware and/or TrustedHostMiddleware on your own and not use the builtin Starlite configurations. Is there an issue doing this? If you think that CORSMiddleware shoulds be called before the TrustedHostMiddleware, please give me a compelling reason why this is so. Its not a big adjustment to do.

[slavugan]

Despite the fact that in Starlie TrustedHostMiddleware is placed before the CORSMiddleware, as you can see in my log for Starlite CORSMiddleware is called first. But for these two middlewares any order is fine I think. As TrustedHostMiddleware does not return any useful data in the body.
But if some middleware returns data in the body which client app needs to read it is important to call this middleware after CORSMiddleware

[slavugan]

Added PR #105

[Goldziher]

Thanks for your contribution! v1.3.0 has been released with your fixes.

[slavugan]

Great! Thanks.