From 0fd32f8b9d707d807c236156de12118b0a695d69 Mon Sep 17 00:00:00 2001 From: Paul Cristian Sarbu Date: Tue, 14 May 2024 16:48:20 +0200 Subject: logging: Do not make assumptions in emit calls The Emit method of the Logger class, when called with a string as second argument, expects it to be a format string. It should be considered a programming error to pass a string variable as that argument without knowing for certain that it does not contain any format escape character ('{', '}'); instead, one should be conservative and use the blind format string "{}" as second argument and pass the unknown string variable as third argument. --- .../execution_service/bytestream_server.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'src/buildtool/execution_api/execution_service/bytestream_server.cpp') diff --git a/src/buildtool/execution_api/execution_service/bytestream_server.cpp b/src/buildtool/execution_api/execution_service/bytestream_server.cpp index 3433f031..2a4b7e21 100644 --- a/src/buildtool/execution_api/execution_service/bytestream_server.cpp +++ b/src/buildtool/execution_api/execution_service/bytestream_server.cpp @@ -52,12 +52,12 @@ auto BytestreamServiceImpl::Read( auto hash = ParseResourceName(request->resource_name()); if (!hash) { auto str = fmt::format("could not parse {}", request->resource_name()); - logger_.Emit(LogLevel::Error, str); + logger_.Emit(LogLevel::Error, "{}", str); return ::grpc::Status{::grpc::StatusCode::INVALID_ARGUMENT, str}; } if (auto error_msg = IsAHash(*hash); error_msg) { - logger_.Emit(LogLevel::Debug, *error_msg); + logger_.Emit(LogLevel::Debug, "{}", *error_msg); return ::grpc::Status{::grpc::StatusCode::INVALID_ARGUMENT, *error_msg}; } @@ -81,7 +81,7 @@ auto BytestreamServiceImpl::Read( } if (!path) { auto str = fmt::format("could not find {}", *hash); - logger_.Emit(LogLevel::Error, str); + logger_.Emit(LogLevel::Error, "{}", str); return ::grpc::Status{::grpc::StatusCode::NOT_FOUND, str}; } std::ifstream blob{*path}; @@ -113,11 +113,11 @@ auto BytestreamServiceImpl::Write( auto hash = ParseResourceName(request.resource_name()); if (!hash) { auto str = fmt::format("could not parse {}", request.resource_name()); - logger_.Emit(LogLevel::Error, str); + logger_.Emit(LogLevel::Error, "{}", str); return ::grpc::Status{::grpc::StatusCode::INVALID_ARGUMENT, str}; } if (auto error_msg = IsAHash(*hash); error_msg) { - logger_.Emit(LogLevel::Debug, *error_msg); + logger_.Emit(LogLevel::Debug, "{}", *error_msg); return ::grpc::Status{::grpc::StatusCode::INVALID_ARGUMENT, *error_msg}; } logger_.Emit(LogLevel::Trace, @@ -150,7 +150,7 @@ auto BytestreamServiceImpl::Write( if (NativeSupport::IsTree(*hash)) { if (not storage_->CAS().StoreTree(tmp)) { auto str = fmt::format("could not store tree {}", *hash); - logger_.Emit(LogLevel::Error, str); + logger_.Emit(LogLevel::Error, "{}", str); return ::grpc::Status{::grpc::StatusCode::INVALID_ARGUMENT, str}; } } @@ -158,7 +158,7 @@ auto BytestreamServiceImpl::Write( if (not storage_->CAS().StoreBlob( tmp, /*is_executable=*/false)) { auto str = fmt::format("could not store blob {}", *hash); - logger_.Emit(LogLevel::Error, str); + logger_.Emit(LogLevel::Error, "{}", str); return ::grpc::Status{::grpc::StatusCode::INVALID_ARGUMENT, str}; } } @@ -173,6 +173,6 @@ auto BytestreamServiceImpl::QueryWriteStatus( ::google::bytestream::QueryWriteStatusResponse* /*response*/) -> ::grpc::Status { auto const* str = "QueryWriteStatus not implemented"; - logger_.Emit(LogLevel::Error, str); + logger_.Emit(LogLevel::Error, "{}", str); return ::grpc::Status{grpc::StatusCode::UNIMPLEMENTED, str}; } -- cgit v1.2.3