Skip to content

Commit 80636da

Browse files
committed
fix: address review findings across new modules
- getCipherInfo: use canonical OpenSSL name via cipher.getName(), remove number from signature - Certificate: use safer empty ArrayBuffer pattern (non-null data pointer) - Argon2: include OpenSSL error string in failure messages - Prime: clean up callback signatures with proper union types - Tests: strengthen verifySpkac assertion to assert.isTrue
1 parent 412d20a commit 80636da

6 files changed

Lines changed: 28 additions & 27 deletions

File tree

example/src/tests/certificate/certificate_tests.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ const invalidSpkac = 'not-a-valid-spkac';
2121

2222
test(SUITE, 'verifySpkac returns true for valid SPKAC', () => {
2323
const result = Certificate.verifySpkac(validSpkac);
24-
assert.isBoolean(result);
25-
// Note: result may be false if OpenSSL version doesn't support the specific key format
26-
// The important thing is that it doesn't throw
24+
assert.isTrue(result);
2725
});
2826

2927
test(SUITE, 'verifySpkac returns false for invalid SPKAC', () => {

packages/react-native-quick-crypto/cpp/argon2/HybridArgon2.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include <NitroModules/ArrayBuffer.hpp>
22
#include <memory>
33
#include <ncrypto.h>
4+
#include <openssl/err.h>
45
#include <openssl/opensslv.h>
56
#include <string>
67

@@ -50,7 +51,9 @@ static std::shared_ptr<ArrayBuffer> hashImpl(const std::string& algorithm, const
5051
static_cast<uint32_t>(passes), static_cast<uint32_t>(version), secretBuf, adBuf, type);
5152

5253
if (!result) {
53-
throw std::runtime_error("Argon2 operation failed");
54+
unsigned long err = ERR_peek_last_error();
55+
const char* reason = err ? ERR_reason_error_string(err) : nullptr;
56+
throw std::runtime_error(reason ? std::string("Argon2 operation failed: ") + reason : "Argon2 operation failed");
5457
}
5558

5659
return ToNativeArrayBuffer(reinterpret_cast<const uint8_t*>(result.get()), result.size());

packages/react-native-quick-crypto/cpp/certificate/HybridCertificate.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@ std::shared_ptr<ArrayBuffer> HybridCertificate::exportPublicKey(const std::share
1313
auto bio = ncrypto::ExportPublicKey(reinterpret_cast<const char*>(spkac->data()), spkac->size());
1414

1515
if (!bio) {
16-
return std::make_shared<NativeArrayBuffer>(nullptr, 0, nullptr);
16+
auto empty = new uint8_t[0];
17+
return std::make_shared<NativeArrayBuffer>(empty, 0, [empty]() { delete[] empty; });
1718
}
1819

1920
BUF_MEM* mem = bio;
2021
if (!mem || mem->length == 0) {
21-
return std::make_shared<NativeArrayBuffer>(nullptr, 0, nullptr);
22+
auto empty = new uint8_t[0];
23+
return std::make_shared<NativeArrayBuffer>(empty, 0, [empty]() { delete[] empty; });
2224
}
2325

2426
return ToNativeArrayBuffer(reinterpret_cast<const uint8_t*>(mem->data), mem->length);
@@ -28,7 +30,8 @@ std::shared_ptr<ArrayBuffer> HybridCertificate::exportChallenge(const std::share
2830
auto buf = ncrypto::ExportChallenge(reinterpret_cast<const char*>(spkac->data()), spkac->size());
2931

3032
if (buf.data == nullptr) {
31-
return std::make_shared<NativeArrayBuffer>(nullptr, 0, nullptr);
33+
auto empty = new uint8_t[0];
34+
return std::make_shared<NativeArrayBuffer>(empty, 0, [empty]() { delete[] empty; });
3235
}
3336

3437
auto result = ToNativeArrayBuffer(reinterpret_cast<const uint8_t*>(buf.data), buf.len);

packages/react-native-quick-crypto/cpp/cipher/HybridCipher.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ std::optional<CipherInfo> HybridCipher::getCipherInfo(const std::string& name, s
376376
}
377377
}
378378

379-
std::string name_str(name);
379+
std::string name_str(cipher.getName());
380380
std::transform(name_str.begin(), name_str.end(), name_str.begin(), ::tolower);
381381

382382
std::string mode_str(cipher.getModeLabel());

packages/react-native-quick-crypto/src/cipher.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,11 @@ export function getCiphers(): string[] {
5757
}
5858

5959
export function getCipherInfo(
60-
nameOrNid: string | number,
60+
name: string,
6161
options?: { keyLength?: number; ivLength?: number },
6262
): CipherInfoResult | undefined {
63-
if (typeof nameOrNid === 'string') {
64-
if (nameOrNid.length === 0) return undefined;
65-
return CipherUtils.getCipherInfo(
66-
nameOrNid,
67-
options?.keyLength,
68-
options?.ivLength,
69-
);
70-
}
71-
throw new TypeError('nameOrNid must be a string');
63+
if (typeof name !== 'string' || name.length === 0) return undefined;
64+
return CipherUtils.getCipherInfo(name, options?.keyLength, options?.ivLength);
7265
}
7366

7467
interface CipherArgs {

packages/react-native-quick-crypto/src/prime.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,16 +66,18 @@ export function generatePrimeSync(
6666
return result;
6767
}
6868

69+
type GeneratePrimeCallback = (
70+
err: Error | null,
71+
prime: Buffer | bigint,
72+
) => void;
73+
6974
export function generatePrime(
7075
size: number,
71-
options?: GeneratePrimeOptions,
72-
callback?: (err: Error | null, prime: Buffer | bigint) => void,
76+
options: GeneratePrimeOptions | GeneratePrimeCallback,
77+
callback?: GeneratePrimeCallback,
7378
): void {
7479
if (typeof options === 'function') {
75-
callback = options as unknown as (
76-
err: Error | null,
77-
prime: Buffer | bigint,
78-
) => void;
80+
callback = options;
7981
options = {};
8082
}
8183
const safe = options?.safe ?? false;
@@ -108,16 +110,18 @@ export function checkPrimeSync(
108110
return getNative().checkPrimeSync(buf, checks);
109111
}
110112

113+
type CheckPrimeCallback = (err: Error | null, result: boolean) => void;
114+
111115
export function checkPrime(
112116
candidate: BinaryLike | bigint,
113-
options?: CheckPrimeOptions | ((err: Error | null, result: boolean) => void),
114-
callback?: (err: Error | null, result: boolean) => void,
117+
options: CheckPrimeOptions | CheckPrimeCallback,
118+
callback?: CheckPrimeCallback,
115119
): void {
116120
if (typeof options === 'function') {
117121
callback = options;
118122
options = {};
119123
}
120-
const checks = (options as CheckPrimeOptions)?.checks ?? 0;
124+
const checks = options.checks ?? 0;
121125
const buf =
122126
typeof candidate === 'bigint'
123127
? bigIntToBuffer(candidate)

0 commit comments

Comments
 (0)