Rocket
Rocket copied to clipboard
Allow routes that match any method
What's missing?
There are some scenarios which currently require cloning a route multiple times to make sure each and every method is accepted. For me these scenarios are at the very least:
- HTTP -> HTTPS redirector
- Proxying requests to another server (not redirecting because it isn’t accessible from the web)
- Internal redirects (modifying a request and dispatching it, making sure it’s processed elsewhere in the app)
Ideal Solution
From the look of it, the following changes should suffice:
- Change type of
Route::method
intoMethodSet
, an alias forenumset<Method>
. This should allow most of the existing code to work without changes. - Add
#[any(…)]
attribute macro – essentially identical to#[get(…)]
but settingRoute::method
toMethodSet::all()
. - Modify
#[route(…)]
attribute macro to accept multiple methods:#[route(GET | POST, …)]
. - Modify
#[route(…)]
attribute macro to accept the special keywordANY
as method parameter meaningMethodSet::all()
. An alternative approach would making the method optional here and default toMethodSet::all()
but personally I’d rather have things explicit. - Modify
Route::matches()
implementation, this change is trivial. From what I can tell, Rocket doesn’t optimize route matching, so the only changes required otherwise should be some logging code.
I should be able to create a PR if this approach is approved.
Why can't this be implemented outside of Rocket?
Creating attribute macros to mimic those built into Rocket would require considerable complexity (duplication of logic). Also, it would still require creating an excessive number of routes in this scenario.
Are there workarounds usable today?
This limitation can certainly be worked around, see redirector in the TLS example in this repository. The work-around there is heavy on boilerplate however instead of using the usual descriptive approach of Rocket.
My work-around does slightly better:
#[rocket::get("/<_..>")]
async fn redirect(
config: &State<Config>,
host: &Host<'_>,
uri: &Origin<'_>,
) -> Result<Redirect, Status> {
…
}
let redirects = [Get, Put, Post, Delete, Options, Head, Trace, Connect, Patch]
.into_iter()
.map(|m| {
let mut route: Route = rocket::routes![redirect].remove(0);
route.method = m;
route
})
.collect::<Vec<_>>();
However, this still creates nine routes instead of one. If nothing else, this has an impact on performance.
Also, this looks like a route for the GET method, simply because I have to specify some method. This makes code harder to read, the intent behind this route is no longer obvious on the first glance and has to be deduced from context.
Alternative Solutions
Rather than working with sets of methods, it would be possible to define a special Any
method. The result would be largely the same, only without routes supporting a subset of all methods.
Additional Context
No response
System Checks
- [X] I do not believe that this feature can or should be implemented outside of Rocket.
- [X] I was unable to find a previous request for this feature.
This has been requested in the past, but there hasn't been much support behind the request. It seems rare that one needs this feature, and when it is needed it's so one-off, and the work-arounds easy-enough, that it outweighs the complexity to implement the feature.
I should note that there is no performance impact caused by the work-around beyond creating the multiple routes. The router indexes by method, so there's no additional cost when the application is running.
As to actually doing this, I'm not opposed to the idea, but we'd need a way for Rocket to continue to statically check that only payload-bearing methods are allowed to have a data
component. The most elegant approach would probably be to allow a data
attribute iff all of the methods in the set support a payload. That might work. What do you think?
Oh, one last thing to note: one thing I'd like Rocket to do in the near future is to allow non-standard HTTP methods in addition to the current set of standard methods. Ideally whatever implementation we choose here wouldn't need to be rethought to support non-standard methods.
Oh, one last thing to note: one thing I'd like Rocket to do in the near future is to allow non-standard HTTP methods in addition to the current set of standard methods. Ideally whatever implementation we choose here wouldn't need to be rethought to support non-standard methods.
Yes, I’ve seen this issue. Unfortunately, I don’t see how this can be made to work with enumset
. So it’s either Any
as a special wildcard “method” (and no arbitrary combinations of methods) or making the method
field a HashSet<String>
. The latter would require more significant code changes and have some impact on memory usage. Whether it is worth it depends on whether you consider combinations like GET | POST worth implementing – they aren’t necessary for my use case.
I’d also have to take a closer look at routing, if you say that it is indexed by method then I apparently missed something. There is the question of how it can process the scenario where matching routes exist for both the specific method and “any.” Should it always take the route for the specific method? Or maybe go by rank, so that there can be a “default” GET route invoked when everything else fails?
The most elegant approach would probably be to allow a data attribute iff all of the methods in the set support a payload.
I’d rather say: iff any of the methods in the set support a payload. Proxying and internal redirects generally need to forward data if present, while supporting whatever methods there could be. Yes, weaker static guarantees in this case.
Fun fact: I did encounter web applications using GET requests with a payload.
I found Router::route
method which I missed before, this one will have to be adjusted as well. And here it is in fact easier to have Method::Any
as a wildcard, arbitrary combinations of methods are much more complex and not worth the effort. Eventually, there will also be Method::Custom(String)
but this won’t really change much.
Altogether, the required changes seem to be:
- Add
Method::Any
enum value and#[any(…)]
attribute macro that will create a route with this “method.” Also modify#[route(…)]
attribute macro to allowANY
as method. - Modify
Route::matches()
implementation: if method isAny
, any method parameter is accepted. - Modify
Router::route()
implementation: it needs to retrieve the list of routes both for the actual method and forMethod::Any
, then merge both iterators. As both are sorted by rank, producing a merged iterator that will keep the sorting is fairly simple.
I think that’s it.
I've implemented a version of this in the wildcard-method branch. It still needs testing (and tests), and I'm not satisfied with the memory trade-off currently implemented in the router. To support this properly, we need to make the router a bit more sophisticated.
The tests that need to be added are those that:
- Check for positive and negative collisions/matches between routes/requests with and without methods.
- Check for the same with respect to formats
format =
in routes/requests with and without methods. - Ensure ranking works properly: a lower ranked route without a method supercedes one with a method and vice-versa.
Furthermore, we should run the fuzzer which checks the required matcher/collision properties (https://github.com/rwf2/Rocket/issues/2790#issuecomment-2106141862).