Skip to content

Conversation

@rlidwka
Copy link
Contributor

@rlidwka rlidwka commented Apr 9, 2015

This works:

$ iojs -e 'new (require("readline").Interface)({ input: process.stdin, output: process.stdout })'

This doesn't:

$ iojs -e 'require("readline").Interface({ input: process.stdin, output: process.stdout })'
readline.js:98
    input.on('data', ondata);
          ^
TypeError: undefined is not a function
    at new Interface (readline.js:98:11)
    at Object.Interface (readline.js:29:12)
    at [eval]:1:21
    at Object.exports.runInThisContext (vm.js:54:17)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:446:26)
    at evalScript (node.js:413:25)
    at startup (node.js:72:7)
    at node.js:799:3

The reason is that option object detection here relies on a number of arguments passed to the constructor.

But instanceof guard here always makes it four arguments.


So the issue happens when usual instanceof check is combined with logic based on arguments.length.

There are two places in io.js core when it matters. Second one is in buffer.js and it's already handled correctly.

@silverwind silverwind added the readline Issues and PRs related to the built-in readline module. label Apr 9, 2015
lib/readline.js Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: use const to keep it lexically scoped.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to const in the next commit

@bnoordhuis
Copy link
Member

LGTM with a suggestion.

Previously, we detected options object based on amount of arguments
supplied. But if we're calling readline without new operator,
constructor gets re-called and will always have 4 arguments.
bnoordhuis pushed a commit that referenced this pull request Apr 10, 2015
Previously, we detected options object based on amount of arguments
supplied. But if we're calling readline without new operator,
constructor gets re-called and will always have 4 arguments.

PR-URL: #1385
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@bnoordhuis
Copy link
Member

Cheers @rlidwka, landed in f0bf6bb.

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

Labels

readline Issues and PRs related to the built-in readline module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants