Skip to content

Conversation

@afjoseph
Copy link
Contributor

@afjoseph afjoseph commented May 23, 2025

Hey! Thanks for the great project.

I noticed that cpr::CaBuffer doesn't load all certificates in the buffer: just the first one. PEM_read_bio_X509 is meant to run in a loop (some examples) to add all certs in the buffer. This PR covers this.

It also fixes the inclusion of openssl/pemerr.h in BoringSSL, which doesn't exist. This closes #333 (comment)

There's also an unused error printer that I removed, which caused issues with -Werror

Cheers!

Copy link
Member

@COM8 COM8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@afjoseph thanks for contributing and fixing this.
Only auto needs to be replaced and a few clang-tidy things.

@COM8 COM8 added the Bug 🐛 label May 26, 2025
@COM8 COM8 added this to the CPR 1.11.x milestone May 26, 2025
@afjoseph
Copy link
Contributor Author

@COM8 Changes modified accordingly

cpr/ssl_ctx.cpp Outdated
// Create a memory BIO using the data of cert_buf
// Note: It is assumed, that cert_buf is nul terminated and its length is determined by strlen
char* cert_buf = static_cast<char*>(raw_cert_buf);
bio_ptr bio = BIO_new_mem_buf(cert_buf, -1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bio_ptr bio = BIO_new_mem_buf(cert_buf, -1);
BIO * bio = BIO_new_mem_buf(cert_buf, -1);

I guess here I suggested the wrong type. The CI fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@COM8 COM8 merged commit 2a393ff into libcpr:master Jun 14, 2025
44 checks passed
COM8 added a commit that referenced this pull request Jun 14, 2025
Load all certs in a CaBuffer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NDK and CPR

2 participants