wasphin commented on code in PR #3658:
URL: https://github.com/apache/brpc/pull/3658#discussion_r4379163593
##########
src/brpc/policy/crc32c_checksum.cpp:
##########
@@ -61,12 +63,15 @@ void Crc32cCompute(const ChecksumIn& in) {
}
bool Crc32cVerify(const ChecksumIn& in) {
- uint32_t crc = ComputeCrc32c(in);
auto& val = ControllerPrivateAccessor(const_cast<Controller*>(in.cntl))
.checksum_value();
- CHECK_EQ(val.size(), sizeof(crc));
- auto expected = *reinterpret_cast<const uint32_t*>(val.data());
+ uint32_t expected;
+ if (val.size() != sizeof(expected)) {
+ return false;
+ }
+ memcpy(&expected, val.data(), sizeof(expected));
Review Comment:
Updated to std::memcpy in 0417de77, matching the <cstring> include.
##########
test/brpc_checksum_unittest.cpp:
##########
@@ -50,6 +50,18 @@ TEST_F(ChecksumAttachmentTest,
verify_succeeds_when_body_only) {
EXPECT_TRUE(brpc::policy::Crc32cVerify(verify_in));
}
+TEST_F(ChecksumAttachmentTest, verify_rejects_invalid_checksum_lengths) {
+ brpc::Controller cntl;
+ butil::IOBuf body;
+ body.append("request body");
+ brpc::ChecksumIn in{&body, &cntl, nullptr};
+ brpc::ControllerPrivateAccessor accessor(&cntl);
+ for (size_t size : {size_t(0), size_t(3), size_t(5)}) {
+ accessor.set_checksum_value(std::string(size, '\0'));
+ EXPECT_FALSE(brpc::policy::Crc32cVerify(in)) << "size=" << size;
+ }
+}
Review Comment:
The surrounding tests use four-space indentation, including
verify_succeeds_when_body_only and verify_succeeds_when_attachment_included.
The repository AGENTS.md also explicitly requires four-space indentation.
Keeping the existing indentation for consistency.
--
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]
