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

Node crashes for corner case with streams / readline #12152

Closed
rgrannell1 opened this issue Mar 31, 2017 · 2 comments
Closed

Node crashes for corner case with streams / readline #12152

rgrannell1 opened this issue Mar 31, 2017 · 2 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. stream Issues and PRs related to the stream subsystem.

Comments

@rgrannell1
Copy link

  • version: v7.8.0
  • platform: Linux ryan-XPS-13-9360 4.4.0-71-generic #92-Ubuntu SMP Fri Mar 24 12:59:01 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
  • subsystem: stream, readline
  • minimal failing example:
require('readline').createInterface({
	input:    process.stdin,
	output:   require('stream').Writable({
		write: (chunk, encoding, callback) => {
		    process.stdout.write('broken', encoding);
			callback( )
		}
	}),
	terminal: true
}).question('this will fail', _ => { })

I wish I could explain why this code fails, but I'm not entirely sure. At a glance it looks like a type-error occurs in V8 due to a missing check for the buffer-type in Node.js:

/usr/bin/nodejs[27737]: ../src/stream_base.cc:192:int node::StreamBase::WriteBuffer(const v8::FunctionCallbackInfo<v8::Value>&): Assertion `Buffer::HasInstance(args[1])' failed.
 1: node::Abort() [node]
 2: node::Assert(char const* const (*) [4]) [node]
 3: node::StreamBase::WriteBuffer(v8::FunctionCallbackInfo<v8::Value> const&) [node]
 4: void node::StreamBase::JSMethod<node::StreamWrap, &node::StreamBase::WriteBuffer>(v8::FunctionCallbackInfo<v8::Value> const&) [node]
 5: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) [node]
 6: 0xabfbed [node]
 7: v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) [node]
 8: 0x1264281843a7

If you need any more details let me know

@mscdex mscdex added c++ Issues and PRs that require attention from people who are familiar with C++. readline Issues and PRs related to the built-in readline module. v7.x labels Mar 31, 2017
@addaleax addaleax added the stream Issues and PRs related to the stream subsystem. label Mar 31, 2017
@addaleax
Copy link
Member

This should be a reduced reproduction: process.stdout.write('broken', 'buffer')

Basically, you are passing a string to the stream and claiming it’s a buffer. There could probably be some typechecking for that so that the process doesn’t crash, but that’s it.

@addaleax addaleax removed readline Issues and PRs related to the built-in readline module. v7.x labels Mar 31, 2017
@addaleax
Copy link
Member

PR to fix: #12753

addaleax added a commit to addaleax/node that referenced this issue May 3, 2017
Turn a `CHECK()` that could be brought to fail using public APIs
into throwing an error.

Fixes: nodejs#12152
anchnk pushed a commit to anchnk/node that referenced this issue May 6, 2017
Turn a `CHECK()` that could be brought to fail using public APIs
into throwing an error.

Fixes: nodejs#12152
PR-URL: nodejs#12753
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
gibfahn pushed a commit that referenced this issue Jun 18, 2017
Turn a `CHECK()` that could be brought to fail using public APIs
into throwing an error.

Fixes: #12152
PR-URL: #12753
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
gibfahn pushed a commit that referenced this issue Jun 20, 2017
Turn a `CHECK()` that could be brought to fail using public APIs
into throwing an error.

Fixes: #12152
PR-URL: #12753
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 11, 2017
Turn a `CHECK()` that could be brought to fail using public APIs
into throwing an error.

Fixes: #12152
PR-URL: #12753
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants