tls: show human-readable error messages #6897
Conversation
Before this commit, verification exceptions had err.message set to the
OpenSSL error code (e.g. 'UNABLE_TO_VERIFY_LEAF_SIGNATURE').
This commit moves the error code to err.code and replaces err.message
with a human-readable error. Example:
// before
{
message: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE'
}
// after
{
code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE',
message: 'unable to verify the first certificate'
}
UNABLE_TO_VERIFY_LEAF_SIGNATURE is a good example of why you want this:
the error code suggests that it's the last certificate that fails to
validate while it's actually the first certificate in the chain.
Going by the number of mailing list posts and StackOverflow questions,
it's a source of confusion to many people.
| template <class Base> | ||
| void SSLWrap<Base>::VerifyError(const FunctionCallbackInfo<Value>& args) { | ||
| HandleScope scope(node_isolate); | ||
| Isolate* isolate = args.GetIsolate(); | ||
| HandleScope scope(isolate); |
There was a problem hiding this comment.
Looks a bit unrelated, right? Mind opening issue for changing all HandleScopes in this file?
There was a problem hiding this comment.
Well, it's related in the sense that we need the isolate further down below. I thought I'd fix up the HandleScope while I'm here, saves work if/when node grows multi-isolate support. I can back it out if you want.
I'm not going to fix up all the HandleScopes in the file however, I'm strictly working on a scratch-my-own-itch basis. (Or scratch-a-user-itch as is the case here.)
There was a problem hiding this comment.
I think you got me wrong, I'm going to fix it myself. Just would be cool if this patch won't contain partial fix.
There was a problem hiding this comment.
Oh sorry, 'opening issue' - yes, I misread that. :-) Will do and I'll back out this change.
|
Few very minor nits, otherwise LGTM! Thank you! |
|
Thank you very much! Landed in 262a752 |
Before this commit, verification exceptions had err.message set to the
OpenSSL error code (e.g. 'UNABLE_TO_VERIFY_LEAF_SIGNATURE').
This commit moves the error code to err.code and replaces err.message
with a human-readable error. Example:
UNABLE_TO_VERIFY_LEAF_SIGNATURE is a good example of why you want this:
the error code suggests that it's the last certificate that fails to
validate while it's actually the first certificate in the chain.
Going by the number of mailing list posts and StackOverflow questions,
it's a source of confusion to many people.
Suggested reviewer: @indutny