Skip to content

F-3805: length check in wh_Client_CertReadTrustedResponse#365

Open
padelsbach wants to merge 2 commits into
wolfSSL:mainfrom
padelsbach:f-3805
Open

F-3805: length check in wh_Client_CertReadTrustedResponse#365
padelsbach wants to merge 2 commits into
wolfSSL:mainfrom
padelsbach:f-3805

Conversation

@padelsbach
Copy link
Copy Markdown
Contributor

No description provided.

@padelsbach padelsbach marked this pull request as ready for review May 14, 2026 18:08
Copy link
Copy Markdown
Contributor

@bigbrett bigbrett left a comment

Choose a reason for hiding this comment

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

Yay for using new test harness

Comment thread test-refactor/wh_test_list.c Outdated
const size_t whTestsServerCount = sizeof(whTestsServer) / sizeof(whTestsServer[0]);

const whTestCase whTestsClient[] = {
{ "whTest_CertReadTrustedSmallBuffer", whTest_CertReadTrustedSmallBuffer },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

instead of being "flat" where each individual test is registered here, how do we feel about this just holding whTest_ClientCerts() which is the main driver for all client-side cert tests, then the new test you added could be a static function (as well as all the other cert tests that will be added) but they are all grouped together under one "functional" suite of tests.

For example, we already essentially do this with the crypto algo tests - just have one whTest_CryptoAes() and not whTest_CryptoAes128CheckBadArgsForKey() with nine million other permutations like that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea, makes for smaller diffs as tests are added. Coming right up...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

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.

2 participants