crypto: better crypto error messages #14725

gla5001 · 2017-08-10T03:36:06Z

Feature request to add openSSL error stack to the exception object
thrown from crypto. New exception property only added to object
if the error stack has not cleared out prior to calling
ThrowCryptoError.

Refs: #5444

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

Feature request to add openSSL error stack to the exception object thrown from crypto. New exception property only added to object if the error stack has not cleared out prior to calling ThrowCryptoError. Refs: nodejs#5444

gla5001 · 2017-08-10T03:39:10Z

Decided to open up this PR with my initial commit to get some general feedback to see if this is what was expected and if i'm on the right track. I didn't include any tests at this time since im just making sure what I have right now is correct. Once I know if this is correct or not, i'll add some tests.

gla5001 · 2017-08-10T03:40:54Z

src/node_crypto.cc

 void ThrowCryptoError(Environment* env,
                      unsigned long err,  // NOLINT(runtime/int)
                      const char* default_message = nullptr) {
  HandleScope scope(env->isolate());


Below is the bulk of the change. Updated ThrowCryptoError to get the current error state and add the stack to a new property on the exception object. I figured it would be a good idea to include the errorStack regardless if the error passed in is a default_message or openSSL error.

Also. Do i need to change any documentation on this? if so, where is it located?

docs would be somewhere in here: https://github.com/nodejs/node/blob/master/doc/api/errors.md#class-error

There is no specific class for crypto errors, so docs will just have to go with generic Error but described as "only for crypto"

gla5001 · 2017-08-10T03:48:55Z

src/node_crypto.cc


-  ClearErrorOnReturn clear_error_on_return;
+  // ClearErrorOnReturn clear_error_on_return;



so i comment out this line for now. Maybe i'm wrong, but it feels like this was in the wrong spot. This is not the binding function and I thought the openssl's error stack needs to be balanced upon entry and exit of the VerifyFinal bound function. So i moved it there. Please let me know if im wrong and I'll uncomment this and remove the line from the bound function, but I might need a little guidance on what I need to move around to not clear out the error stack when exiting this function. Maybe we just throw the error from this function instead of the bound function?

You make a compelling case, but I would have to look more closely and don't have time right now. I think there was a bug recently where sign failure was causing subsequent operations to fail because stack wasn't cleared, but that just means its important to do, not that its important to do here, so this needs careful thought.

gla5001 · 2017-08-10T03:50:54Z

src/node_crypto.cc


+  ERR_set_mark();
+
  bp = BIO_new_mem_buf(const_cast<char*>(key_pem), key_pem_len);


setting the err mark. #5444 (comment). Im not sure where else we might need to do this in this file.

I admit I don't understand why that is necessary when you have a ClearErrorOnReturn in the binding function.

gla5001 · 2017-08-10T03:51:39Z

src/node_crypto.cc


+  ClearErrorOnReturn clear_error_on_return;
+
  Verify* verify;


here is adding the clear_error_on_return in the function thats bound

gla5001 · 2017-08-10T03:52:19Z

src/node_crypto.cc

+
  Verify* verify;
  ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());



Will add tests once I know if this is correct route to handle this or what I need to change

gla5001 · 2017-08-10T04:01:32Z

src/node_crypto.cc

+
+  exception->Set(env->context(), key, errorStack).FromJust();
+  env->isolate()->ThrowException(exception);
 }


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' ]

Useful comment, please put it in the source code

Trott · 2017-08-10T04:31:17Z

@nodejs/crypto

sam-github · 2017-08-10T18:14:33Z

src/node_crypto.cc

    ERR_error_string_n(err, errmsg, sizeof(errmsg));
-    env->ThrowError(errmsg);
+    message = String::NewFromUtf8(env->isolate(), errmsg,
+                                  v8::NewStringType::kInternalized)


maybe the performance diff isn't observable, but I wonder if errors are thrown enough for it to be worth internalizing the string

I'll change this to normal and the one below in the else

sam-github · 2017-08-10T18:15:29Z

src/node_crypto.cc

+
+  Local<Value> exception_v = Exception::Error(message);
+  CHECK(!exception_v.IsEmpty());
+  // add the openSSLErrorStack property


node comment style is to have capitalized and punctuated sentences, pls take a look at all the comments

sam-github · 2017-08-10T18:16:35Z

src/node_crypto.cc

+
+  // get the error state
+  // will only add to errorStack if state has not been cleared out
+  ERR_STATE *es;


merge this line with next, so declaration and initialization is done in one line

sam-github · 2017-08-10T18:18:08Z

src/node_crypto.cc

+
+  // add content to the openssl error stack
+  unsigned int i = 0;
+  while (es->bottom != es->top


given that this while loop has an initialization before it, and increments its state at the end, should it be a for loop?

Yup. Will do

sam-github · 2017-08-10T18:21:57Z

src/node_crypto.cc

+    unsigned long err_buf = es->err_buffer[es->top];  // NOLINT(runtime/int)
+    // only add if there is valid err_buffer
+    if (err_buf) {
+      char tmpStr[128] = { 0 };


I'd choose 256, it is what ERR_error_string() uses. I think it doesn't need zero initialization, ERR_error_string_n() terminates.

sam-github · 2017-08-10T18:22:59Z

src/node_crypto.cc

+
+  exception->Set(env->context(), key, errorStack).FromJust();
+  env->isolate()->ThrowException(exception);
 }


Useful comment, please put it in the source code

sam-github · 2017-08-10T18:25:38Z

src/node_crypto.cc


-  ClearErrorOnReturn clear_error_on_return;
+  // ClearErrorOnReturn clear_error_on_return;



You make a compelling case, but I would have to look more closely and don't have time right now. I think there was a bug recently where sign failure was causing subsequent operations to fail because stack wasn't cleared, but that just means its important to do, not that its important to do here, so this needs careful thought.

sam-github · 2017-08-10T18:36:34Z

src/node_crypto.cc

 void ThrowCryptoError(Environment* env,
                      unsigned long err,  // NOLINT(runtime/int)
                      const char* default_message = nullptr) {
  HandleScope scope(env->isolate());


docs would be somewhere in here: https://github.com/nodejs/node/blob/master/doc/api/errors.md#class-error

There is no specific class for crypto errors, so docs will just have to go with generic Error but described as "only for crypto"

gla5001 · 2017-08-11T21:22:52Z

Added some updates from previous review and tests

gla5001 · 2017-08-11T21:24:29Z

test/parallel/test-crypto.js

  const credentials = tls.createSecureContext(options);
  const context = credentials.context;
  const notcontext = { setOptions: context.setOptions, setKey: context.setKey };
  tls.createSecureContext({ secureOptions: 1 }, notcontext);


Updated tests below to check for existence of new openSSLErrorStack property for when it should and should not be there

outdated

addaleax · 2017-08-14T17:48:49Z

ping @sam-github, most of your comments have been addressed

addaleax · 2017-08-14T17:49:53Z

doc/api/errors.md

-provide a text description of the error.
+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.


tiny nits: use long forms (it's → it is) and I’d put the sentence in a new paragraph.

addaleax · 2017-08-14T17:50:18Z

src/node_crypto.cc

-    env->ThrowError(errmsg);
+    message = String::NewFromUtf8(env->isolate(), errmsg,
+                                  v8::NewStringType::kNormal)
+                                    .ToLocalChecked();


tiny style nit: 4 spaces indentation for statement continuations (applies elsewhere too)

addaleax · 2017-08-14T17:51:32Z

src/node_crypto.cc

+  Local<Object> exception = exception_v.As<Object>();
+  Local<String> key = String::NewFromUtf8(env->isolate(), "openSSLErrorStack",
+                                          v8::NewStringType::kInternalized)
+                                            .ToLocalChecked();


I think you can feel free to add this to the list in env.h. (Also, using FIXED_ONE_BYTE_STRING should be okay here too.)

@addaleax thanks for reviewing this. Just to clarify, you're saying that I could add the property to list in env.h or use FIXED_ONE_BYTE_STRING, correct?

Yes – either one is fine, and I’d have a slight preference for the former :)

addaleax · 2017-08-14T17:53:18Z

test/parallel/test-crypto.js

+  // The openSSL stack should have content.
+  if ((err instanceof Error) &&
+      /asn1 encoding routines:ASN1_CHECK_TLEN:wrong tag/.test(err) &&
+      err.openSSLErrorStack !== undefined &&


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

gla5001 · 2017-08-15T01:53:03Z

src/node_crypto.cc

                                  v8::NewStringType::kNormal)
-                                    .ToLocalChecked();
+                                      .ToLocalChecked();
  } else {


added additional space to these since its a continuation of line

addaleax · 2017-08-15T14:32:53Z

src/node_crypto.cc

+  Local<Object> exception = exception_v.As<Object>();
+  Local<Array> errorStack = Array::New(env->isolate());
+
+  ERR_STATE *es = ERR_get_state();


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

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

I’m not sure myself. 😄

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?

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

addaleax · 2017-08-15T14:33:33Z

src/node_crypto.cc

+    if (err_buf) {
+      char tmpStr[256] = { };
+      ERR_error_string_n(err_buf, tmpStr, sizeof(tmpStr));
+      errorStack->Set(i, String::NewFromUtf8(env->isolate(), tmpStr,


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

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

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

bnoordhuis · 2017-08-16T10:17:44Z

src/node_crypto.cc

+  Local<Object> exception = exception_v.As<Object>();
+  Local<Array> errorStack = Array::New(env->isolate());
+
+  ERR_STATE *es = ERR_get_state();


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

bnoordhuis · 2017-08-16T10:18:06Z

src/node_crypto.cc

+  Local<Value> exception_v = Exception::Error(message);
+  CHECK(!exception_v.IsEmpty());
+  Local<Object> exception = exception_v.As<Object>();
+  Local<Array> errorStack = Array::New(env->isolate());


Style issue: use snake_case for locals, not camelCase.

gla5001 · 2017-08-18T16:00:38Z

So i believe i have addressed everyones review comments. The only thing left outstanding is the comment about moving ClearErrorOnReturn clear_error_on_return; in the first commit. Any additional inout would be great.

gla5001 · 2017-08-24T17:20:30Z

There has not been any activity on this for several days now. @addaleax @bnoordhuis is there anything else that should be done on this?

addaleax · 2017-08-24T17:27:30Z

@gla5001 Thanks for the ping – as far as I am concerned, the code looks good, but I’m not familiar enough with OpenSSL’s APIs to give this an approval.

ping @nodejs/crypto

When ESM support was added it created a regression in the test runner that broke the ability to run individual tests. This commit re-introduces the use of `NormalizePath` which fixes the regression in the test runner Refs: nodejs#15300 PR-URL: nodejs#15329 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Update AUTHORS list using tools/update-authors.sh. Update .mailmap to handle duplicates. PR-URL: nodejs#15181 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: nodejs#15002 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: nodejs#15235 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

`domain.dispose()` is generally considered an anti-pattern, has been runtime-deprecated for over 4 years, and is a part of the `domain` module that can not be emulated by `async_hooks`; so remove it. Ref: https://nodejs.org/en/docs/guides/domain-postmortem/ Ref: 4a74fc9 PR-URL: nodejs#15412 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>

Currently the following compiler warning is generated: 1 warning generated. ../src/udp_wrap.cc:238:12: warning: comparison of integers of different signs: 'int' and 'uint32_t' (aka 'unsigned int') [-Wsign-compare] if (size != args[0].As<Uint32>()->Value()) { ~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1 warning generated. This commit changes the check to see that the Uint32 value does not exceed the max int size instead of first casting and then comparing. PR-URL: nodejs#15402 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>

The list is defunct at this point. I believe this to be in effect a minor defect from 3eecdf9 PR-URL: nodejs#8422 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Julien Gilli <jgilli@nodejs.org> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Refs: nodejs#14985 PR-URL: nodejs#15461 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Fixes: nodejs#14577 PR-URL: nodejs#14581 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

sam-github · 2017-09-20T18:16:03Z

All my comments addressed, so LGTM

I haven't had time to review for #14725 (comment), though, and still don't, so someone else will have to sign off on that.

gla5001 · 2017-09-20T18:21:47Z

thanks @sam-github

sam-github · 2017-09-20T18:26:26Z

btw, looks like it needs rebasing.

gla5001 · 2017-09-20T18:32:23Z

yeah, i'll take care of that once I get a few more approvals.

sam-github · 2017-09-20T18:44:07Z

ping @nodejs/crypto

bnoordhuis

Bikeshed: anyone have opinions on the .openSSLErrorStack property name?

Performance: creating an array and setting a property for every exception might not be cheap.

bnoordhuis · 2017-09-20T18:34:20Z

doc/api/errors.md

 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.


Can you name the property here?

bnoordhuis · 2017-09-20T18:35:13Z

src/node_crypto.cc

+  Local<Object> exception = exception_v.As<Object>();
+  Local<Array> error_stack = Array::New(env->isolate());
+
+  ERR_STATE *es = ERR_get_state();


Style: star leans left.

yup. will fix

bnoordhuis · 2017-09-20T18:50:45Z

src/node_crypto.cc

+  ERR_STATE *es = ERR_get_state();
+  // Build the error_stack array to be added to openSSLErrorStack property.
+  for (unsigned int i = 0; es->bottom != es->top
+        && (es->err_flags[es->top] & ERR_FLAG_MARK) == 0; i++) {


This doesn't look correct to me. For starters, es->top and es->bottom should be restored after you're done. Second, this assumes es->bottom <= es->top but that isn't necessarily true because it's a ring buffer.

ok. i think i understand. I'll fix this

bnoordhuis · 2017-09-20T18:51:09Z

src/node_crypto.cc

+    unsigned long err_buf = es->err_buffer[es->top];  // NOLINT(runtime/int)
+    // Only add error string if there is valid err_buffer.
+    if (err_buf) {
+      char tmpStr[256] = { };


Style: snake_case and no init necessary.

bnoordhuis · 2017-09-20T18:51:56Z

src/node_crypto.cc

+  // '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->openssl_error_stack(), error_stack);


Can you use the Set() that takes a Local<Context>?

bnoordhuis · 2017-09-20T18:53:43Z

src/node_crypto.cc


+  ERR_set_mark();
+
  bp = BIO_new_mem_buf(const_cast<char*>(key_pem), key_pem_len);


I admit I don't understand why that is necessary when you have a ClearErrorOnReturn in the binding function.

PR-URL: nodejs#15463 Fixes: nodejs#15462 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: nodejs#15481 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#13003 Fixes: nodejs#12951 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

V8 platform tasks may schedule other tasks (both background and foreground), and may perform asynchronous operations like resolving Promises. To address that: - Run the task queue drain call inside a callback scope. This makes sure asynchronous operations inside it, like resolving promises, lead to the microtask queue and any subsequent operations not being silently forgotten. - Move the task queue drain call before `EmitBeforeExit()` and only run `EmitBeforeExit()` if there is no new event loop work. - Account for possible new foreground tasks scheduled by background tasks in `DrainBackgroundTasks()`. PR-URL: nodejs#15428 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Matthew Loring <mattloring@google.com>

Feature request to add openSSL error stack to the exception object thrown from crypto. New exception property only added to object if the error stack has not cleared out prior to calling ThrowCryptoError. Refs: nodejs#5444

gla5001 · 2017-09-21T02:28:26Z

having some git issues. will resolve them

gla5001 · 2017-09-21T02:54:16Z

I did something terribly wrong git wise here. i opened a new PR #15518 with all the updated changes. Please review that. I am closing this PR.

sam-github · 2017-09-21T18:13:09Z

FYI, for next time, you can force push a branch to replace it's contents without closing a PR.

gla5001 · 2017-09-21T18:18:25Z

cool. thanks for the tip.

crypto: better crypto error messages

57b5b47

Feature request to add openSSL error stack to the exception object thrown from crypto. New exception property only added to object if the error stack has not cleared out prior to calling ThrowCryptoError. Refs: nodejs#5444

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. labels Aug 10, 2017

gla5001 commented Aug 10, 2017

View reviewed changes

sam-github previously requested changes Aug 10, 2017

View reviewed changes

Review updates and tests

1c695af

gla5001 commented Aug 11, 2017

View reviewed changes

addaleax reviewed Aug 14, 2017

View reviewed changes

Additional review updates.

ed9f9b5

gla5001 commented Aug 15, 2017

View reviewed changes

addaleax reviewed Aug 15, 2017

View reviewed changes

Review update. Use the overload of Set()

a72d7e1

bnoordhuis reviewed Aug 16, 2017

View reviewed changes

Nit fix: changing local variable to snake_case

0538b25

TimothyGu and others added 3 commits September 12, 2017 08:25

errors: eliminate circular dependency on assert

6ff521b

PR-URL: nodejs#15002 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

gergelyke and others added 6 commits September 20, 2017 10:42

test: backward compatible api for tty

750c080

PR-URL: nodejs#15235 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>

http2: improved coverage of Http2Stream destroy

6c38ab3

Refs: nodejs#14985 PR-URL: nodejs#15461 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

inspector: break in eval script

75606c4

Fixes: nodejs#14577 PR-URL: nodejs#14581 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>

sam-github approved these changes Sep 20, 2017

View reviewed changes

bnoordhuis requested changes Sep 20, 2017

View reviewed changes

vsemozhetbyt and others added 11 commits September 20, 2017 22:39

doc: fix external links with 404 status

ee46c73

PR-URL: nodejs#15463 Fixes: nodejs#15462 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

doc: fix some links in http2.md

6763fb2

PR-URL: nodejs#15481 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: James M Snell <jasnell@gmail.com>

test: refactor test for readability

01c680b

PR-URL: nodejs#13003 Fixes: nodejs#12951 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

crypto: better crypto error messages

4c19cef

Feature request to add openSSL error stack to the exception object thrown from crypto. New exception property only added to object if the error stack has not cleared out prior to calling ThrowCryptoError. Refs: nodejs#5444

Review updates and tests

84ea2b8

Additional review updates.

a83fc25

Review update. Use the overload of Set()

efa8086

Nit fix: changing local variable to snake_case

c035ba0

PR review updates

720a5af

Merging

49f4f31

gla5001 mentioned this pull request Sep 21, 2017

Crypto add opensslerror stack #15518

Closed

4 tasks

gla5001 closed this Sep 21, 2017


		ClearErrorOnReturn clear_error_on_return;
		// ClearErrorOnReturn clear_error_on_return;


		ERR_set_mark();

		bp = BIO_new_mem_buf(const_cast<char*>(key_pem), key_pem_len);


		Verify* verify;
		ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder());

Uh oh!

crypto: better crypto error messages #14725

crypto: better crypto error messages #14725

Uh oh!

Conversation

gla5001 commented Aug 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Uh oh!

gla5001 commented Aug 10, 2017

Uh oh!

gla5001 Aug 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trott commented Aug 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gla5001 commented Aug 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

addaleax commented Aug 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gla5001 Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

gla5001 commented Aug 10, 2017 •

edited

Loading

gla5001 Aug 10, 2017 •

edited

Loading

gla5001 commented Aug 11, 2017 •

edited

Loading

gla5001 Aug 14, 2017 •

edited

Loading

gla5001 commented Aug 24, 2017 •

edited

Loading