Copilot commented on code in PR #3676: URL: https://github.com/apache/brpc/pull/3676#discussion_r4548090100
########## test/fuzzing/gen_thrift_seed_corpus.py: ########## @@ -0,0 +1,172 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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. +"""Generate the seed corpus for fuzz_thrift. + +Every seed is synthesised here from readable source -- there is no captured +traffic, so the corpus is kept as this script rather than as binary files. +oss-fuzz.sh runs it before packaging the corpus; to regenerate the seeds by +hand: + + python3 gen_thrift_seed_corpus.py [output-dir] +""" + +import os +import struct +import sys + +# Thrift message types (TMessageType in thrift/protocol/TBinaryProtocol.h). +CALL = 1 +REPLY = 2 +EXCEPTION = 3 +ONEWAY = 4 + +# Thrift field types (TType in thrift/protocol/TBinaryProtocol.h). +T_STOP = 0x00 +T_I16 = 0x06 +T_I32 = 0x08 +T_I64 = 0x0A +T_STRING = 0x0B +T_STRUCT = 0x0C +T_MAP = 0x0D +T_LIST = 0x0F + +# Every struct -- including the top-level one that holds the arguments or +# results -- ends with this single stop-marker byte. +STOP = bytes([T_STOP]) + +# Strict binary-protocol version word: THRIFT_HEAD_VERSION_1 | message type. +# ParseThriftMessage() in src/brpc/policy/thrift_protocol.cpp rejects a frame +# unless (word & 0xffffff00) == 0x80010000, with the low byte as the type. +VERSION_1 = 0x8001 + + +# ---------------------------------------------- thrift binary-protocol pieces +def field(ftype, fid, value): + """One field: type byte, field id, then the encoded value.""" + return bytes([ftype]) + struct.pack(">h", fid) + value + + +def i16(value): + return struct.pack(">h", value) + + +def i32(value): + return struct.pack(">i", value) + + +def i64(value): + return struct.pack(">q", value) + + +def tstr(text): + encoded = text.encode() + return struct.pack(">I", len(encoded)) + encoded + + +def tlist(elem_type, elements): + return (bytes([elem_type]) + struct.pack(">I", len(elements)) + + b"".join(elements)) + + +def tmap(key_type, value_type, pairs): + return (bytes([key_type, value_type]) + struct.pack(">I", len(pairs)) Review Comment: Thrift binary protocol and framed transport encode string length, sequence id, and frame size as i32 on the wire. Using unsigned packing (`>I`) produces identical bytes for small values, but it diverges from the spec and can be misleading if these helpers are reused/extended (e.g., negative seq_id, high-bit lengths). Prefer signed packing (`>i`) for the i32 fields to match the Thrift wire format. ########## test/fuzzing/gen_thrift_seed_corpus.py: ########## @@ -0,0 +1,172 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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. +"""Generate the seed corpus for fuzz_thrift. + +Every seed is synthesised here from readable source -- there is no captured +traffic, so the corpus is kept as this script rather than as binary files. +oss-fuzz.sh runs it before packaging the corpus; to regenerate the seeds by +hand: + + python3 gen_thrift_seed_corpus.py [output-dir] +""" + +import os +import struct +import sys + +# Thrift message types (TMessageType in thrift/protocol/TBinaryProtocol.h). +CALL = 1 +REPLY = 2 +EXCEPTION = 3 +ONEWAY = 4 + +# Thrift field types (TType in thrift/protocol/TBinaryProtocol.h). +T_STOP = 0x00 +T_I16 = 0x06 +T_I32 = 0x08 +T_I64 = 0x0A +T_STRING = 0x0B +T_STRUCT = 0x0C +T_MAP = 0x0D +T_LIST = 0x0F + +# Every struct -- including the top-level one that holds the arguments or +# results -- ends with this single stop-marker byte. +STOP = bytes([T_STOP]) + +# Strict binary-protocol version word: THRIFT_HEAD_VERSION_1 | message type. +# ParseThriftMessage() in src/brpc/policy/thrift_protocol.cpp rejects a frame +# unless (word & 0xffffff00) == 0x80010000, with the low byte as the type. +VERSION_1 = 0x8001 + + +# ---------------------------------------------- thrift binary-protocol pieces +def field(ftype, fid, value): + """One field: type byte, field id, then the encoded value.""" + return bytes([ftype]) + struct.pack(">h", fid) + value + + +def i16(value): + return struct.pack(">h", value) + + +def i32(value): + return struct.pack(">i", value) + + +def i64(value): + return struct.pack(">q", value) + + +def tstr(text): + encoded = text.encode() + return struct.pack(">I", len(encoded)) + encoded + + +def tlist(elem_type, elements): + return (bytes([elem_type]) + struct.pack(">I", len(elements)) + + b"".join(elements)) + + +def tmap(key_type, value_type, pairs): + return (bytes([key_type, value_type]) + struct.pack(">I", len(pairs)) + + b"".join(key + value for key, value in pairs)) + + +def tstruct(*fields): + """A struct body: zero or more fields closed by a stop marker.""" + return b"".join(fields) + STOP + + +def message_begin(name, mtype, seq_id): + """Version word + method name + sequence id.""" + return (struct.pack(">I", (VERSION_1 << 16) | (mtype & 0xFF)) + + tstr(name) + struct.pack(">I", seq_id)) + + +def frame(payload): + """Wrap a thrift message in thrift framed's 4-byte size prefix.""" + return struct.pack(">I", len(payload)) + payload Review Comment: Thrift binary protocol and framed transport encode string length, sequence id, and frame size as i32 on the wire. Using unsigned packing (`>I`) produces identical bytes for small values, but it diverges from the spec and can be misleading if these helpers are reused/extended (e.g., negative seq_id, high-bit lengths). Prefer signed packing (`>i`) for the i32 fields to match the Thrift wire format. -- 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]
