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 <ctiller@users.noreply.github.com>

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

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

src/core/lib/surface/validate_metadata.cc

@@ -24,22 +24,28 @@
 #include <grpc/grpc.h>
 #include <grpc/support/alloc.h>
 
+#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) {

tools/codegen/core/gen_legal_metadata_characters.cc

@@ -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 <stdio.h>
-#include <string.h>
-
-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;
-}