search

tower-rs / tower-http

HTTP specific Tower utilities.
675 stars 156 forks source link

Allow async predicate for cors AllowOrigin #478

Closed PoOnesNerfect closed 5 months ago

PoOnesNerfect commented 5 months ago

This PR allows using async predicate for AllowOrigin.

Motivation

This is required when the allowed origins may change depending on the origin and the requested resource; this often requires accessing some external state such as a database or another service.

Pretty common use case is SaaS like CMS where a user makes API calls to your server from their client side. The user will provide the list of allowed origins that can access their data from the client side.

Solution

AllowOrigin now has a function async_predicate which takes a closure that returns a future that returns bool.

Internally, AllowOrigin::to_header is now AllowOrigin::to_future which returns a future AllowOriginFuture that returns Option<(HeaderName, HeaderValue)>. For existing cases like exact, list, or predicate, the future returns immediately with the values on the first poll, so it should not incur much (if any) overhead.

One unfortunate thing about this feature is that, because the returned future needs to have a static lifetime, you cannot pass references to it like:

AllowOrigin::async_predicate(|origin, parts| async move { origin == "https://example.com" })

Instead, you must make sure all the values passed into the future have static lifetimes:

AllowOrigin::async_predicate(|origin, parts| {
    let origin = origin.to_owned();
    async move { origin == "https://example.com" }
})
clowzed commented 5 months ago

Hi @jplatte I'm excited about this feature. My project currently relies on workarounds that this PR could replace, improving efficiency significantly. I'm eager to integrate it without resorting to forking. Can we expedite the merge process?

PoOnesNerfect commented 5 months ago

Hi @jplatte I'm excited about this feature. My project currently relies on workarounds that this PR could replace, improving efficiency significantly. I'm eager to integrate it without resorting to forking. Can we expedite the merge process?

It should be merged within today or tomorrow, assuming there are no other issues. Thanks for waiting.

clowzed commented 5 months ago

Congratulations! Thanks for your work!