restify / restify/node-restify

multi-parametric routing not populating req.params

Open
#1,874 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
10.7k
Forks
975
Avg merge
1d 6h
Merged PRs (30d)
5

Description

Hi. Thanks for a great library. I'll preface this by stating that I am upgrading some VERY legacy code (restify v4 to v8) and such have had a world of pain in doing so.

  • Used appropriate template for the issue type
  • Searched both open and closed issues for duplicates of this issue
  • Title adequately and concisely reflects the feature or the bug

Restify Version: 8.5.1
Node.js Version: 14.16.1

Expected behaviour

My req.params should be populated on all routes.

Actual behaviour

My req.params is only populated on a single route.

Repro case

const restify = require('restify')

const server = restify.createServer({})
const echoParams = (name) => (req, res, next) => {
  res.send(JSON.stringify({ params: req.params, name }))
}
server.get('/:width/:height/:mode/*', echoParams('mode'))
server.get('/:width/:height/*', echoParams('both'))
server.get('/:width/*', echoParams('width'))

server.listen(1234, () => {
  console.log('listen')
})

It does not matter what order I register my routes in, I get the same behaviour:

➜ curl localhost:1234/100/200/fill/test
"{\"params\":{\"width\":\"100\",\"height\":\"200\",\"mode\":\"fill\",\"*\":\"test\"},\"name\":\"mode\"}"%                                                                                                                                                       
➜ curl localhost:1234/100/200/test
"{\"params\":{\"*\":\"test\"},\"name\":\"both\"}"%
➜ curl localhost:1234/100/test
"{\"params\":{\"*\":\"test\"},\"name\":\"width\"}"%

Cause

I wish I knew.

EDIT: I looked at the code and dumped a ton of logging in there and can see the stuff returned here does not contain it either:
https://github.com/restify/node-restify/blob/master/lib/router.js#L84

I can't open an issue on find-my-way as this uses an extremely out of date version of it.

Are you willing and able to fix this?

Sure, if someone can point me in the right direction.

Double EDIT: I've abandoned restify because of this issue. I don't believe this package to be maintained or suitable anymore - express works just fine.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the route-matching code in lib/router.js around line 84 and reproduce the three routes from the issue using the provided Node.js example. Done means req.params contains width, height, and mode where those named parameters appear, including the shorter routes, with route behavior covered by a regression test.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.