From 54ad16a00865fea3ff24996bbbea56199138673c Mon Sep 17 00:00:00 2001 From: Joe Abraham Date: Wed, 7 Aug 2024 18:44:55 +0530 Subject: [PATCH] Update test cases Introduce utility class for encoding and refactor --- velox/common/encode/Base64.cpp | 38 +------- velox/common/encode/Base64.h | 22 +---- velox/common/encode/EncoderUtils.h | 89 +++++++++++++++++++ velox/common/encode/tests/Base64Test.cpp | 15 +--- velox/common/encode/tests/CMakeLists.txt | 2 +- .../common/encode/tests/EncoderUtilsTests.cpp | 35 ++++++++ 6 files changed, 129 insertions(+), 72 deletions(-) create mode 100644 velox/common/encode/EncoderUtils.h create mode 100644 velox/common/encode/tests/EncoderUtilsTests.cpp diff --git a/velox/common/encode/Base64.cpp b/velox/common/encode/Base64.cpp index dfb6bc88f0e9..41209366f53b 100644 --- a/velox/common/encode/Base64.cpp +++ b/velox/common/encode/Base64.cpp @@ -85,15 +85,6 @@ constexpr const Base64::ReverseIndex kBase64UrlReverseIndexTable = { 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255}; -// Validate the character in charset with ReverseIndex table -constexpr bool checkForwardIndex( - uint8_t idx, - const Base64::Charset& charset, - const Base64::ReverseIndex& reverseIndex) { - return (reverseIndex[static_cast(charset[idx])] == idx) && - (idx > 0 ? checkForwardIndex(idx - 1, charset, reverseIndex) : true); -} - // Verify that for every entry in kBase64Charset, the corresponding entry // in kBase64ReverseIndexTable is correct. static_assert( @@ -112,27 +103,6 @@ static_assert( kBase64UrlReverseIndexTable), "kBase64UrlCharset has incorrect entries"); -// Searches for a character within a charset up to a certain index. -constexpr bool findCharacterInCharset( - const Base64::Charset& charset, - uint8_t index, - const char character) { - return index < charset.size() && - ((charset[index] == character) || - findCharacterInCharset(charset, index + 1, character)); -} - -// Checks the consistency of a reverse index mapping for a given character set. -constexpr bool checkReverseIndex( - uint8_t index, - const Base64::Charset& charset, - const Base64::ReverseIndex& reverseIndex) { - return (reverseIndex[index] == 255 - ? !findCharacterInCharset(charset, 0, static_cast(index)) - : (charset[reverseIndex[index]] == index)) && - (index > 0 ? checkReverseIndex(index - 1, charset, reverseIndex) : true); -} - // Verify that for every entry in kBase64ReverseIndexTable, the corresponding // entry in kBase64Charset is correct. static_assert( @@ -327,13 +297,7 @@ uint8_t Base64::base64ReverseLookup( char p, const Base64::ReverseIndex& reverseIndex, Status& status) { - auto curr = reverseIndex[(uint8_t)p]; - if (curr >= 0x40) { - status = Status::UserError( - "Base64::decode() - invalid input string: contains invalid characters."); - return 0; // Return 0 or any other error code indicating failure - } - return curr; + return reverseLookup(p, reverseIndex, status, Base64::kCharsetSize); } // static diff --git a/velox/common/encode/Base64.h b/velox/common/encode/Base64.h index efeb42054ee4..7041a181662e 100644 --- a/velox/common/encode/Base64.h +++ b/velox/common/encode/Base64.h @@ -23,6 +23,7 @@ #include "velox/common/base/GTestMacros.h" #include "velox/common/base/Status.h" +#include "velox/common/encode/EncoderUtils.h" namespace facebook::velox::encoding { @@ -111,24 +112,6 @@ class Base64 { size_t outputSize); private: - // Padding character used in encoding. - static const char kPadding = '='; - - // Checks if there is padding in encoded input. - static inline bool isPadded(std::string_view input, size_t inputSize) { - return (inputSize > 0 && input[inputSize - 1] == kPadding); - } - - // Counts the number of padding characters in encoded input. - static inline size_t numPadding(std::string_view input, size_t inputSize) { - size_t numPadding{0}; - while (inputSize > 0 && input[inputSize - 1] == kPadding) { - numPadding++; - inputSize--; - } - return numPadding; - } - // Performs a reverse lookup in the reverse index to retrieve the original // index of a character in the base. static uint8_t @@ -154,9 +137,6 @@ class Base64 { char* output, size_t outputSize, const ReverseIndex& reverseIndex); - - VELOX_FRIEND_TEST(Base64Test, isPadded); - VELOX_FRIEND_TEST(Base64Test, numPadding); VELOX_FRIEND_TEST(Base64Test, testDecodeImpl); }; diff --git a/velox/common/encode/EncoderUtils.h b/velox/common/encode/EncoderUtils.h new file mode 100644 index 000000000000..167cb73c0b55 --- /dev/null +++ b/velox/common/encode/EncoderUtils.h @@ -0,0 +1,89 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include +#include "velox/common/base/Status.h" + +namespace facebook::velox::encoding { + +/// Padding character used in encoding. +const static char kPadding = '='; + +// Checks if there is padding in encoded input. +static inline bool isPadded(std::string_view input, size_t inputSize) { + return (inputSize > 0 && input[inputSize - 1] == kPadding); +} + +// Counts the number of padding characters in encoded input. +static inline size_t numPadding(std::string_view input, size_t inputSize) { + size_t numPadding{0}; + while (inputSize > 0 && input[inputSize - 1] == kPadding) { + numPadding++; + inputSize--; + } + return numPadding; +} + +// Validate the character in charset with ReverseIndex table +template +constexpr bool checkForwardIndex( + uint8_t idx, + const Charset& charset, + const ReverseIndex& reverseIndex) { + return (reverseIndex[static_cast(charset[idx])] == idx) && + (idx > 0 ? checkForwardIndex(idx - 1, charset, reverseIndex) : true); +} + +// Searches for a character within a charset up to a certain index. +template +constexpr bool findCharacterInCharset( + const Charset& charset, + uint8_t index, + const char character) { + return index < charset.size() && + ((charset[index] == character) || + findCharacterInCharset(charset, index + 1, character)); +} + +// Checks the consistency of a reverse index mapping for a given character set. +template +constexpr bool checkReverseIndex( + uint8_t index, + const Charset& charset, + const ReverseIndex& reverseIndex) { + return (reverseIndex[index] == 255 + ? !findCharacterInCharset(charset, 0, static_cast(index)) + : (charset[reverseIndex[index]] == index)) && + (index > 0 ? checkReverseIndex(index - 1, charset, reverseIndex) : true); +} + +template +uint8_t reverseLookup( + char p, + const ReverseIndexType& reverseIndex, + Status& status, + uint8_t kBase) { + auto curr = reverseIndex[(uint8_t)p]; + if (curr >= kBase) { + status = + Status::UserError("invalid input string: contains invalid characters."); + return 0; // Return 0 or any other error code indicating failure + } + return curr; +} + +} // namespace facebook::velox::encoding \ No newline at end of file diff --git a/velox/common/encode/tests/Base64Test.cpp b/velox/common/encode/tests/Base64Test.cpp index 736dd5688f9e..df56ce524d72 100644 --- a/velox/common/encode/tests/Base64Test.cpp +++ b/velox/common/encode/tests/Base64Test.cpp @@ -76,17 +76,6 @@ TEST_F(Base64Test, calculateDecodedSizeImproperSize) { "SGVsbG8sIFdvcmxkIQ===", encodedSize, decodedSize)); } -TEST_F(Base64Test, isPadded) { - EXPECT_TRUE(Base64::isPadded("ABC=", 4)); - EXPECT_FALSE(Base64::isPadded("ABC", 3)); -} - -TEST_F(Base64Test, numPadding) { - EXPECT_EQ(0, Base64::numPadding("ABC", 3)); - EXPECT_EQ(1, Base64::numPadding("ABC=", 4)); - EXPECT_EQ(2, Base64::numPadding("AB==", 4)); -} - TEST_F(Base64Test, testDecodeImpl) { constexpr const Base64::ReverseIndex reverseTable = { 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, @@ -131,7 +120,7 @@ TEST_F(Base64Test, testDecodeImpl) { output1, sizeof(output1), Status::UserError( - "Base64::decode() - invalid input string: contains invalid characters.")); + "invalid input string: contains invalid characters.")); // All characters are padding characters testDecode("====", output1, sizeof(output1), Status::OK()); @@ -170,7 +159,7 @@ TEST_F(Base64Test, testDecodeImpl) { output1, sizeof(output1), Status::UserError( - "Base64::decode() - invalid input string: contains invalid characters.")); + "invalid input string: contains invalid characters.")); // insufficient buffer size testDecode( diff --git a/velox/common/encode/tests/CMakeLists.txt b/velox/common/encode/tests/CMakeLists.txt index 266e091d0291..29e34fb8f45d 100644 --- a/velox/common/encode/tests/CMakeLists.txt +++ b/velox/common/encode/tests/CMakeLists.txt @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -add_executable(velox_common_encode_test Base64Test.cpp) +add_executable(velox_common_encode_test Base64Test.cpp EncoderUtilsTests.cpp) add_test(velox_common_encode_test velox_common_encode_test) target_link_libraries( velox_common_encode_test diff --git a/velox/common/encode/tests/EncoderUtilsTests.cpp b/velox/common/encode/tests/EncoderUtilsTests.cpp new file mode 100644 index 000000000000..22a1ce1af9fb --- /dev/null +++ b/velox/common/encode/tests/EncoderUtilsTests.cpp @@ -0,0 +1,35 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include "velox/common/base/tests/GTestUtils.h" +#include "velox/common/encode/EncoderUtils.h" + +namespace facebook::velox::encoding { +class EncoderUtilsTest : public ::testing::Test {}; + +TEST_F(EncoderUtilsTest, isPadded) { + EXPECT_TRUE(isPadded("ABC=", 4)); + EXPECT_FALSE(isPadded("ABC", 3)); +} + +TEST_F(EncoderUtilsTest, numPadding) { + EXPECT_EQ(0, numPadding("ABC", 3)); + EXPECT_EQ(1, numPadding("ABC=", 4)); + EXPECT_EQ(2, numPadding("AB==", 4)); +} + +} // namespace facebook::velox::encoding