Skip to content

Commit

Permalink
src: rename IsAnyByteSource to IsAnyBufferSource
Browse files Browse the repository at this point in the history
The current name is somewhat confusing. There is an internal ByteSource
class, which is entirely unrelated to what IsAnyByteSource() does, even
though both exist in the crypto subsystem of the C++ code. ByteSource
objects can also be constructed from strings, for example, for which
IsAnyByteSource() returns false.

Web IDL calls the types for which this function returns true
BufferSource. This type is commonly used across Web Crypto, for example.
Thus, rename the function to match the Web IDL naming.

Because the function also appears to accept BufferSource objects backed
by SharedArrayBuffer instances, the exact Web IDL name would be
AllowSharedBufferSource, but that seems unnecessarily long, so I decided
to stick with "any".

PR-URL: nodejs#49346
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
  • Loading branch information
tniessen committed Aug 29, 2023
1 parent 452bae1 commit 43704dd
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 21 deletions.
4 changes: 2 additions & 2 deletions src/crypto/crypto_aes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ bool ValidateAuthTag(
AESCipherConfig* params) {
switch (cipher_mode) {
case kWebCryptoCipherDecrypt: {
if (!IsAnyByteSource(value)) {
if (!IsAnyBufferSource(value)) {
THROW_ERR_CRYPTO_INVALID_TAG_LENGTH(env);
return false;
}
Expand Down Expand Up @@ -419,7 +419,7 @@ bool ValidateAdditionalData(
Local<Value> value,
AESCipherConfig* params) {
// Additional Data
if (IsAnyByteSource(value)) {
if (IsAnyBufferSource(value)) {
ArrayBufferOrViewContents<char> additional(value);
if (UNLIKELY(!additional.CheckSizeInt32())) {
THROW_ERR_OUT_OF_RANGE(env, "additionalData is too big");
Expand Down
6 changes: 3 additions & 3 deletions src/crypto/crypto_ec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ ECPointPointer ECDH::BufferToPoint(Environment* env,
void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK(IsAnyByteSource(args[0]));
CHECK(IsAnyBufferSource(args[0]));

ECDH* ecdh;
ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder());
Expand Down Expand Up @@ -347,7 +347,7 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo<Value>& args) {
ECDH* ecdh;
ASSIGN_OR_RETURN_UNWRAP(&ecdh, args.Holder());

CHECK(IsAnyByteSource(args[0]));
CHECK(IsAnyBufferSource(args[0]));

MarkPopErrorOnReturn mark_pop_error_on_return;

Expand Down Expand Up @@ -393,7 +393,7 @@ void ECDH::ConvertKey(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK_EQ(args.Length(), 3);
CHECK(IsAnyByteSource(args[0]));
CHECK(IsAnyBufferSource(args[0]));

ArrayBufferOrViewContents<char> args0(args[0]);
if (UNLIKELY(!args0.CheckSizeInt32()))
Expand Down
4 changes: 2 additions & 2 deletions src/crypto/crypto_hkdf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ Maybe<bool> HKDFTraits::AdditionalConfig(

CHECK(args[offset]->IsString()); // Hash
CHECK(args[offset + 1]->IsObject()); // Key
CHECK(IsAnyByteSource(args[offset + 2])); // Salt
CHECK(IsAnyByteSource(args[offset + 3])); // Info
CHECK(IsAnyBufferSource(args[offset + 2])); // Salt
CHECK(IsAnyBufferSource(args[offset + 3])); // Info
CHECK(args[offset + 4]->IsUint32()); // Length

Utf8Value hash(env->isolate(), args[offset]);
Expand Down
6 changes: 3 additions & 3 deletions src/crypto/crypto_keys.cc
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ ManagedEVPPKey::GetPrivateKeyEncodingFromJs(
(*offset)++;
}

if (IsAnyByteSource(args[*offset])) {
if (IsAnyBufferSource(args[*offset])) {
CHECK_IMPLIES(context != kKeyContextInput, result.cipher_ != nullptr);
ArrayBufferOrViewContents<char> passphrase(args[*offset]);
if (UNLIKELY(!passphrase.CheckSizeInt32())) {
Expand Down Expand Up @@ -730,7 +730,7 @@ ManagedEVPPKey ManagedEVPPKey::GetPrivateKeyFromJs(
const FunctionCallbackInfo<Value>& args,
unsigned int* offset,
bool allow_key_object) {
if (args[*offset]->IsString() || IsAnyByteSource(args[*offset])) {
if (args[*offset]->IsString() || IsAnyBufferSource(args[*offset])) {
Environment* env = Environment::GetCurrent(args);
ByteSource key = ByteSource::FromStringOrBuffer(env, args[(*offset)++]);
NonCopyableMaybe<PrivateKeyEncodingConfig> config =
Expand All @@ -756,7 +756,7 @@ ManagedEVPPKey ManagedEVPPKey::GetPrivateKeyFromJs(
ManagedEVPPKey ManagedEVPPKey::GetPublicOrPrivateKeyFromJs(
const FunctionCallbackInfo<Value>& args,
unsigned int* offset) {
if (IsAnyByteSource(args[*offset])) {
if (IsAnyBufferSource(args[*offset])) {
Environment* env = Environment::GetCurrent(args);
ArrayBufferOrViewContents<char> data(args[(*offset)++]);
if (UNLIKELY(!data.CheckSizeInt32())) {
Expand Down
2 changes: 1 addition & 1 deletion src/crypto/crypto_random.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Maybe<bool> RandomBytesTraits::AdditionalConfig(
const FunctionCallbackInfo<Value>& args,
unsigned int offset,
RandomBytesConfig* params) {
CHECK(IsAnyByteSource(args[offset])); // Buffer to fill
CHECK(IsAnyBufferSource(args[offset])); // Buffer to fill
CHECK(args[offset + 1]->IsUint32()); // Offset
CHECK(args[offset + 2]->IsUint32()); // Size

Expand Down
2 changes: 1 addition & 1 deletion src/crypto/crypto_rsa.cc
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ Maybe<bool> RSACipherTraits::AdditionalConfig(
return Nothing<bool>();
}

if (IsAnyByteSource(args[offset + 2])) {
if (IsAnyBufferSource(args[offset + 2])) {
ArrayBufferOrViewContents<char> label(args[offset + 2]);
if (UNLIKELY(!label.CheckSizeInt32())) {
THROW_ERR_OUT_OF_RANGE(env, "label is too big");
Expand Down
4 changes: 2 additions & 2 deletions src/crypto/crypto_timing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ void TimingSafeEqual(const FunctionCallbackInfo<Value>& args) {
// to V8 inlining certain parts of the wrapper. Therefore, keep them in C++.
// Refs: https://github.com/nodejs/node/issues/34073.
Environment* env = Environment::GetCurrent(args);
if (!IsAnyByteSource(args[0])) {
if (!IsAnyBufferSource(args[0])) {
THROW_ERR_INVALID_ARG_TYPE(
env, "The \"buf1\" argument must be an instance of "
"ArrayBuffer, Buffer, TypedArray, or DataView.");
return;
}
if (!IsAnyByteSource(args[1])) {
if (!IsAnyBufferSource(args[1])) {
THROW_ERR_INVALID_ARG_TYPE(
env, "The \"buf2\" argument must be an instance of "
"ArrayBuffer, Buffer, TypedArray, or DataView.");
Expand Down
10 changes: 5 additions & 5 deletions src/crypto/crypto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,8 @@ ByteSource ByteSource::FromEncodedString(Environment* env,

ByteSource ByteSource::FromStringOrBuffer(Environment* env,
Local<Value> value) {
return IsAnyByteSource(value) ? FromBuffer(value)
: FromString(env, value.As<String>());
return IsAnyBufferSource(value) ? FromBuffer(value)
: FromString(env, value.As<String>());
}

ByteSource ByteSource::FromString(Environment* env, Local<String> str,
Expand All @@ -429,9 +429,9 @@ ByteSource ByteSource::FromSecretKeyBytes(
// A key can be passed as a string, buffer or KeyObject with type 'secret'.
// If it is a string, we need to convert it to a buffer. We are not doing that
// in JS to avoid creating an unprotected copy on the heap.
return value->IsString() || IsAnyByteSource(value) ?
ByteSource::FromStringOrBuffer(env, value) :
ByteSource::FromSymmetricKeyObjectHandle(value);
return value->IsString() || IsAnyBufferSource(value)
? ByteSource::FromStringOrBuffer(env, value)
: ByteSource::FromSymmetricKeyObjectHandle(value);
}

ByteSource ByteSource::NullTerminatedCopy(Environment* env,
Expand Down
5 changes: 3 additions & 2 deletions src/crypto/crypto_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,8 @@ void array_push_back(const TypeName* evp_ref,
}
#endif

inline bool IsAnyByteSource(v8::Local<v8::Value> arg) {
// WebIDL AllowSharedBufferSource.
inline bool IsAnyBufferSource(v8::Local<v8::Value> arg) {
return arg->IsArrayBufferView() ||
arg->IsArrayBuffer() ||
arg->IsSharedArrayBuffer();
Expand All @@ -694,7 +695,7 @@ class ArrayBufferOrViewContents {
return;
}

CHECK(IsAnyByteSource(buf));
CHECK(IsAnyBufferSource(buf));
if (buf->IsArrayBufferView()) {
auto view = buf.As<v8::ArrayBufferView>();
offset_ = view->ByteOffset();
Expand Down

0 comments on commit 43704dd

Please sign in to comment.