Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
137 commits
Select commit Hold shift + click to select a range
57b5b47
crypto: better crypto error messages
gla5001 Aug 10, 2017
1c695af
Review updates and tests
gla5001 Aug 11, 2017
ed9f9b5
Additional review updates.
gla5001 Aug 15, 2017
a72d7e1
Review update. Use the overload of Set()
gla5001 Aug 15, 2017
0538b25
Nit fix: changing local variable to snake_case
gla5001 Aug 16, 2017
cb44cd4
test: fix single test runner regression
TimothyGu Sep 11, 2017
3f7813b
doc: update AUTHORS list
targos Sep 4, 2017
6ff521b
errors: eliminate circular dependency on assert
jasnell Aug 24, 2017
8c2eba0
test: improve process warning coverage
jasnell Sep 5, 2017
2510500
meta: allow vague objections to be dismissed
jasnell Sep 6, 2017
6ccb9fe
errors: fix ERR_MODULE_RESOLUTION_LEGACY message
tniessen Sep 9, 2017
da057db
doc: fix some internal links
vsemozhetbyt Sep 9, 2017
c981483
http2: cleanup of h2 compat layer, add tests
apapirovski Sep 12, 2017
a10856a
2017-09-12, Version 8.5.0 (Current)
MylesBorins Sep 10, 2017
e13d1df
assert: support custom errors
geek Sep 10, 2017
22ae8c0
assert: fix boxed primitives in deepStrictEqual
BridgeAR Aug 27, 2017
c1fce1e
doc: update README with SHASUMS256.txt.sig info
maclover7 Aug 31, 2017
cb94905
n-api: stop creating references to primitives
Sep 9, 2017
b4b7ac6
doc: fix nits in esm.md
vsemozhetbyt Sep 10, 2017
d82e107
deps: update V8 to 6.1.534.36
targos Sep 12, 2017
ddc16e5
src: update NODE_MODULE_VERSION to 58
targos Sep 12, 2017
c6e165b
deps: limit regress/regress-crbug-514081 v8 test
mhdawson May 9, 2016
694ef89
deps: fix addons compilation with VS2013
bzoz May 23, 2017
e202d85
deps: cherry-pick f19b889 from upstream V8
targos Aug 16, 2017
e1c37b3
deps: backport 6e9e2e5 from upstream V8
Aug 3, 2017
5976e0f
deps: backport bca8409 from upstream V8
Aug 3, 2017
e55b7f3
deps: backport f9c4b7a from upstream V8
Aug 3, 2017
bd6907b
deps: cherry-pick e020aae394 from V8 upstream
bnoordhuis Aug 17, 2017
9c3182e
deps: cherry-pick 1aead19 from upstream V8
bnoordhuis Aug 17, 2017
31ce2c1
deps: add postmortem metadata for V8 TurboFan
targos Sep 6, 2017
dc1996d
src: fix SmartOS compilation
targos Jun 27, 2017
fca7e49
test: adjust windows failed alloc test to V8 6.2
bzoz Sep 13, 2017
2ac7b43
dgram: support for setting socket buffer size
DamienOReilly Jun 11, 2017
35a526c
http2,async-wrap: introduce AliasedBuffer class
Aug 25, 2017
0dad97c
doc: fix "added in" for Buffer.allocUnsafeSlow()
tuananh Sep 11, 2017
1aca135
http2: add tests for push stream error handling
apapirovski Sep 13, 2017
dcc41fd
src: remove unused perf_hooks uv handles
jasnell Sep 12, 2017
ad3d899
http2: improve http2 coverage
jasnell Sep 5, 2017
9d9552f
http2: custom promisify for http2.connect
jasnell Sep 1, 2017
bf1ca8f
meta: improve contributors guide
jasnell Aug 31, 2017
f68ab39
doc: add missing heading for error
maclover7 Sep 11, 2017
468110b
tls: deprecate parseCertString & move to internal
XadillaX Sep 8, 2017
eb4940e
string_decoder: Migrate to use internal/errors
starkwang Aug 8, 2017
99a7799
doc: add missing doc for readable._destroy
targos Sep 10, 2017
5ee2d3e
test: fix sequential/test-async-wrap-getasyncid
addaleax Sep 9, 2017
a172b7c
test: refactor test-debug-prompt
Trott Sep 2, 2017
6cfd773
test: remove invalid test
Trott Sep 10, 2017
ff16337
doc: use consistent terminology in process doc
Trott Sep 10, 2017
d38e643
test: remove obsolete debugger tests
Trott Sep 1, 2017
b8d532c
test: allow adding known-globals through ENV
refack Sep 4, 2017
ed2f347
child_process: set shell to false in fork()
Sep 9, 2017
688765a
test: add test for fork() + shell
cjihrig Sep 12, 2017
cba206f
docs: update 8.5.0 changelog
MylesBorins Sep 13, 2017
4ae0afb
dgram: added setMulticastInterface()
lostnet Jul 23, 2016
290315a
src: refactor `#include` handling
addaleax Aug 8, 2017
be6d807
src: make in_makecallback() getter const
addaleax Aug 8, 2017
64616bb
src: refactor async callback handling
addaleax Aug 8, 2017
a564c1e
src: remove virtually unused ExecScope
addaleax Aug 8, 2017
8c8c90b
n-api: use AsyncResource for Work tracking
addaleax Aug 8, 2017
bdaa2cb
test: add regression test for 5691
addaleax Sep 5, 2017
2509c34
src: move DomainEnter,DomainExit to node.cc
addaleax Sep 6, 2017
1a0727d
n-api: change async resource name to napi_value
jasongin Sep 9, 2017
7456db9
test: convert buffer benchmark to runBenchmark
maclover7 Sep 11, 2017
d195a06
src: fix typo in probe description
evanlucas Sep 13, 2017
92e5f5c
n-api: refactor napi_addon_register_func
boingoing Aug 30, 2017
0c258bd
n-api: Context for custom async operations
jasongin Aug 26, 2017
973c12f
n-api: napi_is_construct_call->napi_get_new_target
Aug 8, 2017
e9358af
errors: remove duplicate error definition
maclover7 Sep 10, 2017
082c434
test: move test-benchmark-buffer to sequential
Trott Sep 12, 2017
6bfc439
benchmark: improve and add more inspect benchmarks
BridgeAR Aug 17, 2017
01652cc
util: add fast internal array join method
BridgeAR Aug 22, 2017
f9ad23d
util: refactor inspect for performance and more
BridgeAR Aug 17, 2017
bac313b
util: fix out of bounds indices in util.inspect
BridgeAR Sep 9, 2017
e3f4305
test: improve util inspect tests
BridgeAR Sep 9, 2017
8f52ccc
test: fix actual and expected order
BridgeAR Sep 13, 2017
d8a0364
async_hooks,doc: some async_hooks improvements
jasnell Aug 30, 2017
8bd1668
doc: fix emitKeypressEvents stream type
Oblosys Sep 13, 2017
a591610
doc: fix wrong history entry in deepStrictEqual
hisener Sep 13, 2017
dce72c2
module: check file url passed to top-level import
guybedford Sep 13, 2017
01a1812
deps: cherry-pick b6158eb6befae from V8 upstream
addaleax Sep 8, 2017
8403d6b
deps: cherry-pick 9b21865822243 from V8 upstream
addaleax Sep 8, 2017
dcb24e3
src: keep track of env properly in node_perf.cc
addaleax Sep 4, 2017
2e8217c
assert: improve AssertionError in case of "Errors"
BridgeAR Aug 25, 2017
0fc402b
module: coverity fixes for ESM C++
bmeck Sep 8, 2017
11f46a2
benchmark: enable assert benchmark with short len
Trott Sep 3, 2017
3c4c0db
benchmark: provide default methods for assert
Trott Sep 3, 2017
a901849
test: add test-benchmark-assert
Trott Sep 3, 2017
ca2c73c
doc: fix http.ClientRequest method descriptions
antoine-amara Sep 1, 2017
b0d3bec
assert: use Same-value equality in deepStrictEqual
BridgeAR Aug 19, 2017
631c59b
test: don't skip when common.mustCall() is pending
cjihrig Sep 14, 2017
75f7b2f
doc: do not begin yaml value with backtick
maclover7 Sep 17, 2017
9b996f0
deps: patch V8 to 6.1.534.38
MylesBorins Sep 15, 2017
bd85752
src: use InstantiateModule instead of deprecated
danbev Sep 15, 2017
a4e923f
http2: fix subsequent end calls to not throw
apapirovski Sep 14, 2017
de51717
test: fix flaky test-http2-session-timeout
apapirovski Sep 16, 2017
8fa5fcc
http2: emit close event if request aborted
apapirovski Sep 14, 2017
c75f87c
crypto: refactor the crypto module
jasnell Sep 6, 2017
1976654
n-api: add optional string length parameters
Sep 11, 2017
2b7b9f2
doc: prevent displaying empty version picker
Sep 14, 2017
e86952d
build: add support for link-module to vcbuild
bzoz Sep 14, 2017
1ebde6e
doc: make mkdtemp example work on Windows
bzoz Sep 14, 2017
049a8d7
doc: fix entryTypes type and missing link
manidlou Sep 14, 2017
4dc920a
src: remove outdated todo from node_crypto.cc
barnski Aug 30, 2017
8a968e4
benchmark: use smaller n value in some http tests
psmarshall Jun 30, 2017
6f34076
test: do not write fixture in test-require-symlink
Trott Aug 27, 2017
faaefa8
util: improve format performance
BridgeAR Sep 15, 2017
a8c0a43
n-api: remove n-api module loading flag
Aug 18, 2017
db2e093
assert: handle enumerable symbol keys
BridgeAR Aug 19, 2017
4bcb1c3
lib: refactor console startup
BridgeAR Aug 29, 2017
b1c8f15
util: use constructor name
BridgeAR Aug 17, 2017
e167ab7
benchmark: var to const
BridgeAR Sep 14, 2017
3c65a83
timers: clarify lib/timer.js comment
danbev Jan 26, 2017
873e5bd
crypto: support multiple ECDH curves and auto
rogaps Sep 4, 2017
2f5bef4
http2: expand list of known headers
apapirovski Sep 19, 2017
8589c70
http: revert #14024 writable is never set to false
mcollina Sep 14, 2017
15879ad
test: expand http2 frameError test case
apapirovski Sep 15, 2017
784cdad
doc: add documentation for the 'timeout' event
lpinca Sep 17, 2017
3070d53
doc: fix new nits in links
vsemozhetbyt Sep 17, 2017
c5eb5bf
build: enable runtime linking
jBarz Sep 8, 2017
10be20a
http: set socket timeout when socket connects
lpinca Oct 2, 2016
750c080
test: backward compatible api for tty
gergelyke Sep 7, 2017
602fd36
domain: remove `.dispose()`
addaleax Sep 14, 2017
87bddda
src: fix compiler warning in udp_wrap.cc
danbev Sep 14, 2017
11f7dcf
timers: do not expose .unref()._handle._list
Fishrock123 Sep 6, 2016
6c38ab3
http2: improved coverage of Http2Stream destroy
ssbrewster Sep 18, 2017
75606c4
inspector: break in eval script
Aug 1, 2017
ee46c73
doc: fix external links with 404 status
vsemozhetbyt Sep 18, 2017
6763fb2
doc: fix some links in http2.md
vsemozhetbyt Sep 20, 2017
01c680b
test: refactor test for readability
refack May 12, 2017
f27b5e4
src: prepare platform for upstream V8 changes
addaleax Sep 15, 2017
4c19cef
crypto: better crypto error messages
gla5001 Aug 10, 2017
84ea2b8
Review updates and tests
gla5001 Aug 11, 2017
a83fc25
Additional review updates.
gla5001 Aug 15, 2017
efa8086
Review update. Use the overload of Set()
gla5001 Aug 15, 2017
c035ba0
Nit fix: changing local variable to snake_case
gla5001 Aug 16, 2017
720a5af
PR review updates
gla5001 Sep 21, 2017
49f4f31
Merging
gla5001 Sep 21, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,10 @@ intercepted.
A generic JavaScript `Error` object that does not denote any specific
circumstance of why the error occurred. `Error` objects capture a "stack trace"
detailing the point in the code at which the `Error` was instantiated, and may
provide a text description of the error. For crypto only, `Error` objects will
include the OpenSSL error stack, if it's available when the error is thrown, in
a separate property.
provide a text description of the error.

For crypto only, `Error` objects will include the OpenSSL error stack in a
separate property if it is available when the error is thrown.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you name the property here?


All errors generated by Node.js, including all System and JavaScript errors,
will either be instances of, or inherit from, the `Error` class.
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ namespace node {
V(onstreamclose_string, "onstreamclose") \
V(ontrailers_string, "ontrailers") \
V(onwrite_string, "onwrite") \
V(openssl_error_stack, "openSSLErrorStack") \
V(output_string, "output") \
V(order_string, "order") \
V(owner_string, "owner") \
Expand Down
15 changes: 6 additions & 9 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -262,20 +262,16 @@ void ThrowCryptoError(Environment* env,
ERR_error_string_n(err, errmsg, sizeof(errmsg));
message = String::NewFromUtf8(env->isolate(), errmsg,
v8::NewStringType::kNormal)
.ToLocalChecked();
.ToLocalChecked();
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added additional space to these since its a continuation of line

message = String::NewFromUtf8(env->isolate(), default_message,
v8::NewStringType::kNormal)
.ToLocalChecked();
.ToLocalChecked();
}

Local<Value> exception_v = Exception::Error(message);
CHECK(!exception_v.IsEmpty());
// Add the openSSLErrorStack property to the exception object.
Local<Object> exception = exception_v.As<Object>();
Local<String> key = String::NewFromUtf8(env->isolate(), "openSSLErrorStack",
v8::NewStringType::kInternalized)
.ToLocalChecked();
Local<Array> errorStack = Array::New(env->isolate());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style issue: use snake_case for locals, not camelCase.


ERR_STATE *es = ERR_get_state();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to allocate memory, where does that get freed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that the openssl lib would take care of this, but maybe my assumption is incorrect?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure myself. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking is that we should probably just let the openssl lib free the memory. ERR_get_state will return a pointer to the err state struct that was allocated by openssl. If we try to free that memory here, wont that cause potential issues down the line if other functions need to access it/try to read the reference it has to it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't (shouldn't) free the memory, openssl takes care of it internally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: star leans left.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup. will fix

Expand All @@ -289,17 +285,18 @@ void ThrowCryptoError(Environment* env,
ERR_error_string_n(err_buf, tmpStr, sizeof(tmpStr));
errorStack->Set(i, String::NewFromUtf8(env->isolate(), tmpStr,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you always use the overload of Set() that takes a context argument/returns a Maybe? The other one is deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And its ok that I get a compiler warning warning: ignoring return value of function declared with 'warn_unused_result' attribute?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’d call .FromJust()on the result, just like .ToLocalChecked() for MaybeLocals :)

v8::NewStringType::kNormal)
.ToLocalChecked());
.ToLocalChecked());
}
es->top -= 1;
}

// Adding the new property that will look like the following:
// Add the openSSLErrorStack property to the exception object.
// The new property will look like the following:
// openSSLErrorStack: [
// 'error:0906700D:PEM routines:PEM_ASN1_read_bio:ASN1 lib',
// 'error:0D07803A:asn1 encoding routines:ASN1_ITEM_EX_D2I:nested asn1 error'
// ]
exception->Set(env->context(), key, errorStack).FromJust();
exception->Set(env->openssl_error_stack(), errorStack);
env->isolate()->ThrowException(exception);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new property looks like this

openSSLErrorStack:
[ 'error:0906700D:PEM routines:PEM_ASN1_read_bio:ASN1 lib',
  'error:0D07803A:asn1 encoding routines:ASN1_ITEM_EX_D2I:nested asn1 error' ]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useful comment, please put it in the source code


Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ assert.throws(function() {
if ((err instanceof Error) &&
/digest too big for rsa key$/.test(err) &&
err.openSSLErrorStack !== undefined &&
Array.isArray(err.openSSLErrorStack) &&
err.openSSLErrorStack.length === 0) {
return true;
}
Expand All @@ -269,6 +270,7 @@ assert.throws(function() {
if ((err instanceof Error) &&
/asn1 encoding routines:ASN1_CHECK_TLEN:wrong tag/.test(err) &&
err.openSSLErrorStack !== undefined &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe be more specific here and check Array.isArray(err.openSSLErrorStack)?

Array.isArray(err.openSSLErrorStack) &&
err.openSSLErrorStack.length > 0) {
return true;
}
Expand Down