Skip to content

Commit

Permalink
Update test cases
Browse files Browse the repository at this point in the history
Introduce utility class for encoding and refactor
  • Loading branch information
Joe-Abraham committed Sep 18, 2024
1 parent 118ff8f commit 54ad16a
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 72 deletions.
38 changes: 1 addition & 37 deletions velox/common/encode/Base64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>(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(
Expand All @@ -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<char>(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(
Expand Down Expand Up @@ -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
Expand Down
22 changes: 1 addition & 21 deletions velox/common/encode/Base64.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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
Expand All @@ -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);
};

Expand Down
89 changes: 89 additions & 0 deletions velox/common/encode/EncoderUtils.h
Original file line number Diff line number Diff line change
@@ -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 <string>
#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 <typename Charset, typename ReverseIndex>
constexpr bool checkForwardIndex(
uint8_t idx,
const Charset& charset,
const ReverseIndex& reverseIndex) {
return (reverseIndex[static_cast<uint8_t>(charset[idx])] == idx) &&
(idx > 0 ? checkForwardIndex(idx - 1, charset, reverseIndex) : true);
}

// Searches for a character within a charset up to a certain index.
template <typename Charset>
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 <typename Charset, typename ReverseIndex>
constexpr bool checkReverseIndex(
uint8_t index,
const Charset& charset,
const ReverseIndex& reverseIndex) {
return (reverseIndex[index] == 255
? !findCharacterInCharset(charset, 0, static_cast<char>(index))
: (charset[reverseIndex[index]] == index)) &&
(index > 0 ? checkReverseIndex(index - 1, charset, reverseIndex) : true);
}

template <typename ReverseIndexType>
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
15 changes: 2 additions & 13 deletions velox/common/encode/tests/Base64Test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion velox/common/encode/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions velox/common/encode/tests/EncoderUtilsTests.cpp
Original file line number Diff line number Diff line change
@@ -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 <gtest/gtest.h>
#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

0 comments on commit 54ad16a

Please sign in to comment.