Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PEM parsing: check last error instead of first #2148

Merged
merged 1 commit into from Jan 11, 2024

Conversation

botovq
Copy link
Contributor

@botovq botovq commented Jan 11, 2024

Regression introduced in #2120. Ideally, tests would not leave other errors behind.

Fixes #2146

@alex
Copy link
Collaborator

alex commented Jan 11, 2024

Hmm, my impression when I wrote that code is that [0] is the item most recently added to the stack. Is that not right?

Using last seems to presume other errors will be added there after the no start line error.

@botovq
Copy link
Contributor Author

botovq commented Jan 11, 2024

It uses Err_get_error_line_data() which is the oldest error. The code you replaced used ERR_peek_last_error() which is the latest error. Running this in a loop has been stable for more than an hour while it would previously crash after a few minutes.

unsafe extern "C" fn ERR_get_error_all(
file: *mut *const c_char,
line: *mut c_int,
func: *mut *const c_char,
data: *mut *const c_char,
flags: *mut c_int,
) -> ErrType {
let code = ffi::ERR_get_error_line_data(file, line, data, flags);
*func = ffi::ERR_func_error_string(code);
code
}

Regression introduced in sfackler#2120. Ideally, tests would not leave
other errors behind.

Fixes sfackler#2146
@alex
Copy link
Collaborator

alex commented Jan 11, 2024

😱. We probably should make that unambigiously clear in the docs. But for now: thank you!

@alex alex merged commit a14146f into sfackler:master Jan 11, 2024
53 checks passed
@botovq botovq deleted the fix_stack_from_pem branch January 11, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error while loading PEM generated on the fly
2 participants