Skip to content

Commit

Permalink
src: replace manual memory mgmt with std::string
Browse files Browse the repository at this point in the history
PR-URL: nodejs/node#15782
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
bnoordhuis authored and addaleax committed Oct 11, 2017
1 parent 6227d12 commit cb46497
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 25 deletions.
31 changes: 9 additions & 22 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ TLSWrap::TLSWrap(Environment* env,
started_(false),
established_(false),
shutdown_(false),
error_(nullptr),
cycle_depth_(0),
eof_(false) {
node::Wrap(object(), this);
Expand Down Expand Up @@ -103,8 +102,6 @@ TLSWrap::~TLSWrap() {
#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
sni_context_.Reset();
#endif // SSL_CTRL_SET_TLSEXT_SERVERNAME_CB

ClearError();
}


Expand Down Expand Up @@ -367,7 +364,7 @@ void TLSWrap::EncOutCb(WriteWrap* req_wrap, int status) {
}


Local<Value> TLSWrap::GetSSLError(int status, int* err, const char** msg) {
Local<Value> TLSWrap::GetSSLError(int status, int* err, std::string* msg) {
EscapableHandleScope scope(env()->isolate());

// ssl_ is already destroyed in reading EOF by close notify alert.
Expand Down Expand Up @@ -398,13 +395,9 @@ Local<Value> TLSWrap::GetSSLError(int status, int* err, const char** msg) {
OneByteString(env()->isolate(), mem->data, mem->length);
Local<Value> exception = Exception::Error(message);

if (msg != nullptr) {
CHECK_EQ(*msg, nullptr);
char* const buf = new char[mem->length + 1];
memcpy(buf, mem->data, mem->length);
buf[mem->length] = '\0';
*msg = buf;
}
if (msg != nullptr)
msg->assign(mem->data, mem->data + mem->length);

BIO_free_all(bio);

return scope.Escape(exception);
Expand Down Expand Up @@ -516,12 +509,11 @@ bool TLSWrap::ClearIn() {

// Error or partial write
int err;
const char* error_str = nullptr;
std::string error_str;
Local<Value> arg = GetSSLError(written, &err, &error_str);
if (!arg.IsEmpty()) {
MakePending();
InvokeQueued(UV_EPROTO, error_str);
delete[] error_str;
InvokeQueued(UV_EPROTO, error_str.c_str());
clear_in_->Reset();
}

Expand Down Expand Up @@ -570,13 +562,12 @@ int TLSWrap::ReadStop() {


const char* TLSWrap::Error() const {
return error_;
return error_.empty() ? nullptr : error_.c_str();
}


void TLSWrap::ClearError() {
delete[] error_;
error_ = nullptr;
error_.clear();
}


Expand Down Expand Up @@ -624,11 +615,7 @@ int TLSWrap::DoWrite(WriteWrap* w,

if (ssl_ == nullptr) {
ClearError();

static char msg[] = "Write after DestroySSL";
char* tmp = new char[sizeof(msg)];
memcpy(tmp, msg, sizeof(msg));
error_ = tmp;
error_ = "Write after DestroySSL";
return UV_EPROTO;
}

Expand Down
7 changes: 4 additions & 3 deletions src/tls_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@

#include <openssl/ssl.h>

#include <string>

namespace node {

// Forward-declarations
Expand Down Expand Up @@ -148,8 +150,7 @@ class TLSWrap : public AsyncWrap,

void DoRead(ssize_t nread, const uv_buf_t* buf, uv_handle_type pending);

// If |msg| is not nullptr, caller is responsible for calling `delete[] *msg`.
v8::Local<v8::Value> GetSSLError(int status, int* err, const char** msg);
v8::Local<v8::Value> GetSSLError(int status, int* err, std::string* msg);

static void OnClientHelloParseEnd(void* arg);
static void Wrap(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down Expand Up @@ -180,7 +181,7 @@ class TLSWrap : public AsyncWrap,
bool started_;
bool established_;
bool shutdown_;
const char* error_;
std::string error_;
int cycle_depth_;

// If true - delivered EOF to the js-land, either after `close_notify`, or
Expand Down

0 comments on commit cb46497

Please sign in to comment.