Copilot commented on code in PR #3524:
URL: https://github.com/apache/brpc/pull/3524#discussion_r3943108587


##########
test/brpc_http_rpc_protocol_unittest.cpp:
##########
@@ -1940,6 +1942,81 @@ TEST_F(HttpTest, h2_header_list_budget_resets_per_block) 
{
     delete sctx;
 }
 
+// Literal header field without indexing, new name (RFC 7541 6.2.2), with both
+// lengths in a single 7-bit prefix octet. Only used with short names/values.
+void AppendLiteralHeader(butil::IOBuf* out, const std::string& name,
+                         const std::string& value) {
+    char prefix[] = { 0x00, (char)name.size() };
+    out->append(prefix, sizeof(prefix));
+    out->append(name);
+    char value_len = (char)value.size();
+    out->append(&value_len, 1);
+    out->append(value);
+}

Review Comment:
   AppendLiteralHeader encodes HPACK string lengths into a single octet; using 
plain `char` and unchecked casts can silently truncate lengths >= 128 and (on 
signed-char platforms) set the Huffman bit, producing invalid header blocks. 
Add explicit bounds checks and use unsigned byte types to make the helper 
robust if reused/extended.



##########
src/brpc/uri.h:
##########
@@ -99,8 +99,9 @@ class URI {
     void set_port(int port) { _port = port; }
     void SetHostAndPort(const std::string& host_and_optional_port);
     // Set path/query/fragment with the input in form of "path?query#fragment"
-    void SetH2Path(const char* h2_path);
-    void SetH2Path(const std::string& path) { SetH2Path(path.c_str()); }
+    // Returns 0 on success, -1 otherwise and status() is set.
+    int SetH2Path(const char* h2_path);
+    int SetH2Path(const std::string& path) { return SetH2Path(path.c_str()); }

Review Comment:
   Changing URI::SetH2Path from returning void to returning int is a source/ABI 
breaking API change for downstream users (mangled name changes even if callers 
ignore the return). Consider keeping the existing void signature and indicating 
failure via status() (callers check status().ok()), or introduce a new 
differently-named checked method while preserving the old SetH2Path API.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to