From 46bd55233748842926b6e8b2cef9a3be2510cc37 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 17 Aug 2021 20:09:16 -0700 Subject: [PATCH] Eliminate gen_legal_metadata_characters (#27019) * eliminate gen_legal_metadata_characters * Automated change: Fix sanity tests * Automated change: Fix sanity tests * fix * Automated change: Fix sanity tests Co-authored-by: ctiller --- CMakeLists.txt | 27 -------- build_handwritten.yaml | 6 -- src/core/lib/surface/validate_metadata.cc | 57 +++++++++++----- .../core/gen_legal_metadata_characters.cc | 66 ------------------- 4 files changed, 42 insertions(+), 114 deletions(-) delete mode 100644 tools/codegen/core/gen_legal_metadata_characters.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 1821ff3c51f..6bbdc77feb5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -377,7 +377,6 @@ add_custom_target(tools_c add_custom_target(tools_cxx DEPENDS gen_hpack_tables - gen_legal_metadata_characters ) add_custom_target(tools @@ -3998,32 +3997,6 @@ target_link_libraries(gen_hpack_tables ) - -add_executable(gen_legal_metadata_characters - tools/codegen/core/gen_legal_metadata_characters.cc -) - -target_include_directories(gen_legal_metadata_characters - PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR} - ${CMAKE_CURRENT_SOURCE_DIR}/include - ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} - ${_gRPC_RE2_INCLUDE_DIR} - ${_gRPC_SSL_INCLUDE_DIR} - ${_gRPC_UPB_GENERATED_DIR} - ${_gRPC_UPB_GRPC_GENERATED_DIR} - ${_gRPC_UPB_INCLUDE_DIR} - ${_gRPC_XXHASH_INCLUDE_DIR} - ${_gRPC_ZLIB_INCLUDE_DIR} - ${_gRPC_PROTO_GENS_DIR} -) - -target_link_libraries(gen_legal_metadata_characters - ${_gRPC_PROTOBUF_LIBRARIES} - ${_gRPC_ALLTARGETS_LIBRARIES} -) - - if(gRPC_BUILD_TESTS) add_executable(algorithm_test diff --git a/build_handwritten.yaml b/build_handwritten.yaml index 0966e99e5f9..59120ba0e6d 100644 --- a/build_handwritten.yaml +++ b/build_handwritten.yaml @@ -35,12 +35,6 @@ targets: - grpc - gpr uses_polling: false -- name: gen_legal_metadata_characters - build: tool - language: c++ - src: - - tools/codegen/core/gen_legal_metadata_characters.cc - deps: [] configs: asan: CC: clang diff --git a/src/core/lib/surface/validate_metadata.cc b/src/core/lib/surface/validate_metadata.cc index c1ec936e471..929f55c7408 100644 --- a/src/core/lib/surface/validate_metadata.cc +++ b/src/core/lib/surface/validate_metadata.cc @@ -24,22 +24,28 @@ #include #include +#include "src/core/lib/gprpp/bitset.h" #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/iomgr/error.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/slice/slice_string_helpers.h" #include "src/core/lib/surface/validate_metadata.h" +#if __cplusplus > 201103l +#define GRPC_VALIDATE_METADATA_CONSTEXPR_FN constexpr +#define GRPC_VALIDATE_METADATA_CONSTEXPR_VALUE constexpr +#else +#define GRPC_VALIDATE_METADATA_CONSTEXPR_FN +#define GRPC_VALIDATE_METADATA_CONSTEXPR_VALUE const +#endif + static grpc_error_handle conforms_to(const grpc_slice& slice, - const uint8_t* legal_bits, + const grpc_core::BitSet<256>& legal_bits, const char* err_desc) { const uint8_t* p = GRPC_SLICE_START_PTR(slice); const uint8_t* e = GRPC_SLICE_END_PTR(slice); for (; p != e; p++) { - int idx = *p; - int byte = idx / 8; - int bit = idx % 8; - if ((legal_bits[byte] & (1 << bit)) == 0) { + if (!legal_bits.is_set(*p)) { grpc_error_handle error = grpc_error_set_str( grpc_error_set_int(GRPC_ERROR_CREATE_FROM_COPIED_STRING(err_desc), GRPC_ERROR_INT_OFFSET, @@ -58,11 +64,22 @@ static int error2int(grpc_error_handle error) { return r; } +namespace { +class LegalHeaderKeyBits : public grpc_core::BitSet<256> { + public: + GRPC_VALIDATE_METADATA_CONSTEXPR_FN LegalHeaderKeyBits() { + for (int i = 'a'; i <= 'z'; i++) set(i); + for (int i = '0'; i <= '9'; i++) set(i); + set('-'); + set('_'); + set('.'); + } +}; +static GRPC_VALIDATE_METADATA_CONSTEXPR_VALUE LegalHeaderKeyBits + g_legal_header_key_bits; +} // namespace + grpc_error_handle grpc_validate_header_key_is_legal(const grpc_slice& slice) { - static const uint8_t legal_header_bits[256 / 8] = { - 0x00, 0x00, 0x00, 0x00, 0x00, 0x60, 0xff, 0x03, 0x00, 0x00, 0x00, - 0x80, 0xfe, 0xff, 0xff, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; if (GRPC_SLICE_LENGTH(slice) == 0) { return GRPC_ERROR_CREATE_FROM_STATIC_STRING( "Metadata keys cannot be zero length"); @@ -75,20 +92,30 @@ grpc_error_handle grpc_validate_header_key_is_legal(const grpc_slice& slice) { return GRPC_ERROR_CREATE_FROM_STATIC_STRING( "Metadata keys cannot start with :"); } - return conforms_to(slice, legal_header_bits, "Illegal header key"); + return conforms_to(slice, g_legal_header_key_bits, "Illegal header key"); } int grpc_header_key_is_legal(grpc_slice slice) { return error2int(grpc_validate_header_key_is_legal(slice)); } +namespace { +class LegalHeaderNonBinValueBits : public grpc_core::BitSet<256> { + public: + GRPC_VALIDATE_METADATA_CONSTEXPR_FN LegalHeaderNonBinValueBits() { + for (int i = 32; i <= 126; i++) { + set(i); + } + } +}; +static GRPC_VALIDATE_METADATA_CONSTEXPR_VALUE LegalHeaderNonBinValueBits + g_legal_header_non_bin_value_bits; +} // namespace + grpc_error_handle grpc_validate_header_nonbin_value_is_legal( const grpc_slice& slice) { - static const uint8_t legal_header_bits[256 / 8] = { - 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; - return conforms_to(slice, legal_header_bits, "Illegal header value"); + return conforms_to(slice, g_legal_header_non_bin_value_bits, + "Illegal header value"); } int grpc_header_nonbin_value_is_legal(grpc_slice slice) { diff --git a/tools/codegen/core/gen_legal_metadata_characters.cc b/tools/codegen/core/gen_legal_metadata_characters.cc deleted file mode 100644 index fbabd2464f6..00000000000 --- a/tools/codegen/core/gen_legal_metadata_characters.cc +++ /dev/null @@ -1,66 +0,0 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * 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. - * - */ - -/* generates constant table for metadata.cc */ - -#include -#include - -static unsigned char legal_bits[256 / 8]; - -static void legal(int x) { - int byte = x / 8; - int bit = x % 8; - /* NB: the following integer arithmetic operation needs to be in its - * expanded form due to the "integral promotion" performed (see section - * 3.2.1.1 of the C89 draft standard). A cast to the smaller container type - * is then required to avoid the compiler warning */ - legal_bits[byte] = - (unsigned char)((legal_bits[byte] | (unsigned char)(1 << bit))); -} - -static void dump(void) { - int i; - - printf("static const uint8_t legal_header_bits[256/8] = "); - for (i = 0; i < 256 / 8; i++) - printf("%c 0x%02x", i ? ',' : '{', legal_bits[i]); - printf(" };\n"); -} - -static void clear(void) { memset(legal_bits, 0, sizeof(legal_bits)); } - -int main(void) { - int i; - - clear(); - for (i = 'a'; i <= 'z'; i++) legal(i); - for (i = '0'; i <= '9'; i++) legal(i); - legal('-'); - legal('_'); - legal('.'); - dump(); - - clear(); - for (i = 32; i <= 126; i++) { - legal(i); - } - dump(); - - return 0; -}