Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should Route Requirements be Surround with ^...$ in Path Schema Patterns? #1873

Open
chrisguitarguy opened this issue Sep 14, 2021 · 1 comment

Comments

@chrisguitarguy
Copy link
Collaborator

chrisguitarguy commented Sep 14, 2021

Given a route like this:

    <route
        id="users.view"
        methods="GET"
        path="/users/{userId}"
        controller="App\Controller\UsersController::view">
            <requirement key="userId">[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}</requirement>
    </route>

an open API schema gets generated where the path parameter looks like this:

parameters:
  -
    name: userId
    in: path
    required: true
    schema:
      type: string
      pattern: '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}'

But the pattern docs say that without ^ and $ the pattern is a partial match, not a full string.

For a route, where the generated regex would require the specific, full string it might make sense to change the route describer to surround the requirement with ^...$.

Here's the regex that symfony generates on that route, for context:

Screen Shot 2021-09-14 at 10 58 10 AM

Thoughts?

@vodevel
Copy link

vodevel commented Mar 2, 2023

Thank you, @chrisguitarguy! Absolutely make sense!

The problem cannot even be solved by manual adding ^...$ like

#[Route('route/{id}', requirements: ['id' =>  '^\d+$'])]
// or
#[Route('route/{id<^\d+$>}')]

Due to Symfony Router converts all these variants to {^/route/(?P<id>\d+)$}sDu

Therefore, it is worth adding boundary conditions for correct UI validation. Is there any reason not to do this, other than not having a pull request?)

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

No branches or pull requests

2 participants