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

Cannot send numbers or bools as payloads #1607

Closed
3 tasks done
DonutEspresso opened this issue Feb 22, 2018 · 5 comments
Closed
3 tasks done

Cannot send numbers or bools as payloads #1607

DonutEspresso opened this issue Feb 22, 2018 · 5 comments

Comments

@DonutEspresso
Copy link
Member

  • 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

Bug Report

A regression of GH-800. You cannot send numbers or false even with an explicit status code. i.e.,

res.send(200, false);
res.send(200, true);
res.send(200, 0);

These all cause restify to blow up.

Restify Version

6.x

Node.js Version

All

Cause

Looks like this the arguments parsing in lib/response.js's __send method:

        if (
            typeof arguments[index] === 'object' ||
            typeof arguments[index] === 'string'
        ) {
            body = arguments[index++];
        }

The type checks explicitly disallow numbers and bools.

Are you willing and able to fix this?

Yes

@retrohacker
Copy link
Member

This is a duplicate of #1461 I believe

@retrohacker
Copy link
Member

I think we settled on fixing this, with the following caveat:

number is a special case in that there is a previous argument that is a number: statusCode. I'd err on the side of keeping the order of the args, in the case where you want to specify a number for body, you must always specify statusCode to disambiguate.

  • res.send(false);
  • res.send(200, false);
  • res.send(100); // status code of 100
  • res.send(200, 100); // status code of 200, body of 100

@retrohacker
Copy link
Member

So essentially statusCode is optional for everything except a number. If you provide a number as a first argument, it will always be a statusCode.

@DonutEspresso
Copy link
Member Author

DonutEspresso commented Feb 22, 2018

Yep, makes sense, I think we're just missing 2) and 4) from your list.

@DonutEspresso
Copy link
Member Author

Fixed by #1609

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