[Converge] child_process argument type checking
rvagg opened this issue · 9 comments
Continuing from nodejs/node-convergence-archive#22, I don't believe this has been done yet, @jasnell can you confirm please? That thread seems to have enough agreement to pull these changes in. Marking on the 4.0.0 milestone.
These commits add argument type checking to methods within child_process. This needs to be reconciled with the current io.js behavior. The change introduces a new throw so there's a potential API compatibility issue.
/cc @trevnorris
a1b2875afd9affd3e0147a10f04a2daf8c598761 lint: fix lint issues- nodejs/node-v0.x-archive@a1b28758032a210250c86877898331ac4474f39b3a573c6 test: test all spawn parameter positions- nodejs/node-v0.x-archive@8032a2170dafa7b624abd43432e03304d65cc527fbecc11 child_process: check fork args is an array- nodejs/node-v0.x-archive@70dafa7e17c5a72b23f920f291d61f2780068c18768cb92 child_process: check execFile args is an array- nodejs/node-v0.x-archive@e17c5a7
Correct, AFAIK this has not yet been completed. It's actually on my list for later this week but if someone gets to it before then ..... ;-)
@sam-github @trevnorris ... this particular set of commits were yours originally, I believe. Would either of you be able to take a look to see what needs to land in nodejs/node?
lint fix doesn't matter. processing the arguments the same would be helpful, but not sure if it's currently done that way ATM. so may be worth bringing in.
@trevnorris I'm assigning this to you, please close this when it lands, before the end of the week please.
@rvagg want me to just cherry-pick the applicable onto master?
@trevnorris your call, including assessing whether this is even necessary, PR obviously even if cherry-picking.
ping @trevnorris, are we close here?
@rvagg @trevnorris ... I'm going to take a look at this today. I'll port over the changes and open a new PR against master