thrift: save one comparison of thrift compact protocol (#34736)

Risk Level: low Testing: existing tests Docs Changes: n/a Release Notes: n/a Platform Specific Features: n/a Signed-off-by: kuochunghsu <[email protected]>
envoyproxy · Jun 28, 2024 · 183e8b4 · 183e8b4
1 parent aef38c4
commit 183e8b4
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 16 deletions.
diff --git a/source/extensions/filters/network/thrift_proxy/compact_protocol_impl.cc b/source/extensions/filters/network/thrift_proxy/compact_protocol_impl.cc
@@ -112,24 +112,15 @@ bool CompactProtocolImpl::peekReplyPayload(Buffer::Instance& buffer, ReplyType&
     return false;
   }
 
-  validateFieldId(id);
+  if (id < std::numeric_limits<int16_t>::min() || id > std::numeric_limits<int16_t>::max()) {
+    throw EnvoyException(absl::StrCat("invalid compact protocol field id ", id));
+  }
+
   // successful response struct in field id 0, error (IDL exception) in field id greater than 0
   reply_type = id == 0 ? ReplyType::Success : ReplyType::Error;
   return true;
 }
 
-void CompactProtocolImpl::validateFieldId(int32_t id) {
-  if (id >= 0 && id <= std::numeric_limits<int16_t>::max()) {
-    return;
-  }
-
-  if (id < 0 && id >= std::numeric_limits<int16_t>::min()) {
-    return;
-  }
-
-  throw EnvoyException(absl::StrCat("invalid compact protocol field id ", id));
-}
-
 bool CompactProtocolImpl::readStructBegin(Buffer::Instance& buffer, std::string& name) {
   UNREFERENCED_PARAMETER(buffer);
   name.clear(); // compact protocol does not transmit struct names
@@ -187,7 +178,10 @@ bool CompactProtocolImpl::readFieldBegin(Buffer::Instance& buffer, std::string&
       return false;
     }
 
-    validateFieldId(id);
+    if (id < std::numeric_limits<int16_t>::min() || id > std::numeric_limits<int16_t>::max()) {
+      throw EnvoyException(absl::StrCat("invalid compact protocol field id ", id));
+    }
+
     compact_field_type = static_cast<CompactFieldType>(delta_and_type);
     compact_field_id = static_cast<int16_t>(id);
   } else {

diff --git a/source/extensions/filters/network/thrift_proxy/compact_protocol_impl.h b/source/extensions/filters/network/thrift_proxy/compact_protocol_impl.h
@@ -98,8 +98,6 @@ class CompactProtocolImpl : public Protocol {
   void writeFieldBeginInternal(Buffer::Instance& buffer, FieldType field_type, int16_t field_id,
                                absl::optional<CompactFieldType> field_type_override);
 
-  static void validateFieldId(int32_t id);
-
   std::stack<int16_t> last_field_id_stack_{};
   int16_t last_field_id_{0};