Add missing RNP_ERROR_SIG_* strings to rnp_result_to_string() - #2403
Add missing RNP_ERROR_SIG_* strings to rnp_result_to_string()#2403ronaldtse wants to merge 4 commits into
Conversation
|
@ni4 — could you please take a look at this small PR when you have a moment? It adds the missing |
35452dd to
3a1ad03
Compare
There was a problem hiding this comment.
Pull request overview
This PR fills in missing RNP_ERROR_SIG_* mappings in rnp_result_to_string() so FFI consumers can get meaningful, non-fallback messages for signature verification failures, and extends the existing FFI test to prevent regressions.
Changes:
- Added
casestatements inrnp_result_to_string()for the full signature-validation error range (RNP_ERROR_SIG_*). - Extended
test_result_to_stringto validate the new error-code range and assert exact strings for all mapped codes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/lib/rnp.cpp | Adds string mappings for signature-validation RNP_ERROR_SIG_* result codes in rnp_result_to_string(). |
| src/tests/ffi.cpp | Extends test_result_to_string to cover the signature error range and lock down exact strings for regression prevention. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case RNP_ERROR_EOF: | ||
| return "EOF detected"; | ||
|
|
||
| case RNP_ERROR_SIG_ERROR: | ||
| return "Signature error"; |
There was a problem hiding this comment.
Done in b83a4a6 - RNP_ERROR_NOT_FOUND is now mapped in rnp_result_to_string and covered by the range and exact-string tests.
| std::vector<std::pair<rnp_result_t, rnp_result_t>> error_codes = { | ||
| {RNP_ERROR_GENERIC, RNP_ERROR_NULL_POINTER}, | ||
| {RNP_ERROR_ACCESS, RNP_ERROR_WRITE}, |
There was a problem hiding this comment.
Done in b83a4a6 - RNP_ERROR_NOT_FOUND is now mapped in rnp_result_to_string and covered by the range and exact-string tests.
b83a4a6 to
7da2a86
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2403 +/- ##
==========================================
+ Coverage 85.36% 85.41% +0.04%
==========================================
Files 126 126
Lines 22861 22916 +55
==========================================
+ Hits 19516 19573 +57
+ Misses 3345 3343 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1158b6c to
8553268
Compare
The signature validation error range (RNP_ERROR_SIG_*, starting at 0x14000000 and added in 1c616df) had no cases in rnp_result_to_string(), so all 28 codes fell through to "Unsupported error code". These codes reach FFI callers via rnp_signature_error_at() and rnp_op_verify_signature_get_status(), leaving downstream bindings without meaningful messages; surfaced by rnpgp/php-rnp#15. Add one case with a descriptive string per code, and extend test_result_to_string to cover the new range and to assert the exact string of every defined error code.
Copilot review noted that RNP_ERROR_NOT_FOUND is a defined public result code but was not covered by rnp_result_to_string() or the regression test. Add the case, extend the range check, and add the exact-string entry.
8553268 to
96e76fa
Compare
Bug
include/rnp/rnp_err.hdefines a contiguous "Signature validation" error range —RNP_ERROR_SIG_ERROR = 0x14000000throughRNP_ERROR_SIG_UNUSABLE_KEY(28 codes, added in 1c616df and 6c0a141) — butrnp_result_to_string()has no cases for any of them, so every code in the range returns"Unsupported error code".These codes are delivered to FFI consumers via
rnp_signature_error_at()andrnp_op_verify_signature_get_status(), so downstream bindings cannot turn signature verification failures into meaningful messages. Surfaced by downstream binding work: rnpgp/php-rnp#15 (same class of gap as #1878).Fix
One
caseper code with a precise string in the existing terse style, derived from the enum name and the condition under which librnp raises it, e.g.:RNP_ERROR_SIG_PARSE_ERROR→"Signature parse error"RNP_ERROR_SIG_WEAK_HASH→"Signature uses a weak hash algorithm"RNP_ERROR_SIG_NO_SIGNER_KEY→"Signature refers to no signer key"RNP_ERROR_SIG_UNUSABLE_KEY→"Signer key is not usable for verification"No other behavior change.
Tests
Extended
test_result_to_string(src/tests/ffi.cpp):{RNP_ERROR_SIG_ERROR, RNP_ERROR_SIG_UNUSABLE_KEY}to the existing loop that checks every code in a range maps to a unique, non-fallback string;Local run (macOS arm64, Botan 3.12,
-DCRYPTO_BACKEND=botan3 -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Debug): fullrnp_testssuite 272/272 passed (ctest -R rnp_tests), including the extendedrnp_tests.test_result_to_string.