Skip to content

PEM parsing: check last error instead of first#2148

Merged
alex merged 1 commit intorust-openssl:masterfrom
botovq:fix_stack_from_pem
Jan 11, 2024
Merged

PEM parsing: check last error instead of first#2148
alex merged 1 commit intorust-openssl:masterfrom
botovq:fix_stack_from_pem

Conversation

@botovq
Copy link
Copy Markdown
Contributor

@botovq botovq commented Jan 11, 2024

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

Fixes #2146

@alex
Copy link
Copy Markdown
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 botovq force-pushed the fix_stack_from_pem branch from b38b9a5 to 0bbc215 Compare January 11, 2024 20:38
@botovq
Copy link
Copy Markdown
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.

https://github.com/sfackler/rust-openssl/blob/06143eb363f4f59bc4af32da89f0e8d046ebe33c/openssl/src/error.rs#L367-L377

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

Fixes rust-openssl#2146
@botovq botovq force-pushed the fix_stack_from_pem branch from 0bbc215 to aa26e97 Compare January 11, 2024 20:53
@alex
Copy link
Copy Markdown
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 rust-openssl:master Jan 11, 2024
@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