Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 88a4d04

Browse filesBrowse files
davidbengibfahn
authored andcommitted
crypto: do not reach into OpenSSL internals for ThrowCryptoError
There is a perfectly serviceable ERR_get_error function which avoids having to sniff through the OpenSSL ring buffer like that. It does return the errors in the opposite order, but that's easily fixed with std::reverse. Note this behavior is slightly different in that an ERR_get_error loop will ultimately clear the error queue, but this is desirable. Leaving the error queue uncleared means errors in subsequent operations may get mixed up and cause issues. PR-URL: #16701 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent d8eea81 commit 88a4d04
Copy full SHA for 88a4d04

File tree

Expand file treeCollapse file tree

1 file changed

+27
-36
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

1 file changed

+27
-36
lines changed
Open diff view settings
Collapse file

‎src/node_crypto.cc‎

Copy file name to clipboardExpand all lines: src/node_crypto.cc
+27-36Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,13 @@
4343
// StartComAndWoSignData.inc
4444
#include "StartComAndWoSignData.inc"
4545

46+
#include <algorithm>
4647
#include <errno.h>
4748
#include <limits.h> // INT_MAX
4849
#include <math.h>
4950
#include <stdlib.h>
5051
#include <string.h>
52+
#include <vector>
5153

5254
#define THROW_AND_RETURN_IF_NOT_STRING_OR_BUFFER(val, prefix) \
5355
do { \
@@ -270,44 +272,33 @@ void ThrowCryptoError(Environment* env,
270272
Local<Value> exception_v = Exception::Error(message);
271273
CHECK(!exception_v.IsEmpty());
272274
Local<Object> exception = exception_v.As<Object>();
273-
ERR_STATE* es = ERR_get_state();
274-
275-
if (es->bottom != es->top) {
276-
Local<Array> error_stack = Array::New(env->isolate());
277-
int top = es->top;
278-
279-
// Build the error_stack array to be added to opensslErrorStack property.
280-
for (unsigned int i = 0; es->bottom != es->top;) {
281-
unsigned long err_buf = es->err_buffer[es->top]; // NOLINT(runtime/int)
282-
// Only add error string if there is valid err_buffer.
283-
if (err_buf) {
284-
char tmp_str[256];
285-
ERR_error_string_n(err_buf, tmp_str, sizeof(tmp_str));
286-
error_stack->Set(env->context(), i,
287-
String::NewFromUtf8(env->isolate(), tmp_str,
288-
v8::NewStringType::kNormal)
289-
.ToLocalChecked()).FromJust();
290-
// Only increment if we added to error_stack.
291-
i++;
292-
}
293275

294-
// Since the ERR_STATE is a ring buffer, we need to use modular
295-
// arithmetic to loop back around in the case where bottom is after top.
296-
// Using ERR_NUM_ERRORS macro defined in openssl.
297-
es->top = (((es->top - 1) % ERR_NUM_ERRORS) + ERR_NUM_ERRORS) %
298-
ERR_NUM_ERRORS;
276+
std::vector<Local<String>> errors;
277+
for (;;) {
278+
unsigned long err = ERR_get_error(); // NOLINT(runtime/int)
279+
if (err == 0) {
280+
break;
299281
}
300-
301-
// Restore top.
302-
es->top = top;
303-
304-
// Add the opensslErrorStack property to the exception object.
305-
// The new property will look like the following:
306-
// opensslErrorStack: [
307-
// 'error:0906700D:PEM routines:PEM_ASN1_read_bio:ASN1 lib',
308-
// 'error:0D07803A:asn1 encoding routines:ASN1_ITEM_EX_D2I:nested asn1 err'
309-
// ]
310-
exception->Set(env->context(), env->openssl_error_stack(), error_stack)
282+
char tmp_str[256];
283+
ERR_error_string_n(err, tmp_str, sizeof(tmp_str));
284+
errors.push_back(String::NewFromUtf8(env->isolate(), tmp_str,
285+
v8::NewStringType::kNormal)
286+
.ToLocalChecked());
287+
}
288+
289+
// ERR_get_error returns errors in order of most specific to least
290+
// specific. We wish to have the reverse ordering:
291+
// opensslErrorStack: [
292+
// 'error:0906700D:PEM routines:PEM_ASN1_read_bio:ASN1 lib',
293+
// 'error:0D07803A:asn1 encoding routines:ASN1_ITEM_EX_D2I:nested asn1 err'
294+
// ]
295+
if (!errors.empty()) {
296+
std::reverse(errors.begin(), errors.end());
297+
Local<Array> errors_array = Array::New(env->isolate(), errors.size());
298+
for (size_t i = 0; i < errors.size(); i++) {
299+
errors_array->Set(env->context(), i, errors[i]).FromJust();
300+
}
301+
exception->Set(env->context(), env->openssl_error_stack(), errors_array)
311302
.FromJust();
312303
}
313304

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.