/customer/{id} Swallowed /customer/add, Because It Was Declared First

By Lars Nielsen ·

CaRMen's customer section used to be split across two controllers for no particular reason: CustomerController owned the list and detail pages, and a generic WebController owned the "add new customer" form, declared as its own route:

#[Route('/customer/add', name: 'customer_add')]

CustomerController's detail route looked like this, right next to it in the URL space but in a different file entirely:

#[Route('/customer/{id}', name: 'app_customer_show', methods: ['GET'])]

Nothing about either declaration is wrong on its own. Put side by side, though, /customer/{id} with no constraint on what {id} is allowed to contain will match literally any single path segment after /customer/ — including the literal string add.

Which route wins depends on which file loads first

Symfony resolves attribute routes by scanning controller files and registering whatever it finds, in the order of its directory scan — and the attribute directory loader sorts the files by path before loading them, so for src/Controller/ that order is alphabetical, every time: CustomerController.php before WebController.php. That ordering is exactly what turns two harmless-looking routes into a collision. CustomerController's unconstrained /customer/{id} route gets registered first. A request to /customer/add then matches it before the router ever reaches WebController's dedicated /customer/add route — Symfony's matcher takes the first route in its collection that fits the path, and an unconstrained {id} segment fits anything. The add-customer page was reachable only because nothing had happened yet to expose the gap; the two routes were both real and both wired up, and the one that should never have been allowed to answer /customer/add was simply asked first.

The fix is also a reorganization

The repair did two things at once, and either alone would have been incomplete. First, the show route got an explicit constraint:

#[Route('/{id}', name: 'show', requirements: ['id' => '\d+'], methods: ['GET'])]

restricting {id} to digits only, so add can no longer satisfy it regardless of registration order. Second, the add-customer action moved out of WebController entirely and into CustomerController itself, under a class-level route prefix:

#[Route('/customer', name: 'app_customer_')]

with its own sibling route, #[Route('/add', name: 'add', methods: ['GET', 'POST'])]. Both customer-related concerns now live in the same controller, under the same prefix, instead of split across two files that each assumed they had the URL space to themselves. The index route's name changed too, from the slightly odd app_customer to app_customer_index, consistent with the new prefix-plus-action naming — which meant updating the one Twig template that still called path('app_customer') to reference the new name, or the "back to list" link on every customer detail page would have started throwing a route-not-found error instead of a routing collision.

Why the digit constraint alone wouldn't have been enough

It's tempting to read this as purely a missing-constraint bug — add \d+ and move on. That's necessary, but on its own it only fixes this one collision between these two specific routes. The deeper issue is that two controllers were each free to claim parts of the same URL prefix without either one being aware of the other, which is a structural invitation for the next version of the same bug to show up under a different pair of routes the next time someone adds a new action under /customer/. Consolidating both routes under one controller with one explicit prefix doesn't just fix today's collision; it means the next person adding a customer-related route only has one file to check for what's already claimed, instead of needing to know that a completely unrelated-sounding "web" controller might also have opinions about a URL that starts with /customer.

The general shape of the bug

Route collisions like this one are quiet by design: nothing fails until the specific path that exposes the ordering problem gets requested, and by then the symptom looks like "the add-customer page shows a customer that doesn't exist" rather than anything that points at routing. There's no exception, no log line, no 404 — just whichever controller action got matched first doing exactly what it was written to do, with an argument that happens to be the wrong kind of value for it.

It's also the kind of bug that attribute routing makes slightly easier to miss than the old centralized routes.yaml files did. When every route in an application lived in one file, scanning down the list for two patterns that could plausibly both match the same path was at least physically possible, even if nobody actually did it. Spread across however many controller classes a project has grown, each with its own route attributes sitting right next to the method they belong to, there's no single place left to read top to bottom and notice the overlap — the only way to see it is to already suspect it's there and go looking for it on purpose.

The fix that actually holds isn't relying on which controller sorts first — the order is predictable, but renaming a file shouldn't be able to change which page a URL shows. It's not leaving that question mattering at all, by making sure no two routes under the same prefix can both plausibly claim the same path in the first place, and by keeping every route for one resource in one controller so there's at least one obvious place to check.

Filed under

Leave a comment

Comments are reviewed before they appear.

Basic HTML allowed: links, emphasis, code and lists. Web addresses become links automatically.