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

[error] - monsieurbiz_search_search with POST empty throw 500 #201

Open
cbastienbaron opened this issue Dec 15, 2023 · 3 comments · May be fixed by #202
Open

[error] - monsieurbiz_search_search with POST empty throw 500 #201

cbastienbaron opened this issue Dec 15, 2023 · 3 comments · May be fixed by #202

Comments

@cbastienbaron
Copy link
Contributor

Hi ,

Facing an issue when a user post an empty search or try to discover some 500 on app

To reproduce

curl -I -XPOST http://127.0.0.1/fr_FR/search
HTTP/1.1 500 Internal Server Error
Server: nginx/1.21.6
... 

https://github.com/monsieurbiz/SyliusSearchPlugin/blob/master/src/Controller/SearchController.php#L102

    public function postAction(Request $request): RedirectResponse
    {
        $query = (array) $request->request->all()['monsieurbiz_searchplugin_search'] ?? [];
        $query = $query['query'] ?? '';

        return $this->redirect(
            $this->generateUrl(
                'monsieurbiz_search_search',
                ['query' => urlencode($query)]
            )
        );
    }

if query param no exist, an empty string is assigned $query but route monsieurbiz_search_search required 1 char minimum
https://github.com/monsieurbiz/SyliusSearchPlugin/blob/master/src/Resources/config/routing/shop.yaml#L7

monsieurbiz_search_search:
  path: /search/{query}
  methods: [GET]
  defaults:
    _controller: MonsieurBiz\SyliusSearchPlugin\Controller\SearchController::searchAction
  requirements:
    query: .+
  condition: "not(context.getPathInfo() matches '`^%sylius.security.new_api_route%`') and context.checkElasticsearch()"

a possible solution will be to authorize 0 to n char

monsieurbiz_search_search:
  path: /search/{query}
  methods: [GET]
  defaults:
    _controller: MonsieurBiz\SyliusSearchPlugin\Controller\SearchController::searchAction
  requirements:
    query: .*
  condition: "not(context.getPathInfo() matches '`^%sylius.security.new_api_route%`') and context.checkElasticsearch()"
cbastienbaron added a commit to cbastienbaron/SyliusSearchPlugin that referenced this issue Dec 15, 2023
@cbastienbaron cbastienbaron linked a pull request Dec 15, 2023 that will close this issue
@delyriand
Copy link
Member

delyriand commented Sep 23, 2024

Hi @cbastienbaron!

In fact, because of the monsieurbiz_search_post route, you can POST an empty value and this causes a 500 error.
Instead of the PR #202, I suggest generating a 404 in the postAction method if $query is empty.

For information, is it possible to reproduce this error with our test app : curl -I -XPOST https://localhost:8000/en_US/search

@cbastienbaron
Copy link
Contributor Author

Hey @delyriand 🖖
yup the goal is to avoid a 500 (empty search or 404 is fine to me 👍 )

@delyriand
Copy link
Member

@cbastienbaron,

I pushed a new commit on your PR to throw a 404

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 a pull request may close this issue.

2 participants