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

Losing params mapping on chained handler. #1050

Closed
meteormanaged opened this issue Mar 28, 2016 · 5 comments
Closed

Losing params mapping on chained handler. #1050

meteormanaged opened this issue Mar 28, 2016 · 5 comments

Comments

@meteormanaged
Copy link

Sorry for the extra logging in here. Just trying to show, as simply as possible, the few things I'm using (body parser, named handlers, chained handlers) and the results I'm seeing.

Parameters are being mapped by bodyParser. When a route "nexts" to a different named route, the params are no longer mapped.

'use strict';
const restify = require('restify');

const server = restify.createServer({
  name: 'bugtest'
});

server.use(restify.bodyParser({
  mapParams: true
}));

server.post({
  name: 'post.l2',
  path: '/:name/:secondary/'
}, (req, res, next) => {
  console.log('l2');
  console.log(req.params);
  if (!req.params.secondary) return next('post.l1');
  res.send(200);
  next();
});

server.post({
  name: 'post.l1',
  path: '/:name/'
}, (req, res, next) => {
  console.log('l1');
  console.log(req.params);
  res.send(200);
  next();
});

server.listen(8080, (err) => err ? console.log(err) : true);

// POST localhost:8080/thing/stuff
// 'l2'
// { name: 'thing', secondary: 'stuff' } <-- Correct req.params
//
// POST localhost:8080/thing/
// 'l2'
// { name: 'thing', secondary: '' } <-- Correct req.params
// 'l1'
// {} <--- body no longer mapped to req.params ?
@meteormanaged
Copy link
Author

Edit: this isn't really relevant, methinks.

If I add server.use(restify.sanitizePath()); (both before or after the bodyParser), the following is seen:

l2
{ name: 'thing', secondary: '' }
l1
{ name: 'thing' }

I would have thought that sanitizing the path would strip the trailing / and trigger post.1 in the router. Instead, it's still falling through and strangely giving a slightly changed req.params

Any help would be appreciated. $5 says I'm just doing something foolish.

@meteormanaged
Copy link
Author

Updated to handle the path sanitization issue, still produces the same result.

'use strict';
const restify = require('restify');

const server = restify.createServer({
  name: 'bugtest'
});
server.pre(restify.pre.sanitizePath());
server.pre(restify.bodyParser({
  mapParams: true
}));

server.post({
  name: 'post.l1',
  path: '/:name'
}, (req, res, next) => {
  console.log('l1');
  console.log(req.params);
  res.send(200);
  next();
});

server.post({
  name: 'post.l2',
  path: '/:name/:secondary'
}, (req, res, next) => {
  console.log('l2');
  console.log(req.params);
  return next('post.l1');

});

server.listen(8080, (err) => err ? console.log(err) : true);

// POST localhost:8080/thing/stuff
// l2
// { name: 'thing', secondary: 'stuff' }
// l1
// {}

@DonutEspresso
Copy link
Member

Apologies for the delayed reply - I've been OOO. It looks like the mapped params are being stomped on on redirects:

https://github.com/restify/node-restify/blob/5.x/lib/server.js#L902

I don't have enough context to know why this is being done. @yunong? To get around this for now, your best bet is to simply copy the mapped params to another property prior to a redirect.

@DonutEspresso
Copy link
Member

FYI, we will be deprecating this "internal redirection" feature starting 5.x in favor of something a little more comprehensive.

@retrohacker
Copy link
Member

This sounds like it is resolved in the 5.x release.

Closing this for now. If I have missed a nuance, please correct me and we can re-open ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants