Cast to unsigned char for comparisons.

No behavioral change if char is unsigned (for example, internal code).
Fixes incorrect behavior if char is signed (for example, Envoy).

PiperOrigin-RevId: 447703949

quiche/common/balsa/balsa_frame.cc

diff --git a/quiche/common/balsa/balsa_frame.cc b/quiche/common/balsa/balsa_frame.cc
index cb7db65..167632a 100644
--- a/quiche/common/balsa/balsa_frame.cc
+++ b/quiche/common/balsa/balsa_frame.cc
@@ -23,6 +23,21 @@
 #include "quiche/common/balsa/header_properties.h"
 #include "quiche/common/platform/api/quiche_logging.h"
 
+// When comparing characters (other than == and !=), cast to unsigned char
+// to make sure values above 127 rank as expected, even on platforms where char
+// is signed and thus such values are represented as negative numbers before the
+// cast.
+#define CHAR_LT(a, b) \
+  (static_cast<unsigned char>(a) < static_cast<unsigned char>(b))
+#define CHAR_LE(a, b) \
+  (static_cast<unsigned char>(a) <= static_cast<unsigned char>(b))
+#define CHAR_GT(a, b) \
+  (static_cast<unsigned char>(a) > static_cast<unsigned char>(b))
+#define CHAR_GE(a, b) \
+  (static_cast<unsigned char>(a) >= static_cast<unsigned char>(b))
+#define QUICHE_DCHECK_CHAR_GE(a, b) \
+  QUICHE_DCHECK_GE(static_cast<unsigned char>(a), static_cast<unsigned char>(b))
+
 namespace quiche {
 
 namespace {
@@ -88,11 +103,11 @@
                                   const char* end, size_t* first_whitespace,
                                   size_t* first_nonwhite) {
   *first_whitespace = current - begin;
-  while (current < end && *current <= ' ') {
+  while (current < end && CHAR_LE(*current, ' ')) {
     ++current;
   }
   *first_nonwhite = current - begin;
-  while (current<end&& * current> ' ') {
+  while (current < end && CHAR_GT(*current, ' ')) {
     ++current;
   }
   return current;
@@ -151,14 +166,14 @@
 
   // Clean up any trailing whitespace that comes after the third island
   const char* last = end;
-  while (current <= last && *last <= ' ') {
+  while (current <= last && CHAR_LE(*last, ' ')) {
     --last;
   }
   headers->whitespace_4_idx_ = last - begin + 1;
 
   // Either the passed-in line is empty, or it starts with a non-whitespace
   // character.
-  QUICHE_DCHECK(begin == end || *begin > ' ');
+  QUICHE_DCHECK(begin == end || static_cast<unsigned char>(*begin) > ' ');
 
   QUICHE_DCHECK_EQ(0u, headers->whitespace_1_idx_);
   QUICHE_DCHECK_EQ(0u, headers->non_whitespace_1_idx_);
@@ -268,13 +283,13 @@
   QUICHE_DCHECK_LT(colon_loc, line_end);
   QUICHE_DCHECK_EQ(':', *colon_loc);
   QUICHE_DCHECK_EQ(':', *current);
-  QUICHE_DCHECK_GE(' ', *line_end)
+  QUICHE_DCHECK_CHAR_GE(' ', *line_end)
       << "\"" << std::string(line_begin, line_end) << "\"";
 
   // TODO(fenix): Investigate whether or not the bounds tests in the
   // while loops here are redundant, and if so, remove them.
   --current;
-  while (current > line_begin && *current <= ' ') {
+  while (current > line_begin && CHAR_LE(*current, ' ')) {
     --current;
   }
   current += static_cast<int>(current != colon_loc);
@@ -283,7 +298,7 @@
   current = colon_loc;
   QUICHE_DCHECK_EQ(':', *current);
   ++current;
-  while (current < line_end && *current <= ' ') {
+  while (current < line_end && CHAR_LE(*current, ' ')) {
     ++current;
   }
   current_header_line->value_begin_idx = current - stream_begin;
@@ -323,7 +338,7 @@
     // "\r\n", etc -within- the line (and not just at the end of it).
     for (++i; i < lines_size_m1; ++i) {
       const char c = *(stream_begin + lines[i].first);
-      if (c > ' ') {
+      if (CHAR_GT(c, ' ')) {
         // Not a continuation, so stop.  Note that if the 'original' i = 1,
         // and the next line is not a continuation, we'll end up with i = 2
         // when we break. This handles the incrementing of i for the outer
@@ -355,11 +370,11 @@
     --line_end;
     QUICHE_DCHECK_EQ('\n', *line_end)
         << "\"" << std::string(line_begin, line_end) << "\"";
-    while (*line_end <= ' ' && line_end > line_begin) {
+    while (CHAR_LE(*line_end, ' ') && line_end > line_begin) {
       --line_end;
     }
     ++line_end;
-    QUICHE_DCHECK_GE(' ', *line_end);
+    QUICHE_DCHECK_CHAR_GE(' ', *line_end);
     QUICHE_DCHECK_LT(line_begin, line_end);
 
     // We use '0' for the block idx, because we're always writing to the first
@@ -731,7 +746,7 @@
       do {
         const char c = *message_current;
         if (c != '\r' && c != '\n') {
-          if (c <= ' ') {
+          if (CHAR_LE(c, ' ')) {
             HandleError(BalsaFrameEnums::NO_REQUEST_LINE_IN_REQUEST);
             return message_current - original_message_start;
           }
@@ -1294,3 +1309,9 @@
 const int32_t BalsaFrame::kValidTerm2Mask;
 
 }  // namespace quiche
+
+#undef CHAR_LT
+#undef CHAR_LE
+#undef CHAR_GT
+#undef CHAR_GE
+#undef QUICHE_DCHECK_CHAR_GE

quiche/common/balsa/balsa_headers.cc

diff --git a/quiche/common/balsa/balsa_headers.cc b/quiche/common/balsa/balsa_headers.cc
index e024e1a..16c223b 100644
--- a/quiche/common/balsa/balsa_headers.cc
+++ b/quiche/common/balsa/balsa_headers.cc
@@ -182,8 +182,12 @@
   const char* start = header_value.begin();
   const char* end = header_value.end();
   while (true) {
+    // Cast `*start` to unsigned char to make values above 127 rank as expected
+    // on platforms with signed char, where such values are represented as
+    // negative numbers before the cast.
+
     // search for first nonwhitespace, non separator char.
-    while (*start == ',' || *start <= ' ') {
+    while (*start == ',' || static_cast<unsigned char>(*start) <= ' ') {
       ++start;
       if (start == end) {
         return;
@@ -193,7 +197,7 @@
     const char* nws = start;
 
     // search for next whitspace or separator char.
-    while (*start != ',' && *start > ' ') {
+    while (*start != ',' && static_cast<unsigned char>(*start) > ' ') {
       ++start;
       if (start == end) {
         if (nws != start) {