Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sanity checks for writing number in variable length format (resubmit) #48154

Merged
merged 5 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/en/development/build-cross-osx.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ This is intended for continuous integration checks that run on Linux servers. If

The cross-build for macOS is based on the [Build instructions](../development/build.md), follow them first.

## Install Clang-14
## Install Clang-15

Follow the instructions from https://apt.llvm.org/ for your Ubuntu or Debian setup.
For example the commands for Bionic are like:

``` bash
sudo echo "deb [trusted=yes] http://apt.llvm.org/bionic/ llvm-toolchain-bionic-14 main" >> /etc/apt/sources.list
sudo apt-get install clang-14
sudo echo "deb [trusted=yes] http://apt.llvm.org/bionic/ llvm-toolchain-bionic-15 main" >> /etc/apt/sources.list
sudo apt-get install clang-15
```

## Install Cross-Compilation Toolset {#install-cross-compilation-toolset}
Expand Down Expand Up @@ -55,7 +55,7 @@ curl -L 'https://github.com/phracker/MacOSX-SDKs/releases/download/10.15/MacOSX1
cd ClickHouse
mkdir build-darwin
cd build-darwin
CC=clang-14 CXX=clang++-14 cmake -DCMAKE_AR:FILEPATH=${CCTOOLS}/bin/x86_64-apple-darwin-ar -DCMAKE_INSTALL_NAME_TOOL=${CCTOOLS}/bin/x86_64-apple-darwin-install_name_tool -DCMAKE_RANLIB:FILEPATH=${CCTOOLS}/bin/x86_64-apple-darwin-ranlib -DLINKER_NAME=${CCTOOLS}/bin/x86_64-apple-darwin-ld -DCMAKE_TOOLCHAIN_FILE=cmake/darwin/toolchain-x86_64.cmake ..
CC=clang-15 CXX=clang++-15 cmake -DCMAKE_AR:FILEPATH=${CCTOOLS}/bin/x86_64-apple-darwin-ar -DCMAKE_INSTALL_NAME_TOOL=${CCTOOLS}/bin/x86_64-apple-darwin-install_name_tool -DCMAKE_RANLIB:FILEPATH=${CCTOOLS}/bin/x86_64-apple-darwin-ranlib -DLINKER_NAME=${CCTOOLS}/bin/x86_64-apple-darwin-ld -DCMAKE_TOOLCHAIN_FILE=cmake/darwin/toolchain-x86_64.cmake ..
ninja
```

Expand Down
10 changes: 5 additions & 5 deletions src/IO/Progress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ void ProgressValues::read(ReadBuffer & in, UInt64 server_revision)

void ProgressValues::write(WriteBuffer & out, UInt64 client_revision) const
{
writeVarUInt(read_rows, out);
writeVarUInt(read_bytes, out);
writeVarUInt(total_rows_to_read, out);
writeVarUIntOverflow(read_rows, out);
writeVarUIntOverflow(read_bytes, out);
writeVarUIntOverflow(total_rows_to_read, out);
if (client_revision >= DBMS_MIN_REVISION_WITH_CLIENT_WRITE_INFO)
{
writeVarUInt(written_rows, out);
writeVarUInt(written_bytes, out);
writeVarUIntOverflow(written_rows, out);
writeVarUIntOverflow(written_bytes, out);
}
if (client_revision >= DBMS_MIN_PROTOCOL_VERSION_WITH_SERVER_QUERY_TIME_IN_PROGRESS)
{
Expand Down
26 changes: 25 additions & 1 deletion src/IO/VarInt.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <iostream>
#include <base/types.h>
#include <base/defines.h>
#include <IO/ReadBuffer.h>
#include <IO/WriteBuffer.h>

Expand All @@ -14,12 +15,32 @@ namespace ErrorCodes
}


/** Write UInt64 in variable length format (base128) NOTE Only up to 2^63 - 1 are supported. */
/** Variable-Length Quantity (VLQ) Base-128 compression
*
* NOTE: Due to historical reasons, only up to 1<<63-1 are supported, which
* cannot be changed without breaking the backward compatibility.
* Also some drivers may support full 1<<64 range (i.e. python -
* clickhouse-driver), while others has the same limitations as ClickHouse
* (i.e. Rust - clickhouse-rs).
* So implementing VLQ for the whole 1<<64 range will require different set of
* helpers.
*/
constexpr UInt64 VAR_UINT_MAX = (1ULL<<63) - 1;

/** Write UInt64 in variable length format (base128) */
void writeVarUInt(UInt64 x, std::ostream & ostr);
void writeVarUInt(UInt64 x, WriteBuffer & ostr);
char * writeVarUInt(UInt64 x, char * ostr);


/** Write UInt64 in variable length format, wrap the value to VAR_UINT_MAX if it exceed VAR_UINT_MAX (to bypass sanity check) */
template <typename ...Args>
auto writeVarUIntOverflow(UInt64 x, Args && ... args)
{
return writeVarUInt(std::min(x, VAR_UINT_MAX), std::forward<Args>(args)...);
}


/** Read UInt64, written in variable length format (base128) */
void readVarUInt(UInt64 & x, std::istream & istr);
void readVarUInt(UInt64 & x, ReadBuffer & istr);
Expand Down Expand Up @@ -186,6 +207,7 @@ inline const char * readVarUInt(UInt64 & x, const char * istr, size_t size)

inline void writeVarUInt(UInt64 x, WriteBuffer & ostr)
{
chassert(x <= VAR_UINT_MAX);
for (size_t i = 0; i < 9; ++i)
{
uint8_t byte = x & 0x7F;
Expand All @@ -205,6 +227,7 @@ inline void writeVarUInt(UInt64 x, WriteBuffer & ostr)

inline void writeVarUInt(UInt64 x, std::ostream & ostr)
{
chassert(x <= VAR_UINT_MAX);
for (size_t i = 0; i < 9; ++i)
{
uint8_t byte = x & 0x7F;
Expand All @@ -222,6 +245,7 @@ inline void writeVarUInt(UInt64 x, std::ostream & ostr)

inline char * writeVarUInt(UInt64 x, char * ostr)
{
chassert(x <= VAR_UINT_MAX);
for (size_t i = 0; i < 9; ++i)
{
uint8_t byte = x & 0x7F;
Expand Down
14 changes: 10 additions & 4 deletions src/IO/examples/var_uint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,32 @@ int main(int argc, char ** argv)
}

DB::UInt64 x = DB::parse<UInt64>(argv[1]);

std::cout << std::hex << std::showbase << "Input: " << x << std::endl;

Poco::HexBinaryEncoder hex(std::cout);
DB::writeVarUInt(x, hex);
std::cout << "writeVarUIntOverflow(std::ostream): 0x";
DB::writeVarUIntOverflow(x, hex);
std::cout << std::endl;

std::string s;

{
DB::WriteBufferFromString wb(s);
DB::writeVarUInt(x, wb);
DB::writeVarUIntOverflow(x, wb);
wb.next();
}

std::cout << "writeVarUIntOverflow(WriteBuffer): 0x";
hex << s;
std::cout << std::endl;

s.clear();
s.resize(9);

s.resize(DB::writeVarUInt(x, s.data()) - s.data());
s.resize(DB::writeVarUIntOverflow(x, s.data()) - s.data());

std::cout << "writeVarUIntOverflow(char *): 0x";
hex << s;
std::cout << std::endl;

Expand All @@ -46,7 +52,7 @@ int main(int argc, char ** argv)
DB::ReadBufferFromString rb(s);
DB::readVarUInt(y, rb);

std::cerr << "x: " << x << ", y: " << y << std::endl;
std::cerr << "Input: " << x << ", readVarUInt(writeVarUIntOverflow()): " << y << std::endl;

return 0;
}
2 changes: 1 addition & 1 deletion src/Server/TCPHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1895,7 +1895,7 @@ void TCPHandler::sendData(const Block & block)
{
--unknown_packet_in_send_data;
if (unknown_packet_in_send_data == 0)
writeVarUInt(UInt64(-1), *out);
writeVarUInt(VAR_UINT_MAX, *out);
}

writeVarUInt(Protocol::Server::Data, *out);
Expand Down