From 54db7c6520f72529f4624862ee8bdab719c64ed9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Fri, 11 Nov 2022 10:56:18 +0100 Subject: [PATCH] Enforce checking read output --- src/Common/ZooKeeper/ZooKeeperIO.cpp | 5 ++++- src/Common/ZooKeeper/ZooKeeperIO.h | 2 +- src/Compression/CompressedReadBuffer.h | 2 +- src/Compression/CompressedReadBufferFromFile.h | 2 +- src/Coordination/KeeperStateManager.cpp | 2 +- src/Core/PostgreSQLProtocol.h | 2 +- src/DataTypes/Serializations/SerializationIP.cpp | 4 ++-- src/IO/MySQLPacketPayloadReadBuffer.cpp | 2 +- src/IO/ReadBuffer.h | 9 +++------ src/Interpreters/TraceCollector.cpp | 2 +- src/Storages/MergeTree/MergeTreeIndexAnnoy.cpp | 2 +- src/Storages/MergeTree/MergeTreeIndexFullText.cpp | 3 +-- .../MergeTree/MergeTreeIndexGranuleBloomFilter.cpp | 2 +- 13 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/Common/ZooKeeper/ZooKeeperIO.cpp b/src/Common/ZooKeeper/ZooKeeperIO.cpp index c84a8624d78..3bfa5585d87 100644 --- a/src/Common/ZooKeeper/ZooKeeperIO.cpp +++ b/src/Common/ZooKeeper/ZooKeeperIO.cpp @@ -143,7 +143,10 @@ void read(std::string & s, ReadBuffer & in) throw Exception("Too large string size while reading from ZooKeeper", Error::ZMARSHALLINGERROR); s.resize(size); - in.read(s.data(), size); + size_t read_bytes = in.read(s.data(), size); + if (read_bytes != static_cast(size)) + throw Exception( + Error::ZMARSHALLINGERROR, "Buffer size read from Zookeeper is not big enough. Expected {}. Got {}", size, read_bytes); } void read(ACL & acl, ReadBuffer & in) diff --git a/src/Common/ZooKeeper/ZooKeeperIO.h b/src/Common/ZooKeeper/ZooKeeperIO.h index ec77b46f3d9..2c5fdd5d8a3 100644 --- a/src/Common/ZooKeeper/ZooKeeperIO.h +++ b/src/Common/ZooKeeper/ZooKeeperIO.h @@ -67,7 +67,7 @@ void read(std::array & s, ReadBuffer & in) read(size, in); if (size != N) throw Exception("Unexpected array size while reading from ZooKeeper", Error::ZMARSHALLINGERROR); - in.read(s.data(), N); + in.readStrict(s.data(), N); } template diff --git a/src/Compression/CompressedReadBuffer.h b/src/Compression/CompressedReadBuffer.h index 4148f4fe4d4..1d338303c84 100644 --- a/src/Compression/CompressedReadBuffer.h +++ b/src/Compression/CompressedReadBuffer.h @@ -21,7 +21,7 @@ public: { } - size_t readBig(char * to, size_t n) override; + [[nodiscard]] size_t readBig(char * to, size_t n) override; /// The compressed size of the current block. size_t getSizeCompressed() const diff --git a/src/Compression/CompressedReadBufferFromFile.h b/src/Compression/CompressedReadBufferFromFile.h index 719959b96f4..d307503fb99 100644 --- a/src/Compression/CompressedReadBufferFromFile.h +++ b/src/Compression/CompressedReadBufferFromFile.h @@ -53,7 +53,7 @@ public: /// we store this offset inside nextimpl_working_buffer_offset. void seek(size_t offset_in_compressed_file, size_t offset_in_decompressed_block) override; - size_t readBig(char * to, size_t n) override; + [[nodiscard]] size_t readBig(char * to, size_t n) override; void setProfileCallback(const ReadBufferFromFileBase::ProfileCallback & profile_callback_, clockid_t clock_type_ = CLOCK_MONOTONIC_COARSE) { diff --git a/src/Coordination/KeeperStateManager.cpp b/src/Coordination/KeeperStateManager.cpp index 9b6aab5533e..9a3b423d4ac 100644 --- a/src/Coordination/KeeperStateManager.cpp +++ b/src/Coordination/KeeperStateManager.cpp @@ -349,7 +349,7 @@ nuraft::ptr KeeperStateManager::read_state() auto buffer_size = content_size - sizeof read_checksum - sizeof version; auto state_buf = nuraft::buffer::alloc(buffer_size); - read_buf.read(reinterpret_cast(state_buf->data_begin()), buffer_size); + read_buf.readStrict(reinterpret_cast(state_buf->data_begin()), buffer_size); SipHash hash; hash.update(version); diff --git a/src/Core/PostgreSQLProtocol.h b/src/Core/PostgreSQLProtocol.h index 994494fc92f..a20151ec167 100644 --- a/src/Core/PostgreSQLProtocol.h +++ b/src/Core/PostgreSQLProtocol.h @@ -175,7 +175,7 @@ public: FrontMessageType receiveMessageType() { char type = 0; - in->read(type); + in->readStrict(type); return static_cast(type); } diff --git a/src/DataTypes/Serializations/SerializationIP.cpp b/src/DataTypes/Serializations/SerializationIP.cpp index ed0e9d54415..2aea08f9b62 100644 --- a/src/DataTypes/Serializations/SerializationIP.cpp +++ b/src/DataTypes/Serializations/SerializationIP.cpp @@ -47,7 +47,7 @@ void SerializationIPv4::deserializeText(IColumn & column, ReadBuffer & istr, con } char buffer[IPV4_MAX_TEXT_LENGTH + 1] = {'\0'}; - istr.read(buffer, sizeof(buffer) - 1); + istr.readStrict(buffer, sizeof(buffer) - 1); UInt32 ipv4_value = 0; bool parse_result = parseIPv4(buffer, reinterpret_cast(&ipv4_value)); @@ -90,7 +90,7 @@ void SerializationIPv6::deserializeText(IColumn & column, ReadBuffer & istr, con } char buffer[IPV6_MAX_TEXT_LENGTH + 1] = {'\0'}; - istr.read(buffer, sizeof(buffer) - 1); + [[maybe_unused]] size_t read_bytes = istr.read(buffer, sizeof(buffer) - 1); std::string ipv6_value(IPV6_BINARY_LENGTH, '\0'); diff --git a/src/IO/MySQLPacketPayloadReadBuffer.cpp b/src/IO/MySQLPacketPayloadReadBuffer.cpp index 9ca7845b2ae..ab58624d0fa 100644 --- a/src/IO/MySQLPacketPayloadReadBuffer.cpp +++ b/src/IO/MySQLPacketPayloadReadBuffer.cpp @@ -30,7 +30,7 @@ bool MySQLPacketPayloadReadBuffer::nextImpl() "Received packet with payload larger than max_packet_size: {}", payload_length); size_t packet_sequence_id = 0; - in.read(reinterpret_cast(packet_sequence_id)); + in.readStrict(reinterpret_cast(packet_sequence_id)); if (packet_sequence_id != sequence_id) throw Exception(ErrorCodes::UNKNOWN_PACKET_FROM_CLIENT, "Received packet with wrong sequence-id: {}. Expected: {}.", packet_sequence_id, static_cast(sequence_id)); diff --git a/src/IO/ReadBuffer.h b/src/IO/ReadBuffer.h index 8d697710081..182eb0b7105 100644 --- a/src/IO/ReadBuffer.h +++ b/src/IO/ReadBuffer.h @@ -149,7 +149,7 @@ public: } /// Reads a single byte. - bool ALWAYS_INLINE read(char & c) + [[nodiscard]] bool ALWAYS_INLINE read(char & c) { if (peek(c)) { @@ -168,7 +168,7 @@ public: } /** Reads as many as there are, no more than n bytes. */ - size_t read(char * to, size_t n) + [[nodiscard]] size_t read(char * to, size_t n) { size_t bytes_copied = 0; @@ -197,10 +197,7 @@ public: * By default - the same as read. * Don't use for small reads. */ - virtual size_t readBig(char * to, size_t n) - { - return read(to, n); - } + [[nodiscard]] virtual size_t readBig(char * to, size_t n) { return read(to, n); } /** Do something to allow faster subsequent call to 'nextImpl' if possible. * It's used for asynchronous readers with double-buffering. diff --git a/src/Interpreters/TraceCollector.cpp b/src/Interpreters/TraceCollector.cpp index d277763a141..41a7fcf8389 100644 --- a/src/Interpreters/TraceCollector.cpp +++ b/src/Interpreters/TraceCollector.cpp @@ -72,7 +72,7 @@ void TraceCollector::run() UInt8 query_id_size = 0; readBinary(query_id_size, in); query_id.resize(query_id_size); - in.read(query_id.data(), query_id_size); + in.readStrict(query_id.data(), query_id_size); UInt8 trace_size = 0; readIntBinary(trace_size, in); diff --git a/src/Storages/MergeTree/MergeTreeIndexAnnoy.cpp b/src/Storages/MergeTree/MergeTreeIndexAnnoy.cpp index 052834358bb..743bb504dbd 100644 --- a/src/Storages/MergeTree/MergeTreeIndexAnnoy.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexAnnoy.cpp @@ -44,7 +44,7 @@ void AnnoyIndex::deserialize(ReadBuffer& istr) readIntBinary(Base::_seed, istr); readVectorBinary(Base::_roots, istr); Base::_nodes = realloc(Base::_nodes, Base::_s * Base::_n_nodes); - istr.read(reinterpret_cast(Base::_nodes), Base::_s * Base::_n_nodes); + istr.readStrict(reinterpret_cast(Base::_nodes), Base::_s * Base::_n_nodes); Base::_fd = 0; // set flags diff --git a/src/Storages/MergeTree/MergeTreeIndexFullText.cpp b/src/Storages/MergeTree/MergeTreeIndexFullText.cpp index b96d40f5759..03335d9ca98 100644 --- a/src/Storages/MergeTree/MergeTreeIndexFullText.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexFullText.cpp @@ -59,8 +59,7 @@ void MergeTreeIndexGranuleFullText::deserializeBinary(ReadBuffer & istr, MergeTr for (auto & bloom_filter : bloom_filters) { - istr.read(reinterpret_cast( - bloom_filter.getFilter().data()), params.filter_size); + istr.readStrict(reinterpret_cast(bloom_filter.getFilter().data()), params.filter_size); } has_elems = true; } diff --git a/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.cpp b/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.cpp index 7efaf0866db..deed9b3f071 100644 --- a/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.cpp @@ -96,7 +96,7 @@ void MergeTreeIndexGranuleBloomFilter::deserializeBinary(ReadBuffer & istr, Merg static size_t atom_size = 8; size_t bytes_size = (bits_per_row * total_rows + atom_size - 1) / atom_size; filter = std::make_shared(bytes_size, hash_functions, 0); - istr.read(reinterpret_cast(filter->getFilter().data()), bytes_size); + istr.readStrict(reinterpret_cast(filter->getFilter().data()), bytes_size); } }