Skip to content

fix: path-based routing broken due to key collisions in sanitize()#2524

Merged
oschwartz10612 merged 4 commits intofosrl:devfrom
shreyaspapi:fix/2294-path-based-routing
Mar 8, 2026
Merged

fix: path-based routing broken due to key collisions in sanitize()#2524
oschwartz10612 merged 4 commits intofosrl:devfrom
shreyaspapi:fix/2294-path-based-routing

Conversation

@shreyaspapi
Copy link
Copy Markdown
Contributor

@shreyaspapi shreyaspapi commented Feb 23, 2026

Community Contribution License Agreement

By creating this pull request, I grant the project maintainers an unlimited,
perpetual license to use, modify, and redistribute these contributions under any terms they
choose, including both the AGPLv3 and the Fossorial Commercial license terms. I
represent that I have the right to grant this license for all contributed content.

Description

Fixes #2294

What this fixes

The sanitize() function used to generate internal map keys for grouping targets in getTraefikConfig can cause path collisions. It replaces all non-alphanumeric characters (including /, ., _) with dashes and collapses them, so structurally different paths can produce the same key:

  • sanitize("/a/b") = "a-b"
  • sanitize("/a-b") = "a-b" (same!)

When two targets get the same key, they're grouped into one loadbalancer service and Traefik round-robins between them instead of routing by path.

Note: this specific collision doesn't affect the / vs /api scenario reported in the issue (those already produce different keys), but it would affect setups with paths like /api/v1 vs /api-v1 or /app.v2 vs /app/v2.

How to test?

  1. Create two resources with paths that would collide under old sanitization (e.g., /api/v1 and /api-v1)
  2. Verify they route to their correct backends independently (no round-robin)
  3. Verify existing resources with simple paths still work with the same router names

Copy link
Copy Markdown
Member

@oschwartz10612 oschwartz10612 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please adjust these two things and then we should be good to merge up. Thanks for the change!

…isions

Use encodePath only for internal map key grouping (collision-free) and
sanitize for Traefik-facing router/service names (unchanged for existing
users). Extract pure functions into pathUtils.ts so tests can run without
DB dependencies.
… scheme

Address PR review comments:
- Remove pathUtils.ts and move sanitize/encodePath directly into utils.ts
- Simplify dual-key approach to single key using encodePath for map keys
- Remove backward-compat logic (not needed per reviewer)
- Update tests to match simplified approach
@shreyaspapi shreyaspapi force-pushed the fix/2294-path-based-routing branch from c8ae0e6 to 75a9097 Compare March 1, 2026 10:19
@shreyaspapi
Copy link
Copy Markdown
Contributor Author

Fixed need a review

@oschwartz10612 oschwartz10612 changed the base branch from main to dev March 8, 2026 05:18
@oschwartz10612
Copy link
Copy Markdown
Member

Thank you!

@oschwartz10612 oschwartz10612 merged commit 9a0a255 into fosrl:dev Mar 8, 2026
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.

Path-Based-Routing is broken

2 participants