search

confluentinc / librdkafka

The Apache Kafka C/C++ library
Other
7.37k stars 3.11k forks source link

Handle overflow in rd_buf_write_remains #4689

Closed anchitj closed 2 weeks ago

anchitj commented 2 months ago

After the implementation of KIP-320, TopicCnt was sent as a varint in the metadata request. This change caused an overflow issue due to the use of rd_buf_erase to erase the remaining bytes. The overflow occurred in the calculation rbuf->rbuf_size - (rbuf->rbuf_len + rbuf->rbuf_erased) because rbuf->rbuf_erased was non-zero when the buffer was almost full, leading to an overflow condition.

As a result, for certain configurations that caused the buffer to be nearly full, the client was unable to send metadata requests, and it also caused crashes in the .NET client.

korchak-aleksandr commented 2 months ago

Hi @emasab,

Could you please review and approve this PR? It's critical for us as it addresses a blocking issue with the confluent-kafka-dotnet library update. Your approval is eagerly awaited.

emasab commented 1 month ago

There also a problem in rd_buf_write_seek because of rd_buf_destroy_segment.

My proposed solution is to add a seg_erased to rd_segment_s and increase it when part of the segment is erased, then subtract seg_erased from rbuf_erased when a segment is removed in rd_buf_write_seek

anchitj commented 1 month ago

There also a problem in rd_buf_write_seek because of rd_buf_destroy_segment.

My proposed solution is to add a seg_erased to rd_segment_s and increase it when part of the segment is erased, then subtract seg_erased from rbuf_erased when a segment is removed in rd_buf_write_seek

Makes sense. I'll create a separate PR to handle that.

emasab commented 1 month ago

@anchitj better to do it here, as it's basically the same thing that is missing