Skip to content

Commit

Permalink
tls_wrap: reach error reporting for UV_EPROTO
Browse files Browse the repository at this point in the history
Do not swallow error details when reporting UV_EPROTO asynchronously,
and when creating artificial errors.

Fix: #3692
PR-URL: #4885
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
indutny authored and rvagg committed Feb 8, 2016
1 parent f7ed473 commit d6fbd81
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 11 deletions.
2 changes: 1 addition & 1 deletion lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 9 additions & 2 deletions src/stream_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,15 @@ class StreamReq {
explicit StreamReq(DoneCb cb) : cb_(cb) {
}

inline void Done(int status) {
cb_(static_cast<Req*>(this), status);
inline void Done(int status, const char* error_str = nullptr) {
Req* req = static_cast<Req*>(this);
Environment* env = req->env();
if (error_str != nullptr) {
req->object()->Set(env->error_string(),
OneByteString(env->isolate(), error_str));
}

cb_(req, status);
}

private:
Expand Down
22 changes: 15 additions & 7 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,15 @@ void TLSWrap::MakePending() {
}


bool TLSWrap::InvokeQueued(int status) {
bool TLSWrap::InvokeQueued(int status, const char* error_str) {
if (pending_write_items_.IsEmpty())
return false;

// Process old queue
WriteItemList queue;
pending_write_items_.MoveBack(&queue);
while (WriteItem* wi = queue.PopFront()) {
wi->w_->Done(status);
wi->w_->Done(status, error_str);
delete wi;
}

Expand Down Expand Up @@ -484,11 +484,12 @@ bool TLSWrap::ClearIn() {

// Error or partial write
int err;
Local<Value> arg = GetSSLError(written, &err, &error_);
const char* error_str = nullptr;
Local<Value> 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();
}

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -775,7 +783,7 @@ void TLSWrap::DestroySSL(const FunctionCallbackInfo<Value>& 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<TLSWrap>::DestroySSL();
Expand Down
2 changes: 1 addition & 1 deletion src/tls_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions test/parallel/test-tls-junk-server.js
Original file line number Diff line number Diff line change
@@ -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();
}));
});

0 comments on commit d6fbd81

Please sign in to comment.