dxbjavid commented on PR #3610: URL: https://github.com/apache/thrift/pull/3610#issuecomment-4856113499
all three addressed in the latest push. 1. reworded the test comment to describe the invariant (the key length has to fit within the header bytes that remain after the length varint) rather than the failure mode. did the same for the commit body. 2. added test_theadertransport_info_header_string_negative_length. it feeds a 5-byte key-length varint with the top bit set (0x80 0x80 0x80 0x80 0x08, decoding to INT32_MIN) and asserts a TTransportException, where the old code would let it reach assign and raise length_error. 3. fair point on the contract. rather than loosen the doc comment i kept the advance-on-success behaviour instead: the bounds check now runs against a local pointer past the varint and ptr is only moved once the checks pass, so a caller that catches the exception still sees ptr unchanged. also read your email, thanks. for future findings in transport or protocol code where a remote peer influences parsing i'll go through [email protected] first rather than opening a public PR. -- 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]
