From 640912d18a63704fd493059a72b19e6367c2fc1c Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 24 Jan 2014 22:08:25 +0400 Subject: [PATCH] tls_wrap: propagate errors to write callbacks fix #6903 --- lib/net.js | 6 +- lib/util.js | 7 +- src/env.h | 1 + src/stream_wrap.cc | 23 ++++- src/stream_wrap.h | 1 + src/tls_wrap.cc | 139 ++++++++++++++++++++--------- src/tls_wrap.h | 15 +++- test/simple/test-tls-econnreset.js | 2 +- 8 files changed, 141 insertions(+), 53 deletions(-) diff --git a/lib/net.js b/lib/net.js index 38d90925b1..65255da26c 100644 --- a/lib/net.js +++ b/lib/net.js @@ -651,7 +651,7 @@ Socket.prototype._writeGeneric = function(writev, data, encoding, cb) { } if (err) - return this._destroy(errnoException(err, 'write'), cb); + return this._destroy(errnoException(err, 'write', req.error), cb); this._bytesDispatched += req.bytes; @@ -745,7 +745,7 @@ Socket.prototype.__defineGetter__('bytesWritten', function() { }); -function afterWrite(status, handle, req) { +function afterWrite(status, handle, req, err) { var self = handle.owner; if (self !== process.stderr && self !== process.stdout) debug('afterWrite', status); @@ -757,7 +757,7 @@ function afterWrite(status, handle, req) { } if (status < 0) { - var ex = errnoException(status, 'write'); + var ex = errnoException(status, 'write', err); debug('write failure', ex); self._destroy(ex, req.cb); return; diff --git a/lib/util.js b/lib/util.js index e3f47234de..de2246cc49 100644 --- a/lib/util.js +++ b/lib/util.js @@ -677,10 +677,13 @@ exports.pump = exports.deprecate(function(readStream, writeStream, callback) { var uv; -exports._errnoException = function(err, syscall) { +exports._errnoException = function(err, syscall, original) { if (isUndefined(uv)) uv = process.binding('uv'); var errname = uv.errname(err); - var e = new Error(syscall + ' ' + errname); + var message = syscall + ' ' + errname; + if (original) + message += ' ' + original; + var e = new Error(message); e.code = errname; e.errno = errname; e.syscall = syscall; diff --git a/src/env.h b/src/env.h index e8796e73ae..23a3959d7b 100644 --- a/src/env.h +++ b/src/env.h @@ -69,6 +69,7 @@ namespace node { V(domain_string, "domain") \ V(enter_string, "enter") \ V(errno_string, "errno") \ + V(error_string, "error") \ V(exit_string, "exit") \ V(exponent_string, "exponent") \ V(exports_string, "exports") \ diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 3b361f7a05..e0079b8136 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -215,6 +215,9 @@ void StreamWrap::WriteBuffer(const FunctionCallbackInfo& args) { req_wrap->Dispatched(); req_wrap_obj->Set(env->bytes_string(), Integer::NewFromUnsigned(length, node_isolate)); + const char* msg = wrap->callbacks()->Error(); + if (msg != NULL) + req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg)); if (err) { req_wrap->~WriteWrap(); @@ -300,6 +303,9 @@ void StreamWrap::WriteStringImpl(const FunctionCallbackInfo& args) { req_wrap->Dispatched(); req_wrap->object()->Set(env->bytes_string(), Integer::NewFromUnsigned(data_size, node_isolate)); + const char* msg = wrap->callbacks()->Error(); + if (msg != NULL) + req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg)); if (err) { req_wrap->~WriteWrap(); @@ -401,6 +407,9 @@ void StreamWrap::Writev(const FunctionCallbackInfo& args) { req_wrap->Dispatched(); req_wrap->object()->Set(env->bytes_string(), Number::New(node_isolate, bytes)); + const char* msg = wrap->callbacks()->Error(); + if (msg != NULL) + req_wrap_obj->Set(env->error_string(), OneByteString(env->isolate(), msg)); if (err) { req_wrap->~WriteWrap(); @@ -441,14 +450,19 @@ void StreamWrap::AfterWrite(uv_write_t* req, int status) { // Unref handle property Local req_wrap_obj = req_wrap->object(); req_wrap_obj->Delete(env->handle_string()); - wrap->callbacks_->AfterWrite(req_wrap); + wrap->callbacks()->AfterWrite(req_wrap); Local argv[] = { Integer::New(status, node_isolate), wrap->object(), - req_wrap_obj + req_wrap_obj, + Undefined() }; + const char* msg = wrap->callbacks()->Error(); + if (msg != NULL) + argv[3] = OneByteString(env->isolate(), msg); + req_wrap->MakeCallback(env->oncomplete_string(), ARRAY_SIZE(argv), argv); req_wrap->~WriteWrap(); @@ -499,6 +513,11 @@ void StreamWrap::AfterShutdown(uv_shutdown_t* req, int status) { } +const char* StreamWrapCallbacks::Error() { + return NULL; +} + + int StreamWrapCallbacks::DoWrite(WriteWrap* w, uv_buf_t* bufs, size_t count, diff --git a/src/stream_wrap.h b/src/stream_wrap.h index ed8c53ebe8..d1a94fb422 100644 --- a/src/stream_wrap.h +++ b/src/stream_wrap.h @@ -73,6 +73,7 @@ class StreamWrapCallbacks { virtual ~StreamWrapCallbacks() { } + virtual const char* Error(); virtual int DoWrite(WriteWrap* w, uv_buf_t* bufs, size_t count, diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 45be55ced7..14ea95c002 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -58,6 +58,10 @@ static const int X509_NAME_FLAGS = ASN1_STRFLGS_ESC_CTRL | XN_FLAG_FN_SN; +size_t TLSCallbacks::error_off_; +char TLSCallbacks::error_buf_[1024]; + + TLSCallbacks::TLSCallbacks(Environment* env, Kind kind, Handle sc, @@ -71,15 +75,16 @@ TLSCallbacks::TLSCallbacks(Environment* env, enc_out_(NULL), clear_in_(NULL), write_size_(0), - pending_write_item_(NULL), started_(false), established_(false), shutdown_(false), + error_(NULL), eof_(false) { node::Wrap(object(), this); // Initialize queue for clearIn writes QUEUE_INIT(&write_item_queue_); + QUEUE_INIT(&pending_write_items_); // We've our own session callbacks SSL_CTX_sess_set_get_cb(sc_->ctx_, SSLWrap::GetSessionCallback); @@ -102,25 +107,52 @@ TLSCallbacks::~TLSCallbacks() { #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB sni_context_.Dispose(); #endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB + + // Move all writes to pending + MakePending(); + + // And destroy + while (!QUEUE_EMPTY(&pending_write_items_)) { + QUEUE* q = QUEUE_HEAD(&pending_write_items_); + QUEUE_REMOVE(q); + + WriteItem* wi = QUEUE_DATA(q, WriteItem, member_); + delete wi; + } } -void TLSCallbacks::InvokeQueued(int status) { - // Empty queue - ignore call - if (pending_write_item_ == NULL) +void TLSCallbacks::MakePending() { + // Aliases + QUEUE* from = &write_item_queue_; + QUEUE* to = &pending_write_items_; + + if (QUEUE_EMPTY(from)) return; - QUEUE* q = &pending_write_item_->member_; - pending_write_item_ = NULL; + // Add items to pending + QUEUE_ADD(to, from); + + // Empty original queue + QUEUE_INIT(from); +} + + +bool TLSCallbacks::InvokeQueued(int status) { + if (QUEUE_EMPTY(&pending_write_items_)) + return false; // Process old queue - while (q != &write_item_queue_) { - QUEUE* next = static_cast(QUEUE_NEXT(q)); - WriteItem* wi = CONTAINER_OF(q, WriteItem, member_); + while (!QUEUE_EMPTY(&pending_write_items_)) { + QUEUE* q = QUEUE_HEAD(&pending_write_items_); + QUEUE_REMOVE(q); + + WriteItem* wi = QUEUE_DATA(q, WriteItem, member_); wi->cb_(&wi->w_->req_, status); delete wi; - q = next; } + + return true; } @@ -276,16 +308,13 @@ void TLSCallbacks::EncOut() { return; // Split-off queue - if (established_ && !QUEUE_EMPTY(&write_item_queue_)) { - pending_write_item_ = CONTAINER_OF(QUEUE_NEXT(&write_item_queue_), - WriteItem, - member_); - QUEUE_INIT(&write_item_queue_); - } + if (established_ && !QUEUE_EMPTY(&write_item_queue_)) + MakePending(); // No data to write if (BIO_pending(enc_out_) == 0) { - InvokeQueued(0); + if (clear_in_->Length() == 0) + InvokeQueued(0); return; } @@ -314,7 +343,6 @@ void TLSCallbacks::EncOut() { void TLSCallbacks::EncOutCb(uv_write_t* req, int status) { TLSCallbacks* callbacks = static_cast(req->data); - Environment* env = callbacks->env(); // Handle error if (status) { @@ -323,12 +351,6 @@ void TLSCallbacks::EncOutCb(uv_write_t* req, int status) { return; // Notify about error - HandleScope handle_scope(env->isolate()); - Context::Scope context_scope(env->context()); - Local arg = String::Concat( - FIXED_ONE_BYTE_STRING(node_isolate, "write cb error, status: "), - Integer::New(status, node_isolate)->ToString()); - callbacks->MakeCallback(env->onerror_string(), 1, &arg); callbacks->InvokeQueued(status); return; } @@ -342,7 +364,33 @@ void TLSCallbacks::EncOutCb(uv_write_t* req, int status) { } -Local TLSCallbacks::GetSSLError(int status, int* err) { +int TLSCallbacks::PrintErrorsCb(const char* str, size_t len, void* arg) { + size_t to_copy = error_off_; + size_t avail = sizeof(error_buf_) - error_off_ - 1; + + if (avail > to_copy) + to_copy = avail; + + memcpy(error_buf_, str, avail); + error_off_ += avail; + assert(error_off_ < sizeof(error_buf_)); + + // Zero-terminate + error_buf_[error_off_] = '\0'; + + return 0; +} + + +const char* TLSCallbacks::PrintErrors() { + error_off_ = 0; + ERR_print_errors_cb(PrintErrorsCb, this); + + return error_buf_; +} + + +Local TLSCallbacks::GetSSLError(int status, int* err, const char** msg) { HandleScope scope(node_isolate); *err = SSL_get_error(ssl_, status); @@ -356,19 +404,18 @@ Local TLSCallbacks::GetSSLError(int status, int* err) { break; default: { - BUF_MEM* mem; - BIO* bio; - assert(*err == SSL_ERROR_SSL || *err == SSL_ERROR_SYSCALL); - bio = BIO_new(BIO_s_mem()); - assert(bio != NULL); - ERR_print_errors(bio); - BIO_get_mem_ptr(bio, &mem); + const char* buf = PrintErrors(); + Local message = - OneByteString(node_isolate, mem->data, mem->length); + OneByteString(node_isolate, buf, strlen(buf)); Local exception = Exception::Error(message); - BIO_free_all(bio); + + if (msg != NULL) { + assert(*msg == NULL); + *msg = buf; + } return scope.Close(exception); } @@ -409,7 +456,7 @@ void TLSCallbacks::ClearOut() { if (read == -1) { int err; - Handle arg = GetSSLError(read, &err); + Local arg = GetSSLError(read, &err, NULL); if (!arg.IsEmpty()) { MakeCallback(env()->onerror_string(), 1, &arg); @@ -445,15 +492,25 @@ bool TLSCallbacks::ClearIn() { // Error or partial write int err; - Handle arg = GetSSLError(written, &err); + Local arg = GetSSLError(written, &err, &error_); if (!arg.IsEmpty()) { - MakeCallback(env()->onerror_string(), 1, &arg); + MakePending(); + if (!InvokeQueued(UV_EPROTO)) + error_ = NULL; + clear_in_->Reset(); } return false; } +const char* TLSCallbacks::Error() { + const char* ret = error_; + error_ = NULL; + return ret; +} + + int TLSCallbacks::DoWrite(WriteWrap* w, uv_buf_t* bufs, size_t count, @@ -508,11 +565,9 @@ int TLSCallbacks::DoWrite(WriteWrap* w, int err; HandleScope handle_scope(env()->isolate()); Context::Scope context_scope(env()->context()); - Handle arg = GetSSLError(written, &err); - if (!arg.IsEmpty()) { - MakeCallback(env()->onerror_string(), 1, &arg); - return -1; - } + Local arg = GetSSLError(written, &err, &error_); + if (!arg.IsEmpty()) + return UV_EPROTO; // No errors, queue rest for (; i < count; i++) diff --git a/src/tls_wrap.h b/src/tls_wrap.h index c1febe0e70..db78009ede 100644 --- a/src/tls_wrap.h +++ b/src/tls_wrap.h @@ -50,6 +50,7 @@ class TLSCallbacks : public crypto::SSLWrap, v8::Handle unused, v8::Handle context); + const char* Error(); int DoWrite(WriteWrap* w, uv_buf_t* bufs, size_t count, @@ -98,7 +99,8 @@ class TLSCallbacks : public crypto::SSLWrap, static void EncOutCb(uv_write_t* req, int status); bool ClearIn(); void ClearOut(); - void InvokeQueued(int status); + void MakePending(); + bool InvokeQueued(int status); inline void Cycle() { ClearIn(); @@ -106,7 +108,10 @@ class TLSCallbacks : public crypto::SSLWrap, EncOut(); } - v8::Local GetSSLError(int status, int* err); + v8::Local GetSSLError(int status, int* err, const char** msg); + const char* PrintErrors(); + + static int PrintErrorsCb(const char* str, size_t len, void* arg); static void OnClientHelloParseEnd(void* arg); static void Wrap(const v8::FunctionCallbackInfo& args); @@ -133,10 +138,11 @@ class TLSCallbacks : public crypto::SSLWrap, size_t write_size_; size_t write_queue_size_; QUEUE write_item_queue_; - WriteItem* pending_write_item_; + QUEUE pending_write_items_; bool started_; bool established_; bool shutdown_; + const char* error_; // If true - delivered EOF to the js-land, either after `close_notify`, or // after the `UV_EOF` on socket. @@ -145,6 +151,9 @@ class TLSCallbacks : public crypto::SSLWrap, #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB v8::Persistent sni_context_; #endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB + + static size_t error_off_; + static char error_buf_[1024]; }; } // namespace node diff --git a/test/simple/test-tls-econnreset.js b/test/simple/test-tls-econnreset.js index 88afcdfbb5..fcadf13f0c 100644 --- a/test/simple/test-tls-econnreset.js +++ b/test/simple/test-tls-econnreset.js @@ -83,7 +83,7 @@ var server = tls.createServer({ ca: ca, cert: cert, key: key }, function(conn) { connectError = err; this.destroy(); server.close(); - }); + }).write('123'); }); process.on('exit', function() {