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

customRoutes will match full routes including query params #1206

Closed
5 of 10 tasks
moustacheful opened this issue Nov 11, 2021 · 2 comments
Closed
5 of 10 tasks

customRoutes will match full routes including query params #1206

moustacheful opened this issue Nov 11, 2021 · 2 comments
Assignees
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented
Milestone

Comments

@moustacheful
Copy link
Contributor

Description

Adding a customRoute which can receive query params will always fail since the match is done against the full URL, which includes the query params part.

What type of issue is this? (place an x in one of the [ ])

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • example code related
  • testing related
  • discussion

Requirements (place an x in each of the [ ])

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Reproducible in:

package version: any

node version: any

OS version(s): any

Steps to reproduce:

  1. Add a custom route
    This means adding something like:
    {
      path: '/say-hi',
      method: ['GET'],
      handler(req, res) {
        const params = qs.parse(req.url)
        res.end(`hello, ${params.name || 'world'}`)
      },
    }
  1. Do a request against our shiny new route with query params
  2. cry

Expected result:

What you expected to happen

curl localhost:3000/say-hi?name=doug%20dimmadome # hello, doug dimmadome

Actual result:

curl localhost:3000/say-hi?name=doug%20dimmadome # 404
@misscoded misscoded added the bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented label Nov 11, 2021
@misscoded
Copy link
Contributor

Hi @moustacheful! Great catch: we certainly overlooked this use case and this is definitely not the intended behavior. Thanks for not only providing us sufficient detail but for also opening a PR to introduce a fix! 🙏🏼

@seratch seratch added this to the 3.9.0 milestone Nov 11, 2021
@seratch
Copy link
Member

seratch commented Nov 16, 2021

Fixed by #1207

@seratch seratch closed this as completed Nov 16, 2021
@seratch seratch self-assigned this Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: confirmed bug report. Issues are confirmed when the reproduction steps are documented
Projects
None yet
Development

No branches or pull requests

3 participants