Skip to content

Fix prefix-suffix conflict due to insertion order of trailing slashes#91

Open
Lencerf wants to merge 2 commits intoibraheemdev:masterfrom
Lencerf:trailing-slashes
Open

Fix prefix-suffix conflict due to insertion order of trailing slashes#91
Lencerf wants to merge 2 commits intoibraheemdev:masterfrom
Lencerf:trailing-slashes

Conversation

@Lencerf
Copy link
Copy Markdown

@Lencerf Lencerf commented Mar 25, 2026

This PR tries to solve the following failure,

#[test]
fn matchit_test1() {
    let mut router = Router::new();
    router.insert("/f{a}o", "picture").unwrap();
    router.insert("/f{a}o/{*path}", "regex").unwrap();
}

#[test]
fn matchit_test2() {
    let mut router = Router::new();
    router.insert("/f{a}o/{*path}", "regex").unwrap();
    router.insert("/f{a}o", "picture").unwrap();
}

matchit_test1 can pass but matchit_test2 fails with a conflict.

When inserting route parameters with static suffixes, matchit checked for conflicts between newly inserted suffixes and existing ones. It had an explicit exception to allow a new suffix that only differed by an extra trailing slash (e.g., o/ vs existing o).

However, this check only evaluated one direction: if the new suffix was longer than the existing suffix. If the routes were inserted in the reverse order (e.g., o/ inserted first, then o inserted second), the router incorrectly flagged them as a prefix-suffix conflict.

This commit makes the trailing slash check bidirectional by adding an else branch to verify if the existing suffix is the one with the extra trailing slash, resolving the insertion order dependency.

This PR is created with Gemini with the prompt of analyzing and fixing test case matchit_test2.

Lencerf added 2 commits March 24, 2026 17:53
Add test cases to unveil an issue where the order of adding routes like
/x20/f{a}o/{*path} and /x20/f{a}o could result in a conflict.

Signed-off-by: Changyuan Lyu <changyuanl@google.com>
When inserting route parameters with static suffixes, `matchit` checked
for conflicts between newly inserted suffixes and existing ones. It had
an explicit exception to allow a new suffix that only differed by an
extra trailing slash (e.g., `o/` vs existing `o`).

However, this check only evaluated one direction: if the *new* suffix
was longer than the *existing* suffix. If the routes were inserted in
the reverse order (e.g., `o/` inserted first, then `o` inserted second),
the router incorrectly flagged them as a prefix-suffix conflict.

This commit makes the trailing slash check bidirectional by adding an
`else` branch to verify if the *existing* suffix is the one with the
extra trailing slash, resolving the insertion order dependency.

Signed-off-by: Changyuan Lyu <changyuanl@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant