Skip to content

Conversation

@sergio-correia
Copy link
Collaborator

@sergio-correia sergio-correia commented May 3, 2025

jose_jwk_thp_buf() returns, on success, the number of bytes written; otherwise, it returns SIZE_MAX.

We now check its return properly, from jcmd_jwk_thp(), which is used by the CLI utility.

Fixes: #170

Copy link
Collaborator

@sarroutbi sarroutbi left a comment

Choose a reason for hiding this comment

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

I would follow an if/else if approach to ease code readibility. Something like:

           size_t nbytes = jose_jwk_thp_buf(NULL, jwk, opt.hash, dec, sizeof(dec)); // dec is the output buffer
+            if (nbytes == SIZE_MAX) {
+                fprintf(stderr, "Error making thumbprint: Reached %ul (SIZE_MAX) nbytes\n", SIZE_MAX); // Please, not %ul might be adjusted here
+                return EXIT_FAILURE;
+            } else if (nbytes == 0) {
                 fprintf(stderr, "Error making thumbprint: nbytes equals 0.\n");
                 return EXIT_FAILURE;
             }

This way, in case error takes place, it will be clear where it is taking place

@sergio-correia
Copy link
Collaborator Author

This way, in case error takes place, it will be clear where it is taking place

It doesn't look like a big (if at all) improvement in this case. I will just check for the SIZE_MAX instead and make it simpler.

@sarroutbi
Copy link
Collaborator

This way, in case error takes place, it will be clear where it is taking place

It doesn't look like a big (if at all) improvement in this case. I will just check for the SIZE_MAX instead and make it simpler.

👍 OTOH ... is it feasible to add a unit test to cover this? I am just wondering

@sergio-correia
Copy link
Collaborator Author

Good point. Yeah, we can add the example from the issue report, I believe. Let me do that.

jose_jwk_thp_buf() returns, on success, the number of
bytes written; otherwise, it returns SIZE_MAX.

We now check its return properly, from jcmd_jwk_thp(),
which is used by the CLI utility.

Signed-off-by: Sergio Correia <[email protected]>
Copy link
Collaborator

@sarroutbi sarroutbi left a comment

Choose a reason for hiding this comment

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

Thanks for including the unit test. LGTM

@sarroutbi sarroutbi merged commit 3d2f6db into latchset:master May 7, 2025
22 checks passed
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.

Incorrect thumbprint calculation for Ed25519 keys

2 participants