From d6fbd81a7a8d5f733edfe09ac330d78fa83bbc51 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 26 Jan 2016 15:01:12 -0500 Subject: [PATCH] tls_wrap: reach error reporting for UV_EPROTO Do not swallow error details when reporting UV_EPROTO asynchronously, and when creating artificial errors. Fix: #3692 PR-URL: https://github.com/nodejs/node/pull/4885 Reviewed-By: Shigeki Ohtsu Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis --- lib/net.js | 2 +- src/stream_base.h | 11 ++++++++-- src/tls_wrap.cc | 22 +++++++++++++------- src/tls_wrap.h | 2 +- test/parallel/test-tls-junk-server.js | 29 +++++++++++++++++++++++++++ 5 files changed, 55 insertions(+), 11 deletions(-) create mode 100644 test/parallel/test-tls-junk-server.js diff --git a/lib/net.js b/lib/net.js index 7a612096ab45d0..d4d9abb38cebd6 100644 --- a/lib/net.js +++ b/lib/net.js @@ -760,7 +760,7 @@ function afterWrite(status, handle, req, err) { } if (status < 0) { - var ex = exceptionWithHostPort(status, 'write', req.address, req.port); + var ex = errnoException(status, 'write', req.error); debug('write failure', ex); self._destroy(ex, req.cb); return; diff --git a/src/stream_base.h b/src/stream_base.h index d0e1d4b9b253d5..fad2ddd2e086f0 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -22,8 +22,15 @@ class StreamReq { explicit StreamReq(DoneCb cb) : cb_(cb) { } - inline void Done(int status) { - cb_(static_cast(this), status); + inline void Done(int status, const char* error_str = nullptr) { + Req* req = static_cast(this); + Environment* env = req->env(); + if (error_str != nullptr) { + req->object()->Set(env->error_string(), + OneByteString(env->isolate(), error_str)); + } + + cb_(req, status); } private: diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index d7bf4ed8bee784..85730b34936b55 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -92,7 +92,7 @@ void TLSWrap::MakePending() { } -bool TLSWrap::InvokeQueued(int status) { +bool TLSWrap::InvokeQueued(int status, const char* error_str) { if (pending_write_items_.IsEmpty()) return false; @@ -100,7 +100,7 @@ bool TLSWrap::InvokeQueued(int status) { WriteItemList queue; pending_write_items_.MoveBack(&queue); while (WriteItem* wi = queue.PopFront()) { - wi->w_->Done(status); + wi->w_->Done(status, error_str); delete wi; } @@ -484,11 +484,12 @@ bool TLSWrap::ClearIn() { // Error or partial write int err; - Local arg = GetSSLError(written, &err, &error_); + const char* error_str = nullptr; + Local arg = GetSSLError(written, &err, &error_str); if (!arg.IsEmpty()) { MakePending(); - if (!InvokeQueued(UV_EPROTO)) - ClearError(); + InvokeQueued(UV_EPROTO, error_str); + delete[] error_str; clear_in_->Reset(); } @@ -589,8 +590,15 @@ int TLSWrap::DoWrite(WriteWrap* w, return 0; } - if (ssl_ == nullptr) + if (ssl_ == nullptr) { + ClearError(); + + static char msg[] = "Write after DestroySSL"; + char* tmp = new char[sizeof(msg)]; + memcpy(tmp, msg, sizeof(msg)); + error_ = tmp; return UV_EPROTO; + } crypto::MarkPopErrorOnReturn mark_pop_error_on_return; @@ -775,7 +783,7 @@ void TLSWrap::DestroySSL(const FunctionCallbackInfo& args) { wrap->MakePending(); // And destroy - wrap->InvokeQueued(UV_ECANCELED); + wrap->InvokeQueued(UV_ECANCELED, "Canceled because of SSL destruction"); // Destroy the SSL structure and friends wrap->SSLWrap::DestroySSL(); diff --git a/src/tls_wrap.h b/src/tls_wrap.h index 31d19523a62d08..471a92056dd848 100644 --- a/src/tls_wrap.h +++ b/src/tls_wrap.h @@ -89,7 +89,7 @@ class TLSWrap : public AsyncWrap, bool ClearIn(); void ClearOut(); void MakePending(); - bool InvokeQueued(int status); + bool InvokeQueued(int status, const char* error_str = nullptr); inline void Cycle() { // Prevent recursion diff --git a/test/parallel/test-tls-junk-server.js b/test/parallel/test-tls-junk-server.js new file mode 100644 index 00000000000000..4eb8129a11dfd6 --- /dev/null +++ b/test/parallel/test-tls-junk-server.js @@ -0,0 +1,29 @@ +'use strict'; +const common = require('../common'); + +if (!common.hasCrypto) { + console.log('1..0 # Skipped: missing crypto'); + return; +} + +const assert = require('assert'); +const https = require('https'); +const net = require('net'); + +const server = net.createServer(function(s) { + s.once('data', function() { + s.end('I was waiting for you, hello!', function() { + s.destroy(); + }); + }); +}); + +server.listen(common.PORT, function() { + const req = https.request({ port: common.PORT }); + req.end(); + + req.once('error', common.mustCall(function(err) { + assert(/unknown protocol/.test(err.message)); + server.close(); + })); +});