Check that number of path parameters matches tuple path extractor exactly

[LHolten]

• I have looked for existing issues (including closed) about this

Feature Request

Currently, Path<String> checks that there is exactly one path parameter, but Path<(String, String)> only checks that there is a minimum of two path parameters. It will then use the first two parameters as the actual values and discard the rest.

I think it would be much better to check that the number of path parameters matches the size of the tuple exactly to match the single parameter case.

Motivation

At work we had an incident that could have been prevented in multiple ways, one of which was if axum had caught the mismatch in parameter count.

What happened is that a path segment was changed from a constant to a parameter like this:

"/thing/:param1/:param2" => /:param0/:param1/:param2

The handler (which was defined in a nested router) used Path<(String, String)> and was not updated. This meant that the new :param0 was now used as param1 and :param1 as param2.

---

So for the sake of correctness of web servers I think it is good to be more strict in the case of tuple path extractors.
As for us, we will probably not use tuple path extractractors anymore (not a big deal), at least until axum checks them more strictly as proposed here.

Backwards Compatibility

While this is technically a backwards incompatible change, I would argue that it is closer to a bug fix. The current behaviour is undocumented and surprising.

Proposal

Implementation is really simple, change the < operator to != here

axum/axum/src/extract/path/de.rs

Line 143 in 068c9a3

if self.url_params.len() < len {

Alternatives

• It would be even better to have the number of path parameters checked when the router is initialized.
• Instead of changing the behaviour, the documentation could be updated to state very clearly that tuples don't check the number of parameters. This would still be surprising behaviour and likely to trip the unwary.

---

I will happily make a pull request with the change and a test!

[jplatte]

I'd welcome a PR for this, but I think this is too risky of a change for another patch release, so I wouldn't release it until 0.8.0, and would like it to be documented as a breaking change.

[LHolten]

I made a PR #2931