From d0f8e8915f94524386588a098398cac6a8b60922 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Mon, 23 Sep 2024 18:40:11 -0400 Subject: [PATCH] Revert "improve encodeBase64Url performance (#2774)" This reverts commit b3d98e0049a364818ef8428153a8fb2436f99cc3. --- src/workerd/api/crypto/aes.c++ | 3 +-- src/workerd/api/crypto/digest.c++ | 3 +-- src/workerd/api/crypto/ec.c++ | 10 +++++----- src/workerd/api/crypto/rsa.c++ | 17 ++++++++--------- src/workerd/api/node/crypto-keys.c++ | 3 +-- src/workerd/api/util.c++ | 12 ------------ src/workerd/api/util.h | 2 -- 7 files changed, 16 insertions(+), 34 deletions(-) diff --git a/src/workerd/api/crypto/aes.c++ b/src/workerd/api/crypto/aes.c++ index 2704e2d8900..79b05238e18 100644 --- a/src/workerd/api/crypto/aes.c++ +++ b/src/workerd/api/crypto/aes.c++ @@ -3,7 +3,6 @@ // https://opensource.org/licenses/Apache-2.0 #include "impl.h" -#include "util.h" #include @@ -167,7 +166,7 @@ private: SubtleCrypto::JsonWebKey jwk; jwk.kty = kj::str("oct"); - jwk.k = fastEncodeBase64Url(keyData); + jwk.k = kj::encodeBase64Url(keyData); jwk.alg = kj::str("A", lengthInBytes * 8, aesMode); jwk.key_ops = getUsages().map([](auto usage) { return kj::str(usage.name()); }); // I don't know why the spec says: diff --git a/src/workerd/api/crypto/digest.c++ b/src/workerd/api/crypto/digest.c++ index 08a11db1109..eac5bbe2d79 100644 --- a/src/workerd/api/crypto/digest.c++ +++ b/src/workerd/api/crypto/digest.c++ @@ -5,7 +5,6 @@ #include "digest.h" #include "impl.h" -#include "util.h" #include #include @@ -78,7 +77,7 @@ private: SubtleCrypto::JsonWebKey jwk; jwk.kty = kj::str("oct"); - jwk.k = fastEncodeBase64Url(keyData); + jwk.k = kj::encodeBase64Url(keyData); jwk.alg = kj::str("HS", keyAlgorithm.hash.name.slice(4)); jwk.key_ops = getUsages().map([](auto usage) { return kj::str(usage.name()); }); // I don't know why the spec says: diff --git a/src/workerd/api/crypto/ec.c++ b/src/workerd/api/crypto/ec.c++ index c78ce6f016e..0eb49bef38c 100644 --- a/src/workerd/api/crypto/ec.c++ +++ b/src/workerd/api/crypto/ec.c++ @@ -70,8 +70,8 @@ SubtleCrypto::JsonWebKey Ec::toJwk(KeyType keyType, kj::StringPtr curveName) con auto xa = handleBn(*x, groupDegreeInBytes); auto ya = handleBn(*y, groupDegreeInBytes); - jwk.x = fastEncodeBase64Url(xa); - jwk.y = fastEncodeBase64Url(ya); + jwk.x = kj::encodeBase64Url(xa); + jwk.y = kj::encodeBase64Url(ya); if (keyType == KeyType::PRIVATE) { const auto privateKey = getPrivateKey(); @@ -79,7 +79,7 @@ SubtleCrypto::JsonWebKey Ec::toJwk(KeyType keyType, kj::StringPtr curveName) con "Error getting private key material for JSON Web Key export", internalDescribeOpensslErrors()); auto pk = handleBn(*privateKey, groupDegreeInBytes); - jwk.d = fastEncodeBase64Url(pk); + jwk.d = kj::encodeBase64Url(pk); } return jwk; } @@ -993,7 +993,7 @@ private: SubtleCrypto::JsonWebKey jwk; jwk.kty = kj::str("OKP"); jwk.crv = kj::str(getAlgorithmName() == "X25519"_kj ? "X25519"_kj : "Ed25519"_kj); - jwk.x = fastEncodeBase64Url(kj::arrayPtr(rawPublicKey, publicKeyLen)); + jwk.x = kj::encodeBase64Url(kj::arrayPtr(rawPublicKey, publicKeyLen)); if (getAlgorithmName() == "Ed25519"_kj) { jwk.alg = kj::str("EdDSA"); } @@ -1011,7 +1011,7 @@ private: KJ_ASSERT(privateKeyLen == 32, privateKeyLen); - jwk.d = fastEncodeBase64Url(kj::arrayPtr(rawPrivateKey, privateKeyLen)); + jwk.d = kj::encodeBase64Url(kj::arrayPtr(rawPrivateKey, privateKeyLen)); } return jwk; diff --git a/src/workerd/api/crypto/rsa.c++ b/src/workerd/api/crypto/rsa.c++ index 656b76b63cf..8ad0df831c5 100644 --- a/src/workerd/api/crypto/rsa.c++ +++ b/src/workerd/api/crypto/rsa.c++ @@ -3,7 +3,6 @@ #include "impl.h" #include "keys.h" #include "simdutf.h" -#include "util.h" #include #include @@ -236,20 +235,20 @@ SubtleCrypto::JsonWebKey Rsa::toJwk( jwk.alg = kj::mv(name); } - jwk.n = fastEncodeBase64Url(KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(n)))); - jwk.e = fastEncodeBase64Url(KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(e)))); + jwk.n = kj::encodeBase64Url(KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(n)))); + jwk.e = kj::encodeBase64Url(KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(e)))); if (keyType == KeyType::PRIVATE) { - jwk.d = fastEncodeBase64Url(KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(d)))); + jwk.d = kj::encodeBase64Url(KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(d)))); jwk.p = - fastEncodeBase64Url(KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(RSA_get0_p(rsa))))); + kj::encodeBase64Url(KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(RSA_get0_p(rsa))))); jwk.q = - fastEncodeBase64Url(KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(RSA_get0_q(rsa))))); - jwk.dp = fastEncodeBase64Url( + kj::encodeBase64Url(KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(RSA_get0_q(rsa))))); + jwk.dp = kj::encodeBase64Url( KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(RSA_get0_dmp1(rsa))))); - jwk.dq = fastEncodeBase64Url( + jwk.dq = kj::encodeBase64Url( KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(RSA_get0_dmq1(rsa))))); - jwk.qi = fastEncodeBase64Url( + jwk.qi = kj::encodeBase64Url( KJ_REQUIRE_NONNULL(bignumToArray(KJ_REQUIRE_NONNULL(RSA_get0_iqmp(rsa))))); } diff --git a/src/workerd/api/node/crypto-keys.c++ b/src/workerd/api/node/crypto-keys.c++ index 4c9f558b22a..c9e4ad38bf7 100644 --- a/src/workerd/api/node/crypto-keys.c++ +++ b/src/workerd/api/node/crypto-keys.c++ @@ -2,7 +2,6 @@ // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 #include "crypto.h" -#include "util.h" #include @@ -47,7 +46,7 @@ public: if (format == "jwk") { SubtleCrypto::JsonWebKey jwk; jwk.kty = kj::str("oct"); - jwk.k = fastEncodeBase64Url(keyData); + jwk.k = kj::encodeBase64Url(keyData); jwk.ext = true; return jwk; } diff --git a/src/workerd/api/util.c++ b/src/workerd/api/util.c++ index 46527451f5f..b36e4df16c0 100644 --- a/src/workerd/api/util.c++ +++ b/src/workerd/api/util.c++ @@ -4,8 +4,6 @@ #include "util.h" -#include "simdutf.h" - #include #include #include @@ -290,14 +288,4 @@ void maybeWarnIfNotText(jsg::Lock& js, kj::StringPtr str) { "\". The result will probably be corrupted. Consider " "checking the Content-Type header before interpreting entities as text.")); } - -kj::String fastEncodeBase64Url(kj::ArrayPtr bytes) { - auto expected_length = simdutf::base64_length_from_binary(bytes.size(), simdutf::base64_url); - auto output = kj::heapArray(expected_length); - auto actual_length = simdutf::binary_to_base64( - bytes.asChars().begin(), bytes.size(), output.asChars().begin(), simdutf::base64_url); - KJ_ASSERT(expected_length == actual_length); - return kj::String(kj::mv(output)); -} - } // namespace workerd::api diff --git a/src/workerd/api/util.h b/src/workerd/api/util.h index d57e674aa67..f86dbc96a66 100644 --- a/src/workerd/api/util.h +++ b/src/workerd/api/util.h @@ -102,6 +102,4 @@ double dateNow(); void maybeWarnIfNotText(jsg::Lock& js, kj::StringPtr str); -kj::String fastEncodeBase64Url(kj::ArrayPtr bytes); - } // namespace workerd::api