From 4ec9ff3a59c362c113598c32cef1d35ec83c33e2 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Mon, 23 Oct 2023 14:17:52 -0700 Subject: [PATCH 01/16] Remove EnvoyException on ZK proxy filter data plane Signed-off-by: Zhewei Hu --- .../network/zookeeper_proxy/decoder.cc | 1164 +++++++++++++---- .../filters/network/zookeeper_proxy/decoder.h | 84 +- .../filters/network/zookeeper_proxy/filter.cc | 26 +- .../filters/network/zookeeper_proxy/filter.h | 10 +- .../filters/network/zookeeper_proxy/utils.cc | 53 +- .../filters/network/zookeeper_proxy/utils.h | 12 +- 6 files changed, 1016 insertions(+), 333 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index 968dc798b321..cf0d7dfd3028 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -42,15 +42,32 @@ const char* createFlagsToString(CreateFlags flags) { return "unknown"; } -absl::optional DecoderImpl::decodeOnData(Buffer::Instance& data, uint64_t& offset) { - ENVOY_LOG(trace, "zookeeper_proxy: decoding request with {} bytes at offset {}", data.length(), +absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instance& data, + uint64_t& offset) { + ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with {} bytes at offset {}", data.length(), offset); // Check message length. - const int32_t len = helper_.peekInt32(data, offset); - ENVOY_LOG(trace, "zookeeper_proxy: decoding request with len {} at offset {}", len, offset); - ensureMinLength(len, XID_LENGTH + INT_LENGTH); // xid + opcode - ensureMaxLength(len); + const absl::StatusOr len = helper_.peekInt32(data, offset); + if (!len.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(fmt::format("peekInt32 for len: {}", len.status().message())); + } + + ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with len {} at offset {}", len.value(), + offset); + + absl::Status status = ensureMinLength(len.value(), XID_LENGTH + INT_LENGTH); // xid + opcode + if (!status.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(fmt::format("ensureMinLength: {}", status.message())); + } + + status = ensureMaxLength(len.value()); + if (!status.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(fmt::format("ensureMaxLength: {}", status.message())); + } auto start_time = time_source_.monotonicTime(); @@ -64,26 +81,46 @@ absl::optional DecoderImpl::decodeOnData(Buffer::Instance& data, uint64 // ZooKeeper server to the next. Thus, the special xid. // However, some client implementations might expose setWatches // as a regular data request, so we support that as well. - const int32_t xid = helper_.peekInt32(data, offset); - ENVOY_LOG(trace, "zookeeper_proxy: decoding request with xid {} at offset {}", xid, offset); - switch (static_cast(xid)) { + const absl::StatusOr xid = helper_.peekInt32(data, offset); + if (!xid.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(fmt::format("peerInt32 for xid: {}", xid.status().message())); + } + + ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with xid {} at offset {}", xid.value(), + offset); + + switch (static_cast(xid.value())) { case XidCodes::ConnectXid: - parseConnect(data, offset, len); - control_requests_by_xid_[xid].push({OpCodes::Connect, std::move(start_time)}); + status = parseConnect(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseConnect: {}", status.message())); + } + + control_requests_by_xid_[xid.value()].push({OpCodes::Connect, std::move(start_time)}); return OpCodes::Connect; case XidCodes::PingXid: offset += OPCODE_LENGTH; callbacks_.onPing(); - control_requests_by_xid_[xid].push({OpCodes::Ping, std::move(start_time)}); + control_requests_by_xid_[xid.value()].push({OpCodes::Ping, std::move(start_time)}); return OpCodes::Ping; case XidCodes::AuthXid: - parseAuthRequest(data, offset, len); - control_requests_by_xid_[xid].push({OpCodes::SetAuth, std::move(start_time)}); + status = parseAuthRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseAuthRequest: {}", status.message())); + } + + control_requests_by_xid_[xid.value()].push({OpCodes::SetAuth, std::move(start_time)}); return OpCodes::SetAuth; case XidCodes::SetWatchesXid: offset += OPCODE_LENGTH; - parseSetWatchesRequest(data, offset, len); - control_requests_by_xid_[xid].push({OpCodes::SetWatches, std::move(start_time)}); + status = parseSetWatchesRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError( + fmt::format("parseSetWatchesRequest: {}", status.message())); + } + + control_requests_by_xid_[xid.value()].push({OpCodes::SetWatches, std::move(start_time)}); return OpCodes::SetWatches; default: // WATCH_XID is generated by the server, so that and everything @@ -97,100 +134,210 @@ absl::optional DecoderImpl::decodeOnData(Buffer::Instance& data, uint64 // for two cases: auth requests can happen at any time and ping requests // must happen every 1/3 of the negotiated session timeout, to keep // the session alive. - const int32_t oc = helper_.peekInt32(data, offset); - ENVOY_LOG(trace, "zookeeper_proxy: decoding request with opcode {} at offset {}", oc, offset); - const auto opcode = static_cast(oc); + const absl::StatusOr oc = helper_.peekInt32(data, offset); + if (!oc.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError( + fmt::format("peekInt32 for opcode: {}", oc.status().message())); + } + + ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with opcode {} at offset {}", oc.value(), + offset); + + const auto opcode = static_cast(oc.value()); switch (opcode) { case OpCodes::GetData: - parseGetDataRequest(data, offset, len); + status = parseGetDataRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseGetDataRequest: {}", status.message())); + } + break; case OpCodes::Create: case OpCodes::Create2: case OpCodes::CreateContainer: case OpCodes::CreateTtl: - parseCreateRequest(data, offset, len, static_cast(opcode)); + status = parseCreateRequest(data, offset, len.value(), static_cast(opcode)); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseCreateRequest: {}", status.message())); + } + break; case OpCodes::SetData: - parseSetRequest(data, offset, len); + status = parseSetRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseSetRequest: {}", status.message())); + } + break; case OpCodes::GetChildren: - parseGetChildrenRequest(data, offset, len, false); + status = parseGetChildrenRequest(data, offset, len.value(), false); + if (!status.ok()) { + return absl::InvalidArgumentError( + fmt::format("parseGetChildrenRequest: {}", status.message())); + } + break; case OpCodes::GetChildren2: - parseGetChildrenRequest(data, offset, len, true); + status = parseGetChildrenRequest(data, offset, len.value(), true); + if (!status.ok()) { + return absl::InvalidArgumentError( + fmt::format("parseGetChildrenRequest: {}", status.message())); + } + break; case OpCodes::Delete: - parseDeleteRequest(data, offset, len); + status = parseDeleteRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseDeleteRequest: {}", status.message())); + } + break; case OpCodes::Exists: - parseExistsRequest(data, offset, len); + status = parseExistsRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseExistsRequest: {}", status.message())); + } + break; case OpCodes::GetAcl: - parseGetAclRequest(data, offset, len); + status = parseGetAclRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseGetAclRequest: {}", status.message())); + } + break; case OpCodes::SetAcl: - parseSetAclRequest(data, offset, len); + status = parseSetAclRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseSetAclRequest: {}", status.message())); + } + break; case OpCodes::Sync: - callbacks_.onSyncRequest(pathOnlyRequest(data, offset, len)); + callbacks_.onSyncRequest(pathOnlyRequest(data, offset, len.value())); break; case OpCodes::Check: - parseCheckRequest(data, offset, len); + status = parseCheckRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseCheckRequest: {}", status.message())); + } + break; case OpCodes::Multi: - parseMultiRequest(data, offset, len); + status = parseMultiRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseMultiRequest: {}", status.message())); + } + break; case OpCodes::Reconfig: - parseReconfigRequest(data, offset, len); + status = parseReconfigRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseReconfigRequest: {}", status.message())); + } + break; case OpCodes::SetWatches: - parseSetWatchesRequest(data, offset, len); + status = parseSetWatchesRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError( + fmt::format("parseSetWatchesRequest: {}", status.message())); + } + break; case OpCodes::SetWatches2: - parseSetWatches2Request(data, offset, len); + status = parseSetWatches2Request(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError( + fmt::format("parseSetWatches2Request: {}", status.message())); + } + break; case OpCodes::AddWatch: - parseAddWatchRequest(data, offset, len); + status = parseAddWatchRequest(data, offset, len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError( + fmt::format("parseAddWatchesRequest: {}", status.message())); + } + break; case OpCodes::CheckWatches: - parseXWatchesRequest(data, offset, len, OpCodes::CheckWatches); + status = parseXWatchesRequest(data, offset, len.value(), OpCodes::CheckWatches); + if (!status.ok()) { + return absl::InvalidArgumentError( + fmt::format("parseXWatchesRequest (check watches): {}", status.message())); + } + break; case OpCodes::RemoveWatches: - parseXWatchesRequest(data, offset, len, OpCodes::RemoveWatches); + status = parseXWatchesRequest(data, offset, len.value(), OpCodes::RemoveWatches); + if (!status.ok()) { + return absl::InvalidArgumentError( + fmt::format("parseXWatchesRequest (remove watches): {}", status.message())); + } + break; case OpCodes::GetEphemerals: - callbacks_.onGetEphemeralsRequest(pathOnlyRequest(data, offset, len)); + callbacks_.onGetEphemeralsRequest(pathOnlyRequest(data, offset, len.value())); break; case OpCodes::GetAllChildrenNumber: - callbacks_.onGetAllChildrenNumberRequest(pathOnlyRequest(data, offset, len)); + callbacks_.onGetAllChildrenNumberRequest(pathOnlyRequest(data, offset, len.value())); break; case OpCodes::Close: callbacks_.onCloseRequest(); break; default: - throw EnvoyException(fmt::format("Unknown opcode: {}", enumToSignedInt(opcode))); + ENVOY_LOG(debug, "[zookeeper_proxy] decodeOnData exception: unknown opcode {}", + enumToSignedInt(opcode)); + callbacks_.onDecodeError(); + return absl::nullopt; } - requests_by_xid_[xid] = {opcode, std::move(start_time)}; + requests_by_xid_[xid.value()] = {opcode, std::move(start_time)}; return opcode; } -absl::optional DecoderImpl::decodeOnWrite(Buffer::Instance& data, uint64_t& offset) { - ENVOY_LOG(trace, "zookeeper_proxy: decoding response with {} bytes at offset {}", data.length(), +absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Instance& data, + uint64_t& offset) { + ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with {} bytes at offset {}", data.length(), offset); // Check message length. - const int32_t len = helper_.peekInt32(data, offset); - ENVOY_LOG(trace, "zookeeper_proxy: decoding response with len {} at offset {}", len, offset); - ensureMinLength(len, XID_LENGTH + ZXID_LENGTH + INT_LENGTH); // xid + zxid + err - ensureMaxLength(len); + const absl::StatusOr len = helper_.peekInt32(data, offset); + if (!len.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(fmt::format("peekInt32 for len: {}", len.status().message())); + } + + ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with len.value() {} at offset {}", + len.value(), offset); + + absl::Status status = + ensureMinLength(len.value(), XID_LENGTH + ZXID_LENGTH + INT_LENGTH); // xid + zxid + err + if (!status.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(fmt::format("ensureMinLength: {}", status.message())); + } + + status = ensureMaxLength(len.value()); + if (!status.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(fmt::format("ensureMaxLength: {}", status.message())); + } - const auto xid = helper_.peekInt32(data, offset); - ENVOY_LOG(trace, "zookeeper_proxy: decoding response with xid {} at offset {}", xid, offset); - const auto xid_code = static_cast(xid); + const absl::StatusOr xid = helper_.peekInt32(data, offset); + if (!xid.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(fmt::format("peekInt32 for xid: {}", xid.status().message())); + } - std::chrono::milliseconds latency; + ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with xid {} at offset {}", xid.value(), + offset); + const auto xid_code = static_cast(xid.value()); + + absl::StatusOr latency; OpCodes opcode; switch (xid_code) { @@ -201,321 +348,751 @@ absl::optional DecoderImpl::decodeOnWrite(Buffer::Instance& data, uint6 case XidCodes::AuthXid: ABSL_FALLTHROUGH_INTENDED; case XidCodes::SetWatchesXid: - latency = fetchControlRequestData(xid, opcode); + latency = fetchControlRequestData(xid.value(), opcode); + if (!latency.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError( + fmt::format("fetchControlRequestData: {}", latency.status().message())); + } break; case XidCodes::WatchXid: // WATCH_XID is generated by the server, no need to fetch opcode and latency here. break; default: - latency = fetchDataRequestData(xid, opcode); + latency = fetchDataRequestData(xid.value(), opcode); + if (!latency.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError( + fmt::format("fetchDataRequestData: {}", latency.status().message())); + } } // Connect responses are special, they have no full reply header - // but just an XID with no zxid nor error fields like the ones + // but just an xid.value() with no zxid.value() nor error fields like the ones // available for all other server generated messages. if (xid_code == XidCodes::ConnectXid) { - parseConnectResponse(data, offset, len, latency); + status = parseConnectResponse(data, offset, len.value(), latency.value()); + if (!status.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(fmt::format("parseConnectResponse: {}", status.message())); + } + return opcode; } // Control responses that aren't connect, with XIDs <= 0. - const auto zxid = helper_.peekInt64(data, offset); - const auto error = helper_.peekInt32(data, offset); - ENVOY_LOG(trace, "zookeeper_proxy: decoding response with zxid {} and error {} at offset {}", - zxid, error, offset); + const absl::StatusOr zxid = helper_.peekInt64(data, offset); + if (!zxid.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError( + fmt::format("peekInt64 for zxid: {}", zxid.status().message())); + } + + const absl::StatusOr error = helper_.peekInt32(data, offset); + if (!error.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError( + fmt::format("peekInt32 for error: {}", error.status().message())); + } + + ENVOY_LOG(trace, + "[zookeeper_proxy] decoding response with zxid.value() {} and error {} at offset {}", + zxid.value(), error.value(), offset); + switch (xid_code) { case XidCodes::PingXid: - callbacks_.onResponse(OpCodes::Ping, xid, zxid, error, latency); + callbacks_.onResponse(OpCodes::Ping, xid.value(), zxid.value(), error.value(), latency.value()); return opcode; case XidCodes::AuthXid: - callbacks_.onResponse(OpCodes::SetAuth, xid, zxid, error, latency); + callbacks_.onResponse(OpCodes::SetAuth, xid.value(), zxid.value(), error.value(), + latency.value()); return opcode; case XidCodes::SetWatchesXid: - callbacks_.onResponse(OpCodes::SetWatches, xid, zxid, error, latency); + callbacks_.onResponse(OpCodes::SetWatches, xid.value(), zxid.value(), error.value(), + latency.value()); return opcode; case XidCodes::WatchXid: - parseWatchEvent(data, offset, len, zxid, error); + status = parseWatchEvent(data, offset, len.value(), zxid.value(), error.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("parseWatchEvent: {}", status.message())); + } + return absl::nullopt; // WATCH_XID is generated by the server, it has no corresponding opcode. default: break; } - callbacks_.onResponse(opcode, xid, zxid, error, latency); - offset += (len - (XID_LENGTH + ZXID_LENGTH + INT_LENGTH)); + callbacks_.onResponse(opcode, xid.value(), zxid.value(), error.value(), latency.value()); + offset += (len.value() - (XID_LENGTH + ZXID_LENGTH + INT_LENGTH)); return opcode; } -void DecoderImpl::ensureMinLength(const int32_t len, const int32_t minlen) const { +absl::Status DecoderImpl::ensureMinLength(const int32_t len, const int32_t minlen) const { if (len < minlen) { - throw EnvoyException("Packet is too small"); + return absl::InvalidArgumentError("packet is too small"); } + return absl::OkStatus(); } -void DecoderImpl::ensureMaxLength(const int32_t len) const { +absl::Status DecoderImpl::ensureMaxLength(const int32_t len) const { if (static_cast(len) > max_packet_bytes_) { - throw EnvoyException("Packet is too big"); + return absl::InvalidArgumentError("packet is too big"); } + return absl::OkStatus(); } -void DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); - +absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, uint32_t len) { + absl::Status status = + ensureMinLength(len, XID_LENGTH + ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } // Skip zxid, timeout, and session id. offset += ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH; // Skip password. - skipString(data, offset); + status = skipString(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } - const bool readonly = maybeReadBool(data, offset); + const absl::StatusOr readonly = maybeReadBool(data, offset); + if (!readonly.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(readonly.status().message()); + } - callbacks_.onConnect(readonly); -} + callbacks_.onConnect(readonly.value()); -void DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + INT_LENGTH + INT_LENGTH); + return absl::OkStatus(); +} +absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { + absl::Status status = + ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + INT_LENGTH + INT_LENGTH); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } // Skip opcode + type. offset += OPCODE_LENGTH + INT_LENGTH; - const std::string scheme = helper_.peekString(data, offset); + + const absl::StatusOr scheme = helper_.peekString(data, offset); + if (!scheme.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(scheme.status().message()); + } + // Skip credential. - skipString(data, offset); + status = skipString(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + callbacks_.onAuthRequest(scheme.value()); - callbacks_.onAuthRequest(scheme); + return absl::OkStatus(); } -void DecoderImpl::parseGetDataRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); +absl::Status DecoderImpl::parseGetDataRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } + + const absl::StatusOr watch = helper_.peekBool(data, offset); + if (!watch.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(watch.status().message()); + } - const std::string path = helper_.peekString(data, offset); - const bool watch = helper_.peekBool(data, offset); + callbacks_.onGetDataRequest(path.value(), watch.value()); - callbacks_.onGetDataRequest(path, watch); + return absl::OkStatus(); } -void DecoderImpl::skipAcls(Buffer::Instance& data, uint64_t& offset) { - const int32_t count = helper_.peekInt32(data, offset); +absl::Status DecoderImpl::skipAcls(Buffer::Instance& data, uint64_t& offset) { + const absl::StatusOr count = helper_.peekInt32(data, offset); + if (!count.ok()) { + return absl::InvalidArgumentError(fmt::format("skipAcls: {}", count.status().message())); + } - for (int i = 0; i < count; ++i) { + for (int i = 0; i < count.value(); ++i) { // Perms. - helper_.peekInt32(data, offset); + absl::StatusOr perms = helper_.peekInt32(data, offset); + if (!perms.ok()) { + return absl::InvalidArgumentError(fmt::format("skipAcls: {}", perms.status().message())); + } // Skip scheme. - skipString(data, offset); + absl::Status status = skipString(data, offset); + if (!status.ok()) { + return status; + } // Skip cred. - skipString(data, offset); + status = skipString(data, offset); + if (!status.ok()) { + return status; + } } + + return absl::OkStatus(); } -void DecoderImpl::parseCreateRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, - OpCodes opcode) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (4 * INT_LENGTH)); +absl::Status DecoderImpl::parseCreateRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, + OpCodes opcode) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (4 * INT_LENGTH)); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } - const std::string path = helper_.peekString(data, offset); + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } // Skip data. - skipString(data, offset); - skipAcls(data, offset); + status = skipString(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + status = skipAcls(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } - const CreateFlags flags = static_cast(helper_.peekInt32(data, offset)); - callbacks_.onCreateRequest(path, flags, opcode); + absl::StatusOr flag_data = helper_.peekInt32(data, offset); + if (!flag_data.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(flag_data.status().message()); + } + + const CreateFlags flags = static_cast(flag_data.value()); + status = callbacks_.onCreateRequest(path.value(), flags, opcode); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + return absl::OkStatus(); } -void DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH)); +absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH)); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } - const std::string path = helper_.peekString(data, offset); // Skip data. - skipString(data, offset); + status = skipString(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + // Ignore version. - helper_.peekInt32(data, offset); + absl::StatusOr version = helper_.peekInt32(data, offset); + if (!version.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(version.status().message()); + } + + callbacks_.onSetRequest(path.value()); - callbacks_.onSetRequest(path); + return absl::OkStatus(); } -void DecoderImpl::parseGetChildrenRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, - const bool two) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); +absl::Status DecoderImpl::parseGetChildrenRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len, const bool two) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } - const std::string path = helper_.peekString(data, offset); - const bool watch = helper_.peekBool(data, offset); + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } + + const absl::StatusOr watch = helper_.peekBool(data, offset); + if (!watch.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(watch.status().message()); + } + + callbacks_.onGetChildrenRequest(path.value(), watch.value(), two); - callbacks_.onGetChildrenRequest(path, watch, two); + return absl::OkStatus(); } -void DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); +absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } + + const absl::StatusOr version = helper_.peekInt32(data, offset); + if (!version.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(version.status().message()); + } - const std::string path = helper_.peekString(data, offset); - const int32_t version = helper_.peekInt32(data, offset); + callbacks_.onDeleteRequest(path.value(), version.value()); - callbacks_.onDeleteRequest(path, version); + return absl::OkStatus(); } -void DecoderImpl::parseExistsRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); +absl::Status DecoderImpl::parseExistsRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } + + const absl::StatusOr watch = helper_.peekBool(data, offset); + if (!watch.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(watch.status().message()); + } - const std::string path = helper_.peekString(data, offset); - const bool watch = helper_.peekBool(data, offset); + callbacks_.onExistsRequest(path.value(), watch.value()); - callbacks_.onExistsRequest(path, watch); + return absl::OkStatus(); } -void DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); +absl::Status DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } - const std::string path = helper_.peekString(data, offset); + callbacks_.onGetAclRequest(path.value()); - callbacks_.onGetAclRequest(path); + return absl::OkStatus(); } -void DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); +absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } + + status = skipAcls(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } - const std::string path = helper_.peekString(data, offset); - skipAcls(data, offset); - const int32_t version = helper_.peekInt32(data, offset); + const absl::StatusOr version = helper_.peekInt32(data, offset); + if (!version.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(version.status().message()); + } + + callbacks_.onSetAclRequest(path.value(), version.value()); - callbacks_.onSetAclRequest(path, version); + return absl::OkStatus(); } -std::string DecoderImpl::pathOnlyRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); +absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); + if (!status.ok()) { + ENVOY_LOG(debug, "[zookeeper_proxy] path only request decoding exception {}", status.message()); + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(status.message()); + } + return helper_.peekString(data, offset); } -void DecoderImpl::parseCheckRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); +absl::Status DecoderImpl::parseCheckRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } + + const absl::StatusOr version = helper_.peekInt32(data, offset); + if (!version.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(version.status().message()); + } - const std::string path = helper_.peekString(data, offset); - const int32_t version = helper_.peekInt32(data, offset); + callbacks_.onCheckRequest(path.value(), version.value()); - callbacks_.onCheckRequest(path, version); + return absl::OkStatus(); } -void DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { +absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { // Treat empty transactions as a decoding error, there should be at least 1 header. - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + MULTI_HEADER_LENGTH); + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + MULTI_HEADER_LENGTH); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } while (true) { - const int32_t opcode = helper_.peekInt32(data, offset); - const bool done = helper_.peekBool(data, offset); + const absl::StatusOr opcode = helper_.peekInt32(data, offset); + if (!opcode.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(opcode.status().message()); + } + + const absl::StatusOr done = helper_.peekBool(data, offset); + if (!done.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(done.status().message()); + } + // Ignore error field. - helper_.peekInt32(data, offset); + const absl::StatusOr error = helper_.peekInt32(data, offset); + if (!error.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(error.status().message()); + } - if (done) { + if (done.value()) { break; } - switch (static_cast(opcode)) { + switch (static_cast(opcode.value())) { case OpCodes::Create: - parseCreateRequest(data, offset, len, OpCodes::Create); + status = parseCreateRequest(data, offset, len, OpCodes::Create); + if (!status.ok()) { + ENVOY_LOG(debug, "[zookeeper_proxy] multi request (create) decoding exception {}", + status.message()); + callbacks_.onDecodeError(); + return status; + } break; case OpCodes::SetData: - parseSetRequest(data, offset, len); + status = parseSetRequest(data, offset, len); + if (!status.ok()) { + ENVOY_LOG(debug, "[zookeeper_proxy] multi request (set) decoding exception {}", + status.message()); + callbacks_.onDecodeError(); + return status; + } break; case OpCodes::Check: - parseCheckRequest(data, offset, len); + status = parseCheckRequest(data, offset, len); + if (!status.ok()) { + ENVOY_LOG(debug, "[zookeeper_proxy] multi request (check) decoding exception {}", + status.message()); + callbacks_.onDecodeError(); + return status; + } break; case OpCodes::Delete: - parseDeleteRequest(data, offset, len); + status = parseDeleteRequest(data, offset, len); + if (!status.ok()) { + ENVOY_LOG(debug, "[zookeeper_proxy] multi request (delete) decoding exception {}", + status.message()); + callbacks_.onDecodeError(); + return status; + } break; default: - throw EnvoyException(fmt::format("Unknown opcode within a transaction: {}", opcode)); + return absl::InvalidArgumentError( + fmt::format("unknown opcode within a transaction: {}", opcode.value())); } } callbacks_.onMultiRequest(); + + return absl::OkStatus(); } -void DecoderImpl::parseReconfigRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH) + LONG_LENGTH); +absl::Status DecoderImpl::parseReconfigRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = + ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH) + LONG_LENGTH); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } // Skip joining. - skipString(data, offset); + status = skipString(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + // Skip leaving. - skipString(data, offset); + status = skipString(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } // Skip new members. - skipString(data, offset); + status = skipString(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + // Read config id. - helper_.peekInt64(data, offset); + absl::StatusOr config_id = helper_.peekInt64(data, offset); + if (!config_id.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(config_id.status().message()); + } callbacks_.onReconfigRequest(); + + return absl::OkStatus(); } -void DecoderImpl::parseSetWatchesRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (3 * INT_LENGTH)); +absl::Status DecoderImpl::parseSetWatchesRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = + ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (3 * INT_LENGTH)); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } // Ignore relative Zxid. - helper_.peekInt64(data, offset); + absl::StatusOr zxid = helper_.peekInt64(data, offset); + if (!zxid.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(zxid.status().message()); + } + // Data watches. - skipStrings(data, offset); + status = skipStrings(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + // Exist watches. - skipStrings(data, offset); + status = skipStrings(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + // Child watches. - skipStrings(data, offset); + status = skipStrings(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } callbacks_.onSetWatchesRequest(); + + return absl::OkStatus(); } -void DecoderImpl::parseSetWatches2Request(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (5 * INT_LENGTH)); +absl::Status DecoderImpl::parseSetWatches2Request(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = + ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (5 * INT_LENGTH)); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } // Ignore relative Zxid. - helper_.peekInt64(data, offset); + absl::StatusOr zxid = helper_.peekInt64(data, offset); + if (!zxid.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(zxid.status().message()); + } + // Data watches. - skipStrings(data, offset); + status = skipStrings(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + // Exist watches. - skipStrings(data, offset); + status = skipStrings(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + // Child watches. - skipStrings(data, offset); + status = skipStrings(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + // Persistent watches. - skipStrings(data, offset); + status = skipStrings(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + // Persistent recursive watches. - skipStrings(data, offset); + status = skipStrings(data, offset); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } callbacks_.onSetWatches2Request(); + + return absl::OkStatus(); } -void DecoderImpl::parseAddWatchRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); +absl::Status DecoderImpl::parseAddWatchRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } + + const absl::StatusOr mode = helper_.peekInt32(data, offset); + if (!mode.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(mode.status().message()); + } - const std::string path = helper_.peekString(data, offset); - const int32_t mode = helper_.peekInt32(data, offset); + callbacks_.onAddWatchRequest(path.value(), mode.value()); - callbacks_.onAddWatchRequest(path, mode); + return absl::OkStatus(); } -void DecoderImpl::parseXWatchesRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, - OpCodes opcode) { - ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); +absl::Status DecoderImpl::parseXWatchesRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len, OpCodes opcode) { + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } - const std::string path = helper_.peekString(data, offset); - const int32_t type = helper_.peekInt32(data, offset); + const absl::StatusOr watch_type = helper_.peekInt32(data, offset); + if (!watch_type.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(watch_type.status().message()); + } if (opcode == OpCodes::CheckWatches) { - callbacks_.onCheckWatchesRequest(path, type); + callbacks_.onCheckWatchesRequest(path.value(), watch_type.value()); } else { - callbacks_.onRemoveWatchesRequest(path, type); + callbacks_.onRemoveWatchesRequest(path.value(), watch_type.value()); } + + return absl::OkStatus(); } -void DecoderImpl::skipString(Buffer::Instance& data, uint64_t& offset) { - const int32_t slen = helper_.peekInt32(data, offset); - if (slen < 0) { +absl::Status DecoderImpl::skipString(Buffer::Instance& data, uint64_t& offset) { + const absl::StatusOr slen = helper_.peekInt32(data, offset); + if (!slen.ok()) { + return absl::InvalidArgumentError(fmt::format("skipString: {}", slen.status().message())); + } + + if (slen.value() < 0) { ENVOY_LOG(trace, - "zookeeper_proxy: decoding response with negative string length {} at offset {}", - slen, offset); - return; + "[zookeeper_proxy] decoding response with negative string length {} at offset {}", + slen.value(), offset); + return absl::OkStatus(); } - helper_.skip(slen, offset); + + helper_.skip(slen.value(), offset); + + return absl::OkStatus(); } -void DecoderImpl::skipStrings(Buffer::Instance& data, uint64_t& offset) { - const int32_t count = helper_.peekInt32(data, offset); +absl::Status DecoderImpl::skipStrings(Buffer::Instance& data, uint64_t& offset) { + const absl::StatusOr count = helper_.peekInt32(data, offset); + if (!count.ok()) { + return absl::InvalidArgumentError(fmt::format("skipStrings: {}", count.status().message())); + } - for (int i = 0; i < count; ++i) { - skipString(data, offset); + for (int i = 0; i < count.value(); ++i) { + absl::Status status = skipString(data, offset); + if (!status.ok()) { + return status; + } } + + return absl::OkStatus(); } Network::FilterStatus DecoderImpl::onData(Buffer::Instance& data) { @@ -529,9 +1106,14 @@ Network::FilterStatus DecoderImpl::onWrite(Buffer::Instance& data) { Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, DecodeType dtype, Buffer::OwnedImpl& zk_filter_buffer) { const uint32_t zk_filter_buffer_len = zk_filter_buffer.length(); + absl::Status status; if (zk_filter_buffer_len == 0) { - decodeAndBufferHelper(data, dtype, zk_filter_buffer); + status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); + if (!status.ok()) { + ENVOY_LOG(debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); + } + return Network::FilterStatus::Continue; } @@ -539,52 +1121,67 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod // Prepending ZooKeeper filter buffer to the current network filter buffer can help to generate // full packets. data.prepend(zk_filter_buffer); - decodeAndBufferHelper(data, dtype, zk_filter_buffer); + + status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); + if (!status.ok()) { + ENVOY_LOG(debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); + } + // Drain the prepended ZooKeeper filter buffer. data.drain(zk_filter_buffer_len); return Network::FilterStatus::Continue; } -void DecoderImpl::decodeAndBufferHelper(Buffer::Instance& data, DecodeType dtype, - Buffer::OwnedImpl& zk_filter_buffer) { - ASSERT(dtype == DecodeType::READ || dtype == DecodeType::WRITE); +absl::Status DecoderImpl::decodeAndBufferHelper(Buffer::Instance& data, DecodeType dtype, + Buffer::OwnedImpl& zk_filter_buffer) { + ASSERT(dtype == DecodeType::READ || dtype == DecodeType::WRITE); // can we still use ASSERT??? const uint32_t data_len = data.length(); uint64_t offset = 0; - uint32_t len = 0; + absl::StatusOr len = 0; + absl::Status status; // Boolean to check whether there is at least one full packet in the network filter buffer (to // which the ZooKeeper filter buffer is prepended). bool has_full_packets = false; while (offset < data_len) { - TRY_NEEDS_AUDIT { - // Peek packet length. - len = helper_.peekInt32(data, offset); - ensureMinLength(len, dtype == DecodeType::READ ? XID_LENGTH + INT_LENGTH - : XID_LENGTH + ZXID_LENGTH + INT_LENGTH); - ensureMaxLength(len); - offset += len; - if (offset <= data_len) { - has_full_packets = true; - } + // Peek packet length. + len = helper_.peekInt32(data, offset); + if (!len.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(len.status().message()); } - END_TRY catch (const EnvoyException& e) { - ENVOY_LOG(debug, "zookeeper_proxy: decoding exception {}", e.what()); + + status = ensureMinLength(len.value(), dtype == DecodeType::READ + ? XID_LENGTH + INT_LENGTH + : XID_LENGTH + ZXID_LENGTH + INT_LENGTH); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + status = ensureMaxLength(len.value()); + if (!status.ok()) { callbacks_.onDecodeError(); - return; + return status; + } + + offset += len.value(); + if (offset <= data_len) { + has_full_packets = true; } } if (offset == data_len) { decode(data, dtype, offset); - return; + return absl::OkStatus(); } ASSERT(offset > data_len); std::string temp_data; if (has_full_packets) { - offset -= INT_LENGTH + len; + offset -= INT_LENGTH + len.value(); ASSERT(offset < data_len); // Decode full packets. // offset here represents the length of all full packets. @@ -601,87 +1198,135 @@ void DecoderImpl::decodeAndBufferHelper(Buffer::Instance& data, DecodeType dtype data.copyOut(0, data_len, temp_data.data()); zk_filter_buffer.add(temp_data.data(), temp_data.length()); } + + return absl::OkStatus(); } void DecoderImpl::decode(Buffer::Instance& data, DecodeType dtype, uint64_t full_packets_len) { uint64_t offset = 0; - TRY_NEEDS_AUDIT { - while (offset < full_packets_len) { - // Reset the helper's cursor, to ensure the current message stays within the - // allowed max length, even when it's different than the declared length - // by the message. - // - // Note: we need to keep two cursors — offset and helper_'s internal one — because - // a buffer may contain multiple messages, so offset is global while helper_'s - // internal cursor gets reset for each individual message. - helper_.reset(); - - const uint64_t current = offset; - absl::optional opcode; - switch (dtype) { - case DecodeType::READ: - opcode = decodeOnData(data, offset); - callbacks_.onRequestBytes(opcode, offset - current); + while (offset < full_packets_len) { + // Reset the helper's cursor, to ensure the current message stays within the + // allowed max length, even when it's different than the declared length + // by the message. + // + // Note: we need to keep two cursors — offset and helper_'s internal one — because + // a buffer may contain multiple messages, so offset is global while helper_'s + // internal cursor gets reset for each individual message. + helper_.reset(); + + const uint64_t current = offset; + absl::StatusOr> opcode; + switch (dtype) { + case DecodeType::READ: + opcode = decodeOnData(data, offset); + if (opcode.ok()) { + callbacks_.onRequestBytes(opcode.value(), offset - current); break; - case DecodeType::WRITE: - opcode = decodeOnWrite(data, offset); - callbacks_.onResponseBytes(opcode, offset - current); + } else { + ENVOY_LOG(debug, "[zookeeper_proxy] decodeOnData exception: {}", opcode.status().message()); + goto exit_decode_loop; + } + case DecodeType::WRITE: + opcode = decodeOnWrite(data, offset); + if (opcode.ok()) { + callbacks_.onResponseBytes(opcode.value(), offset - current); break; + } else { + ENVOY_LOG(debug, "[zookeeper_proxy] decodeOnWrite exception: {}", + opcode.status().message()); + goto exit_decode_loop; } } } - END_TRY catch (const EnvoyException& e) { - ENVOY_LOG(debug, "zookeeper_proxy: decoding exception {}", e.what()); - callbacks_.onDecodeError(); - } +exit_decode_loop:; } -void DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& offset, uint32_t len, - const std::chrono::milliseconds latency) { - ensureMinLength(len, PROTOCOL_VERSION_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); +absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& offset, + uint32_t len, + const std::chrono::milliseconds latency) { + absl::Status status = + ensureMinLength(len, PROTOCOL_VERSION_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); + if (!status.ok()) { + return status; + } - const auto timeout = helper_.peekInt32(data, offset); + const absl::StatusOr timeout = helper_.peekInt32(data, offset); + if (!timeout.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(timeout.status().message()); + } // Skip session id + password. offset += SESSION_LENGTH; - skipString(data, offset); + status = skipString(data, offset); + if (!status.ok()) { + return status; + } - const bool readonly = maybeReadBool(data, offset); + const absl::StatusOr readonly = maybeReadBool(data, offset); + if (!readonly.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(readonly.status().message()); + } + + callbacks_.onConnectResponse(0, timeout.value(), readonly.value(), latency); - callbacks_.onConnectResponse(0, timeout, readonly, latency); + return absl::OkStatus(); } -void DecoderImpl::parseWatchEvent(Buffer::Instance& data, uint64_t& offset, const uint32_t len, - const int64_t zxid, const int32_t error) { - ensureMinLength(len, SERVER_HEADER_LENGTH + (3 * INT_LENGTH)); +absl::Status DecoderImpl::parseWatchEvent(Buffer::Instance& data, uint64_t& offset, + const uint32_t len, const int64_t zxid, + const int32_t error) { + absl::Status status = ensureMinLength(len, SERVER_HEADER_LENGTH + (3 * INT_LENGTH)); + if (!status.ok()) { + callbacks_.onDecodeError(); + return status; + } + + const absl::StatusOr event_type = helper_.peekInt32(data, offset); + if (!event_type.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(event_type.status().message()); + } + + const absl::StatusOr client_state = helper_.peekInt32(data, offset); + if (!client_state.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(client_state.status().message()); + } + + const absl::StatusOr path = helper_.peekString(data, offset); + if (!path.ok()) { + callbacks_.onDecodeError(); + return absl::InvalidArgumentError(path.status().message()); + } - const auto event_type = helper_.peekInt32(data, offset); - const auto client_state = helper_.peekInt32(data, offset); - const auto path = helper_.peekString(data, offset); + callbacks_.onWatchEvent(event_type.value(), client_state.value(), path.value(), zxid, error); - callbacks_.onWatchEvent(event_type, client_state, path, zxid, error); + return absl::OkStatus(); } -bool DecoderImpl::maybeReadBool(Buffer::Instance& data, uint64_t& offset) { +absl::StatusOr DecoderImpl::maybeReadBool(Buffer::Instance& data, uint64_t& offset) { if (data.length() >= offset + 1) { return helper_.peekBool(data, offset); } return false; } -std::chrono::milliseconds DecoderImpl::fetchControlRequestData(const int32_t xid, OpCodes& opcode) { +absl::StatusOr DecoderImpl::fetchControlRequestData(const int32_t xid, + OpCodes& opcode) { // Find the corresponding request queue for this XID. const auto it = control_requests_by_xid_.find(xid); // If this fails, it's either a server-side bug or a malformed packet. if (it == control_requests_by_xid_.end()) { - throw EnvoyException(fmt::format("control request xid {} not found", xid)); + return absl::InvalidArgumentError(fmt::format("control request xid {} not found", xid)); } std::queue& rq_queue = it->second; if (rq_queue.empty()) { - throw EnvoyException(fmt::format("control request queue for {} is empty", xid)); + return absl::InvalidArgumentError(fmt::format("control request queue for {} is empty", xid)); } std::chrono::milliseconds latency = std::chrono::duration_cast( @@ -692,13 +1337,14 @@ std::chrono::milliseconds DecoderImpl::fetchControlRequestData(const int32_t xid return latency; } -std::chrono::milliseconds DecoderImpl::fetchDataRequestData(const int32_t xid, OpCodes& opcode) { +absl::StatusOr DecoderImpl::fetchDataRequestData(const int32_t xid, + OpCodes& opcode) { // Find the corresponding request for this XID. const auto it = requests_by_xid_.find(xid); // If this fails, it's either a server-side bug or a malformed packet. if (it == requests_by_xid_.end()) { - throw EnvoyException(fmt::format("xid {} not found", xid)); + return absl::InvalidArgumentError(fmt::format("data request xid {} not found", xid)); } std::chrono::milliseconds latency = std::chrono::duration_cast( diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.h b/source/extensions/filters/network/zookeeper_proxy/decoder.h index 34ad9b4d32ff..8f20f8c6be7a 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.h +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.h @@ -12,6 +12,7 @@ #include "source/extensions/filters/network/zookeeper_proxy/utils.h" #include "absl/container/node_hash_map.h" +#include "absl/status/statusor.h" namespace Envoy { namespace Extensions { @@ -85,16 +86,17 @@ class DecoderCallbacks { virtual void onPing() PURE; virtual void onAuthRequest(const std::string& scheme) PURE; virtual void onGetDataRequest(const std::string& path, bool watch) PURE; - virtual void onCreateRequest(const std::string& path, CreateFlags flags, OpCodes opcode) PURE; + virtual absl::Status onCreateRequest(const std::string& path, CreateFlags flags, + OpCodes opcode) PURE; virtual void onSetRequest(const std::string& path) PURE; virtual void onGetChildrenRequest(const std::string& path, bool watch, bool v2) PURE; - virtual void onGetEphemeralsRequest(const std::string& path) PURE; - virtual void onGetAllChildrenNumberRequest(const std::string& path) PURE; + virtual void onGetEphemeralsRequest(const absl::StatusOr& path) PURE; + virtual void onGetAllChildrenNumberRequest(const absl::StatusOr& path) PURE; virtual void onDeleteRequest(const std::string& path, int32_t version) PURE; virtual void onExistsRequest(const std::string& path, bool watch) PURE; virtual void onGetAclRequest(const std::string& path) PURE; virtual void onSetAclRequest(const std::string& path, int32_t version) PURE; - virtual void onSyncRequest(const std::string& path) PURE; + virtual void onSyncRequest(const absl::StatusOr& path) PURE; virtual void onCheckRequest(const std::string& path, int32_t version) PURE; virtual void onMultiRequest() PURE; virtual void onReconfigRequest() PURE; @@ -151,44 +153,50 @@ class DecoderImpl : public Decoder, Logger::Loggable { // (4) removes the prepended data. Network::FilterStatus decodeAndBuffer(Buffer::Instance& data, DecodeType dtype, Buffer::OwnedImpl& zk_filter_buffer); - void decodeAndBufferHelper(Buffer::Instance& data, DecodeType dtype, - Buffer::OwnedImpl& zk_filter_buffer); + absl::Status decodeAndBufferHelper(Buffer::Instance& data, DecodeType dtype, + Buffer::OwnedImpl& zk_filter_buffer); void decode(Buffer::Instance& data, DecodeType dtype, uint64_t full_packets_len); // decodeOnData and decodeOnWrite return ZooKeeper opcode or absl::nullopt. // absl::nullopt indicates WATCH_XID, which is generated by the server and has no corresponding // opcode. - absl::optional decodeOnData(Buffer::Instance& data, uint64_t& offset); - absl::optional decodeOnWrite(Buffer::Instance& data, uint64_t& offset); - void parseConnect(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseAuthRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseGetDataRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseCreateRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, OpCodes opcode); - void skipAcls(Buffer::Instance& data, uint64_t& offset); - void parseSetRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseGetChildrenRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, bool two); - void parseDeleteRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseExistsRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseGetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseSetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseCheckRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseMultiRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseReconfigRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseSetWatchesRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseSetWatches2Request(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseAddWatchRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseXWatchesRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, OpCodes opcode); - void skipString(Buffer::Instance& data, uint64_t& offset); - void skipStrings(Buffer::Instance& data, uint64_t& offset); - void ensureMinLength(int32_t len, int32_t minlen) const; - void ensureMaxLength(int32_t len) const; - std::string pathOnlyRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); - void parseConnectResponse(Buffer::Instance& data, uint64_t& offset, uint32_t len, - const std::chrono::milliseconds latency); - void parseWatchEvent(Buffer::Instance& data, uint64_t& offset, uint32_t len, int64_t zxid, - int32_t error); - bool maybeReadBool(Buffer::Instance& data, uint64_t& offset); - std::chrono::milliseconds fetchControlRequestData(const int32_t xid, OpCodes& opcode); - std::chrono::milliseconds fetchDataRequestData(const int32_t xid, OpCodes& opcode); + absl::StatusOr> decodeOnData(Buffer::Instance& data, uint64_t& offset); + absl::StatusOr> decodeOnWrite(Buffer::Instance& data, uint64_t& offset); + absl::Status parseConnect(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseAuthRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseGetDataRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseCreateRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, + OpCodes opcode); + absl::Status skipAcls(Buffer::Instance& data, uint64_t& offset); + absl::Status parseSetRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseGetChildrenRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, + bool two); + absl::Status parseDeleteRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseExistsRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseGetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseSetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseCheckRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseMultiRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseReconfigRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseSetWatchesRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseSetWatches2Request(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseAddWatchRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len); + absl::Status parseXWatchesRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, + OpCodes opcode); + absl::Status skipString(Buffer::Instance& data, uint64_t& offset); + absl::Status skipStrings(Buffer::Instance& data, uint64_t& offset); + absl::Status ensureMinLength(int32_t len, int32_t minlen) const; + absl::Status ensureMaxLength(int32_t len) const; + absl::StatusOr pathOnlyRequest(Buffer::Instance& data, uint64_t& offset, + uint32_t len); + absl::Status parseConnectResponse(Buffer::Instance& data, uint64_t& offset, uint32_t len, + const std::chrono::milliseconds latency); + absl::Status parseWatchEvent(Buffer::Instance& data, uint64_t& offset, uint32_t len, int64_t zxid, + int32_t error); + absl::StatusOr maybeReadBool(Buffer::Instance& data, uint64_t& offset); + absl::StatusOr fetchControlRequestData(const int32_t xid, + OpCodes& opcode); + absl::StatusOr fetchDataRequestData(const int32_t xid, + OpCodes& opcode); DecoderCallbacks& callbacks_; const uint32_t max_packet_bytes_; diff --git a/source/extensions/filters/network/zookeeper_proxy/filter.cc b/source/extensions/filters/network/zookeeper_proxy/filter.cc index 013bea03b1e0..260e1406a686 100644 --- a/source/extensions/filters/network/zookeeper_proxy/filter.cc +++ b/source/extensions/filters/network/zookeeper_proxy/filter.cc @@ -293,8 +293,8 @@ void ZooKeeperFilter::onGetDataRequest(const std::string& path, const bool watch setDynamicMetadata({{"opname", "getdata"}, {"path", path}, {"watch", watch ? "true" : "false"}}); } -void ZooKeeperFilter::onCreateRequest(const std::string& path, const CreateFlags flags, - const OpCodes opcode) { +absl::Status ZooKeeperFilter::onCreateRequest(const std::string& path, const CreateFlags flags, + const OpCodes opcode) { std::string opname; switch (opcode) { @@ -315,12 +315,14 @@ void ZooKeeperFilter::onCreateRequest(const std::string& path, const CreateFlags config_->stats_.createttl_rq_.inc(); break; default: - throw EnvoyException(fmt::format("Unknown opcode: {}", enumToSignedInt(opcode))); + return absl::InvalidArgumentError(fmt::format("unknown opcode: {}", enumToSignedInt(opcode))); break; } setDynamicMetadata( {{"opname", opname}, {"path", path}, {"create_type", createFlagsToString(flags)}}); + + return absl::OkStatus(); } void ZooKeeperFilter::onSetRequest(const std::string& path) { @@ -362,9 +364,11 @@ void ZooKeeperFilter::onSetAclRequest(const std::string& path, const int32_t ver setDynamicMetadata({{"opname", "setacl"}, {"path", path}, {"version", std::to_string(version)}}); } -void ZooKeeperFilter::onSyncRequest(const std::string& path) { +void ZooKeeperFilter::onSyncRequest(const absl::StatusOr& path) { config_->stats_.sync_rq_.inc(); - setDynamicMetadata({{"opname", "sync"}, {"path", path}}); + if (path.ok()) { + setDynamicMetadata({{"opname", "sync"}, {"path", path.value()}}); + } } void ZooKeeperFilter::onCheckRequest(const std::string&, const int32_t) { @@ -406,14 +410,18 @@ void ZooKeeperFilter::onAddWatchRequest(const std::string& path, const int32_t m setDynamicMetadata({{"opname", "addwatch"}, {"path", path}, {"mode", std::to_string(mode)}}); } -void ZooKeeperFilter::onGetEphemeralsRequest(const std::string& path) { +void ZooKeeperFilter::onGetEphemeralsRequest(const absl::StatusOr& path) { config_->stats_.getephemerals_rq_.inc(); - setDynamicMetadata({{"opname", "getephemerals"}, {"path", path}}); + if (path.ok()) { + setDynamicMetadata({{"opname", "getephemerals"}, {"path", path.value()}}); + } } -void ZooKeeperFilter::onGetAllChildrenNumberRequest(const std::string& path) { +void ZooKeeperFilter::onGetAllChildrenNumberRequest(const absl::StatusOr& path) { config_->stats_.getallchildrennumber_rq_.inc(); - setDynamicMetadata({{"opname", "getallchildrennumber"}, {"path", path}}); + if (path.ok()) { + setDynamicMetadata({{"opname", "getallchildrennumber"}, {"path", path.value()}}); + } } void ZooKeeperFilter::onCloseRequest() { diff --git a/source/extensions/filters/network/zookeeper_proxy/filter.h b/source/extensions/filters/network/zookeeper_proxy/filter.h index 294d8a3119da..4c7acb19fc35 100644 --- a/source/extensions/filters/network/zookeeper_proxy/filter.h +++ b/source/extensions/filters/network/zookeeper_proxy/filter.h @@ -17,6 +17,8 @@ #include "source/common/stats/symbol_table.h" #include "source/extensions/filters/network/zookeeper_proxy/decoder.h" +#include "absl/status/statusor.h" + namespace Envoy { namespace Extensions { namespace NetworkFilters { @@ -340,14 +342,14 @@ class ZooKeeperFilter : public Network::Filter, void onPing() override; void onAuthRequest(const std::string& scheme) override; void onGetDataRequest(const std::string& path, bool watch) override; - void onCreateRequest(const std::string& path, CreateFlags flags, OpCodes opcode) override; + absl::Status onCreateRequest(const std::string& path, CreateFlags flags, OpCodes opcode) override; void onSetRequest(const std::string& path) override; void onGetChildrenRequest(const std::string& path, bool watch, bool v2) override; void onDeleteRequest(const std::string& path, int32_t version) override; void onExistsRequest(const std::string& path, bool watch) override; void onGetAclRequest(const std::string& path) override; void onSetAclRequest(const std::string& path, int32_t version) override; - void onSyncRequest(const std::string& path) override; + void onSyncRequest(const absl::StatusOr& path) override; void onCheckRequest(const std::string& path, int32_t version) override; void onMultiRequest() override; void onReconfigRequest() override; @@ -356,8 +358,8 @@ class ZooKeeperFilter : public Network::Filter, void onAddWatchRequest(const std::string& path, const int32_t mode) override; void onCheckWatchesRequest(const std::string& path, int32_t type) override; void onRemoveWatchesRequest(const std::string& path, int32_t type) override; - void onGetEphemeralsRequest(const std::string& path) override; - void onGetAllChildrenNumberRequest(const std::string& path) override; + void onGetEphemeralsRequest(const absl::StatusOr& path) override; + void onGetAllChildrenNumberRequest(const absl::StatusOr& path) override; void onCloseRequest() override; void onResponseBytes(const absl::optional opcode, const uint64_t bytes) override; void onConnectResponse(int32_t proto_version, int32_t timeout, bool readonly, diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.cc b/source/extensions/filters/network/zookeeper_proxy/utils.cc index cf1c77a8bf19..f60d693fa595 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.cc +++ b/source/extensions/filters/network/zookeeper_proxy/utils.cc @@ -7,24 +7,33 @@ namespace Extensions { namespace NetworkFilters { namespace ZooKeeperProxy { -int32_t BufferHelper::peekInt32(Buffer::Instance& buffer, uint64_t& offset) { - ensureMaxLen(sizeof(int32_t)); +absl::StatusOr BufferHelper::peekInt32(Buffer::Instance& buffer, uint64_t& offset) { + absl::Status status = ensureMaxLen(sizeof(int32_t)); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("peekInt32: {}", status.message())); + } const int32_t val = buffer.peekBEInt(offset); offset += sizeof(int32_t); return val; } -int64_t BufferHelper::peekInt64(Buffer::Instance& buffer, uint64_t& offset) { - ensureMaxLen(sizeof(int64_t)); +absl::StatusOr BufferHelper::peekInt64(Buffer::Instance& buffer, uint64_t& offset) { + absl::Status status = ensureMaxLen(sizeof(int64_t)); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("peekInt64: {}", status.message())); + } const int64_t val = buffer.peekBEInt(offset); offset += sizeof(int64_t); return val; } -bool BufferHelper::peekBool(Buffer::Instance& buffer, uint64_t& offset) { - ensureMaxLen(1); +absl::StatusOr BufferHelper::peekBool(Buffer::Instance& buffer, uint64_t& offset) { + absl::Status status = ensureMaxLen(1); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("peekBool: {}", status.message())); + } const char byte = buffer.peekInt(offset); const bool val = static_cast(byte); @@ -32,24 +41,30 @@ bool BufferHelper::peekBool(Buffer::Instance& buffer, uint64_t& offset) { return val; } -std::string BufferHelper::peekString(Buffer::Instance& buffer, uint64_t& offset) { +absl::StatusOr BufferHelper::peekString(Buffer::Instance& buffer, uint64_t& offset) { std::string val; - const uint32_t len = peekInt32(buffer, offset); + const absl::StatusOr len = peekInt32(buffer, offset); + if (!len.ok()) { + return absl::InvalidArgumentError(fmt::format("peekString: {}", len.status().message())); + } - if (len == 0) { + if (len.value() == 0) { return val; } - if (buffer.length() < (offset + len)) { - throw EnvoyException("peekString: buffer is smaller than string length"); + if (buffer.length() < (offset + len.value())) { + return absl::InvalidArgumentError("peekString: buffer is smaller than string length"); } - ensureMaxLen(len); + absl::Status status = ensureMaxLen(len.value()); + if (!status.ok()) { + return absl::InvalidArgumentError(fmt::format("peekString: {}", status.message())); + } - std::unique_ptr data(new char[len]); - buffer.copyOut(offset, len, data.get()); - val.assign(data.get(), len); - offset += len; + std::unique_ptr data(new char[len.value()]); + buffer.copyOut(offset, len.value(), data.get()); + val.assign(data.get(), len.value()); + offset += len.value(); return val; } @@ -59,12 +74,14 @@ void BufferHelper::skip(const uint32_t len, uint64_t& offset) { current_ += len; } -void BufferHelper::ensureMaxLen(const uint32_t size) { +absl::Status BufferHelper::ensureMaxLen(const uint32_t size) { current_ += size; if (current_ > max_len_) { - throw EnvoyException("read beyond max length"); + return absl::InvalidArgumentError("read beyond max length"); } + + return absl::OkStatus(); } } // namespace ZooKeeperProxy diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index dc699afaf548..f85910b608e8 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -9,6 +9,8 @@ #include "source/common/common/byte_order.h" #include "source/common/common/logger.h" +#include "absl/status/statusor.h" + namespace Envoy { namespace Extensions { namespace NetworkFilters { @@ -27,15 +29,15 @@ class BufferHelper : public Logger::Loggable { public: BufferHelper(uint32_t max_len) : max_len_(max_len) {} - int32_t peekInt32(Buffer::Instance& buffer, uint64_t& offset); - int64_t peekInt64(Buffer::Instance& buffer, uint64_t& offset); - std::string peekString(Buffer::Instance& buffer, uint64_t& offset); - bool peekBool(Buffer::Instance& buffer, uint64_t& offset); + absl::StatusOr peekInt32(Buffer::Instance& buffer, uint64_t& offset); + absl::StatusOr peekInt64(Buffer::Instance& buffer, uint64_t& offset); + absl::StatusOr peekString(Buffer::Instance& buffer, uint64_t& offset); + absl::StatusOr peekBool(Buffer::Instance& buffer, uint64_t& offset); void skip(uint32_t len, uint64_t& offset); void reset() { current_ = 0; } private: - void ensureMaxLen(uint32_t size); + absl::Status ensureMaxLen(uint32_t size); const uint32_t max_len_; uint32_t current_{}; From bc58eb64b1b430dba35c04a0fdbabd466ec2270f Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Mon, 23 Oct 2023 16:12:21 -0700 Subject: [PATCH 02/16] Code clean up Signed-off-by: Zhewei Hu --- .../network/zookeeper_proxy/decoder.cc | 11 ++++++---- .../filters/network/zookeeper_proxy/filter.cc | 21 +++++++++++-------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index cf0d7dfd3028..0dda1ebe53d9 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -368,12 +368,11 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta } // Connect responses are special, they have no full reply header - // but just an xid.value() with no zxid.value() nor error fields like the ones + // but just an xid with no zxid nor error fields like the ones // available for all other server generated messages. if (xid_code == XidCodes::ConnectXid) { status = parseConnectResponse(data, offset, len.value(), latency.value()); if (!status.ok()) { - callbacks_.onDecodeError(); return absl::InvalidArgumentError(fmt::format("parseConnectResponse: {}", status.message())); } @@ -858,6 +857,7 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of } break; default: + callbacks_.onDecodeError(); return absl::InvalidArgumentError( fmt::format("unknown opcode within a transaction: {}", opcode.value())); } @@ -1134,7 +1134,7 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod absl::Status DecoderImpl::decodeAndBufferHelper(Buffer::Instance& data, DecodeType dtype, Buffer::OwnedImpl& zk_filter_buffer) { - ASSERT(dtype == DecodeType::READ || dtype == DecodeType::WRITE); // can we still use ASSERT??? + ASSERT(dtype == DecodeType::READ || dtype == DecodeType::WRITE); const uint32_t data_len = data.length(); uint64_t offset = 0; @@ -1149,7 +1149,8 @@ absl::Status DecoderImpl::decodeAndBufferHelper(Buffer::Instance& data, DecodeTy len = helper_.peekInt32(data, offset); if (!len.ok()) { callbacks_.onDecodeError(); - return absl::InvalidArgumentError(len.status().message()); + return absl::InvalidArgumentError( + fmt::format("peekInt32 for len: {}", len.status().message())); } status = ensureMinLength(len.value(), dtype == DecodeType::READ @@ -1248,6 +1249,7 @@ absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& absl::Status status = ensureMinLength(len, PROTOCOL_VERSION_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); if (!status.ok()) { + callbacks_.onDecodeError(); return status; } @@ -1261,6 +1263,7 @@ absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& offset += SESSION_LENGTH; status = skipString(data, offset); if (!status.ok()) { + callbacks_.onDecodeError(); return status; } diff --git a/source/extensions/filters/network/zookeeper_proxy/filter.cc b/source/extensions/filters/network/zookeeper_proxy/filter.cc index 260e1406a686..6b9842c1edb0 100644 --- a/source/extensions/filters/network/zookeeper_proxy/filter.cc +++ b/source/extensions/filters/network/zookeeper_proxy/filter.cc @@ -365,10 +365,11 @@ void ZooKeeperFilter::onSetAclRequest(const std::string& path, const int32_t ver } void ZooKeeperFilter::onSyncRequest(const absl::StatusOr& path) { - config_->stats_.sync_rq_.inc(); - if (path.ok()) { - setDynamicMetadata({{"opname", "sync"}, {"path", path.value()}}); + if (!path.ok()) { + return; } + config_->stats_.sync_rq_.inc(); + setDynamicMetadata({{"opname", "sync"}, {"path", path.value()}}); } void ZooKeeperFilter::onCheckRequest(const std::string&, const int32_t) { @@ -411,17 +412,19 @@ void ZooKeeperFilter::onAddWatchRequest(const std::string& path, const int32_t m } void ZooKeeperFilter::onGetEphemeralsRequest(const absl::StatusOr& path) { - config_->stats_.getephemerals_rq_.inc(); - if (path.ok()) { - setDynamicMetadata({{"opname", "getephemerals"}, {"path", path.value()}}); + if (!path.ok()) { + return; } + config_->stats_.getephemerals_rq_.inc(); + setDynamicMetadata({{"opname", "getephemerals"}, {"path", path.value()}}); } void ZooKeeperFilter::onGetAllChildrenNumberRequest(const absl::StatusOr& path) { - config_->stats_.getallchildrennumber_rq_.inc(); - if (path.ok()) { - setDynamicMetadata({{"opname", "getallchildrennumber"}, {"path", path.value()}}); + if (!path.ok()) { + return; } + config_->stats_.getallchildrennumber_rq_.inc(); + setDynamicMetadata({{"opname", "getallchildrennumber"}, {"path", path.value()}}); } void ZooKeeperFilter::onCloseRequest() { From 649b651f3fe8069dbf97381e42d8456a5fd76e16 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Mon, 6 Nov 2023 21:49:45 -0800 Subject: [PATCH 03/16] Use marcos Signed-off-by: Zhewei Hu --- .../network/zookeeper_proxy/decoder.cc | 631 ++++-------------- .../filters/network/zookeeper_proxy/utils.cc | 20 +- .../filters/network/zookeeper_proxy/utils.h | 42 +- 3 files changed, 171 insertions(+), 522 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index 0dda1ebe53d9..7ee17af5a14d 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -49,25 +49,16 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // Check message length. const absl::StatusOr len = helper_.peekInt32(data, offset); - if (!len.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(fmt::format("peekInt32 for len: {}", len.status().message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(len, fmt::format("peekInt32 for len: {}", len.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with len {} at offset {}", len.value(), offset); absl::Status status = ensureMinLength(len.value(), XID_LENGTH + INT_LENGTH); // xid + opcode - if (!status.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(fmt::format("ensureMinLength: {}", status.message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("ensureMinLength: {}", status.message())); status = ensureMaxLength(len.value()); - if (!status.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(fmt::format("ensureMaxLength: {}", status.message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("ensureMaxLength: {}", status.message())); auto start_time = time_source_.monotonicTime(); @@ -82,10 +73,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // However, some client implementations might expose setWatches // as a regular data request, so we support that as well. const absl::StatusOr xid = helper_.peekInt32(data, offset); - if (!xid.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(fmt::format("peerInt32 for xid: {}", xid.status().message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(xid, fmt::format("peerInt32 for xid: {}", xid.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with xid {} at offset {}", xid.value(), offset); @@ -93,9 +81,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan switch (static_cast(xid.value())) { case XidCodes::ConnectXid: status = parseConnect(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseConnect: {}", status.message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseConnect: {}", status.message())); control_requests_by_xid_[xid.value()].push({OpCodes::Connect, std::move(start_time)}); return OpCodes::Connect; @@ -106,19 +92,14 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan return OpCodes::Ping; case XidCodes::AuthXid: status = parseAuthRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseAuthRequest: {}", status.message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseAuthRequest: {}", status.message())); control_requests_by_xid_[xid.value()].push({OpCodes::SetAuth, std::move(start_time)}); return OpCodes::SetAuth; case XidCodes::SetWatchesXid: offset += OPCODE_LENGTH; status = parseSetWatchesRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError( - fmt::format("parseSetWatchesRequest: {}", status.message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseSetWatchesRequest: {}", status.message())); control_requests_by_xid_[xid.value()].push({OpCodes::SetWatches, std::move(start_time)}); return OpCodes::SetWatches; @@ -135,11 +116,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // must happen every 1/3 of the negotiated session timeout, to keep // the session alive. const absl::StatusOr oc = helper_.peekInt32(data, offset); - if (!oc.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError( - fmt::format("peekInt32 for opcode: {}", oc.status().message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(oc, fmt::format("peekInt32 for opcode: {}", oc.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with opcode {} at offset {}", oc.value(), offset); @@ -148,135 +125,77 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan switch (opcode) { case OpCodes::GetData: status = parseGetDataRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseGetDataRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseGetDataRequest: {}", status.message())); break; case OpCodes::Create: case OpCodes::Create2: case OpCodes::CreateContainer: case OpCodes::CreateTtl: status = parseCreateRequest(data, offset, len.value(), static_cast(opcode)); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseCreateRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseCreateRequest: {}", status.message())); break; case OpCodes::SetData: status = parseSetRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseSetRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseSetRequest: {}", status.message())); break; case OpCodes::GetChildren: status = parseGetChildrenRequest(data, offset, len.value(), false); - if (!status.ok()) { - return absl::InvalidArgumentError( - fmt::format("parseGetChildrenRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseGetChildrenRequest: {}", status.message())); break; case OpCodes::GetChildren2: status = parseGetChildrenRequest(data, offset, len.value(), true); - if (!status.ok()) { - return absl::InvalidArgumentError( - fmt::format("parseGetChildrenRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseGetChildrenRequest: {}", status.message())); break; case OpCodes::Delete: status = parseDeleteRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseDeleteRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseDeleteRequest: {}", status.message())); break; case OpCodes::Exists: status = parseExistsRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseExistsRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseExistsRequest: {}", status.message())); break; case OpCodes::GetAcl: status = parseGetAclRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseGetAclRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseGetAclRequest: {}", status.message())); break; case OpCodes::SetAcl: status = parseSetAclRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseSetAclRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseSetAclRequest: {}", status.message())); break; case OpCodes::Sync: callbacks_.onSyncRequest(pathOnlyRequest(data, offset, len.value())); break; case OpCodes::Check: status = parseCheckRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseCheckRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseCheckRequest: {}", status.message())); break; case OpCodes::Multi: status = parseMultiRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseMultiRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseMultiRequest: {}", status.message())); break; case OpCodes::Reconfig: status = parseReconfigRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseReconfigRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseReconfigRequest: {}", status.message())); break; case OpCodes::SetWatches: status = parseSetWatchesRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError( - fmt::format("parseSetWatchesRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseSetWatchesRequest: {}", status.message())); break; case OpCodes::SetWatches2: status = parseSetWatches2Request(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError( - fmt::format("parseSetWatches2Request: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseSetWatches2Request: {}", status.message())); break; case OpCodes::AddWatch: status = parseAddWatchRequest(data, offset, len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError( - fmt::format("parseAddWatchesRequest: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseAddWatchRequest: {}", status.message())); break; case OpCodes::CheckWatches: status = parseXWatchesRequest(data, offset, len.value(), OpCodes::CheckWatches); - if (!status.ok()) { - return absl::InvalidArgumentError( - fmt::format("parseXWatchesRequest (check watches): {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseXWatchesRequest (check watches): {}", status.message())); break; case OpCodes::RemoveWatches: status = parseXWatchesRequest(data, offset, len.value(), OpCodes::RemoveWatches); - if (!status.ok()) { - return absl::InvalidArgumentError( - fmt::format("parseXWatchesRequest (remove watches): {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseXWatchesRequest (remove watches): {}", status.message())); break; case OpCodes::GetEphemerals: callbacks_.onGetEphemeralsRequest(pathOnlyRequest(data, offset, len.value())); @@ -306,32 +225,20 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta // Check message length. const absl::StatusOr len = helper_.peekInt32(data, offset); - if (!len.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(fmt::format("peekInt32 for len: {}", len.status().message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(len, fmt::format("peekInt32 for len: {}", len.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with len.value() {} at offset {}", len.value(), offset); absl::Status status = ensureMinLength(len.value(), XID_LENGTH + ZXID_LENGTH + INT_LENGTH); // xid + zxid + err - if (!status.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(fmt::format("ensureMinLength: {}", status.message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("ensureMinLength: {}", status.message())); status = ensureMaxLength(len.value()); - if (!status.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(fmt::format("ensureMaxLength: {}", status.message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("ensureMaxLength: {}", status.message())); const absl::StatusOr xid = helper_.peekInt32(data, offset); - if (!xid.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(fmt::format("peekInt32 for xid: {}", xid.status().message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(xid, fmt::format("peekInt32 for xid: {}", xid.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with xid {} at offset {}", xid.value(), offset); @@ -349,22 +256,14 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta ABSL_FALLTHROUGH_INTENDED; case XidCodes::SetWatchesXid: latency = fetchControlRequestData(xid.value(), opcode); - if (!latency.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError( - fmt::format("fetchControlRequestData: {}", latency.status().message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(latency, fmt::format("fetchControlRequestData: {}", latency.status().message())); break; case XidCodes::WatchXid: // WATCH_XID is generated by the server, no need to fetch opcode and latency here. break; default: latency = fetchDataRequestData(xid.value(), opcode); - if (!latency.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError( - fmt::format("fetchDataRequestData: {}", latency.status().message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(latency, fmt::format("fetchDataRequestData: {}", latency.status().message())); } // Connect responses are special, they have no full reply header @@ -372,27 +271,16 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta // available for all other server generated messages. if (xid_code == XidCodes::ConnectXid) { status = parseConnectResponse(data, offset, len.value(), latency.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseConnectResponse: {}", status.message())); - } - + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseConnectResponse: {}", status.message())) return opcode; } // Control responses that aren't connect, with XIDs <= 0. const absl::StatusOr zxid = helper_.peekInt64(data, offset); - if (!zxid.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError( - fmt::format("peekInt64 for zxid: {}", zxid.status().message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, fmt::format("peekInt64 for zxid: {}", zxid.status().message())); const absl::StatusOr error = helper_.peekInt32(data, offset); - if (!error.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError( - fmt::format("peekInt32 for error: {}", error.status().message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, fmt::format("peekInt32 for error: {}", error.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with zxid.value() {} and error {} at offset {}", @@ -412,9 +300,7 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta return opcode; case XidCodes::WatchXid: status = parseWatchEvent(data, offset, len.value(), zxid.value(), error.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("parseWatchEvent: {}", status.message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseWatchEvent: {}", status.message())); return absl::nullopt; // WATCH_XID is generated by the server, it has no corresponding opcode. default: @@ -444,25 +330,17 @@ absl::Status DecoderImpl::ensureMaxLength(const int32_t len) const { absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + // Skip zxid, timeout, and session id. offset += ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH; // Skip password. status = skipString(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr readonly = maybeReadBool(data, offset); - if (!readonly.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(readonly.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, readonly.status().message()); callbacks_.onConnect(readonly.value()); @@ -472,25 +350,16 @@ absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + INT_LENGTH + INT_LENGTH); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip opcode + type. offset += OPCODE_LENGTH + INT_LENGTH; const absl::StatusOr scheme = helper_.peekString(data, offset); - if (!scheme.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(scheme.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, scheme.status().message()); // Skip credential. status = skipString(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onAuthRequest(scheme.value()); @@ -500,22 +369,13 @@ absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& off absl::Status DecoderImpl::parseGetDataRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - if (!watch.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(watch.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onGetDataRequest(path.value(), watch.value()); @@ -524,26 +384,18 @@ absl::Status DecoderImpl::parseGetDataRequest(Buffer::Instance& data, uint64_t& absl::Status DecoderImpl::skipAcls(Buffer::Instance& data, uint64_t& offset) { const absl::StatusOr count = helper_.peekInt32(data, offset); - if (!count.ok()) { - return absl::InvalidArgumentError(fmt::format("skipAcls: {}", count.status().message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(count, fmt::format("skipAcls: {}", count.status().message())); for (int i = 0; i < count.value(); ++i) { // Perms. absl::StatusOr perms = helper_.peekInt32(data, offset); - if (!perms.ok()) { - return absl::InvalidArgumentError(fmt::format("skipAcls: {}", perms.status().message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(perms, fmt::format("skipAcls: {}", perms.status().message())); // Skip scheme. absl::Status status = skipString(data, offset); - if (!status.ok()) { - return status; - } + ABSL_STATUS_RETURN_IF_STATUS_NOT_OK(status); // Skip cred. status = skipString(data, offset); - if (!status.ok()) { - return status; - } + ABSL_STATUS_RETURN_IF_STATUS_NOT_OK(status); } return absl::OkStatus(); @@ -552,72 +404,42 @@ absl::Status DecoderImpl::skipAcls(Buffer::Instance& data, uint64_t& offset) { absl::Status DecoderImpl::parseCreateRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, OpCodes opcode) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (4 * INT_LENGTH)); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); // Skip data. status = skipString(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); status = skipAcls(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); absl::StatusOr flag_data = helper_.peekInt32(data, offset); - if (!flag_data.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(flag_data.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(flag_data, flag_data.status().message()); const CreateFlags flags = static_cast(flag_data.value()); status = callbacks_.onCreateRequest(path.value(), flags, opcode); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); return absl::OkStatus(); } absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH)); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); // Skip data. status = skipString(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore version. absl::StatusOr version = helper_.peekInt32(data, offset); - if (!version.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(version.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onSetRequest(path.value()); @@ -627,22 +449,13 @@ absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offs absl::Status DecoderImpl::parseGetChildrenRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, const bool two) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - if (!watch.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(watch.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onGetChildrenRequest(path.value(), watch.value(), two); @@ -652,22 +465,13 @@ absl::Status DecoderImpl::parseGetChildrenRequest(Buffer::Instance& data, uint64 absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr version = helper_.peekInt32(data, offset); - if (!version.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(version.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onDeleteRequest(path.value(), version.value()); @@ -677,22 +481,13 @@ absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseExistsRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - if (!watch.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(watch.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onExistsRequest(path.value(), watch.value()); @@ -702,16 +497,10 @@ absl::Status DecoderImpl::parseExistsRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); callbacks_.onGetAclRequest(path.value()); @@ -721,28 +510,16 @@ absl::Status DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); status = skipAcls(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr version = helper_.peekInt32(data, offset); - if (!version.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(version.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onSetAclRequest(path.value(), version.value()); @@ -752,11 +529,7 @@ absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& o absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); - if (!status.ok()) { - ENVOY_LOG(debug, "[zookeeper_proxy] path only request decoding exception {}", status.message()); - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(status.message()); - } + COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, status.message(), debug, "[zookeeper_proxy] path only request decoding exception {}", status.message()); return helper_.peekString(data, offset); } @@ -764,22 +537,13 @@ absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, absl::Status DecoderImpl::parseCheckRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr version = helper_.peekInt32(data, offset); - if (!version.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(version.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onCheckRequest(path.value(), version.value()); @@ -790,30 +554,18 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of uint32_t len) { // Treat empty transactions as a decoding error, there should be at least 1 header. absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + MULTI_HEADER_LENGTH); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); while (true) { const absl::StatusOr opcode = helper_.peekInt32(data, offset); - if (!opcode.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(opcode.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(opcode, opcode.status().message()); const absl::StatusOr done = helper_.peekBool(data, offset); - if (!done.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(done.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(done, done.status().message()); // Ignore error field. const absl::StatusOr error = helper_.peekInt32(data, offset); - if (!error.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(error.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, error.status().message()); if (done.value()) { break; @@ -822,39 +574,19 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of switch (static_cast(opcode.value())) { case OpCodes::Create: status = parseCreateRequest(data, offset, len, OpCodes::Create); - if (!status.ok()) { - ENVOY_LOG(debug, "[zookeeper_proxy] multi request (create) decoding exception {}", - status.message()); - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] multi request (create) decoding exception {}", status.message()); break; case OpCodes::SetData: status = parseSetRequest(data, offset, len); - if (!status.ok()) { - ENVOY_LOG(debug, "[zookeeper_proxy] multi request (set) decoding exception {}", - status.message()); - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] multi request (set) decoding exception {}", status.message()); break; case OpCodes::Check: status = parseCheckRequest(data, offset, len); - if (!status.ok()) { - ENVOY_LOG(debug, "[zookeeper_proxy] multi request (check) decoding exception {}", - status.message()); - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] multi request (check) decoding exception {}", status.message()); break; case OpCodes::Delete: status = parseDeleteRequest(data, offset, len); - if (!status.ok()) { - ENVOY_LOG(debug, "[zookeeper_proxy] multi request (delete) decoding exception {}", - status.message()); - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] multi request (delete) decoding exception {}", status.message()); break; default: callbacks_.onDecodeError(); @@ -872,37 +604,22 @@ absl::Status DecoderImpl::parseReconfigRequest(Buffer::Instance& data, uint64_t& uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH) + LONG_LENGTH); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip joining. status = skipString(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip leaving. status = skipString(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip new members. status = skipString(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Read config id. absl::StatusOr config_id = helper_.peekInt64(data, offset); - if (!config_id.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(config_id.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(config_id, config_id.status().message()); callbacks_.onReconfigRequest(); @@ -913,38 +630,23 @@ absl::Status DecoderImpl::parseSetWatchesRequest(Buffer::Instance& data, uint64_ uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (3 * INT_LENGTH)); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore relative Zxid. absl::StatusOr zxid = helper_.peekInt64(data, offset); - if (!zxid.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(zxid.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); // Data watches. status = skipStrings(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Exist watches. status = skipStrings(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Child watches. status = skipStrings(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onSetWatchesRequest(); @@ -955,52 +657,31 @@ absl::Status DecoderImpl::parseSetWatches2Request(Buffer::Instance& data, uint64 uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (5 * INT_LENGTH)); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore relative Zxid. absl::StatusOr zxid = helper_.peekInt64(data, offset); - if (!zxid.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(zxid.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); // Data watches. status = skipStrings(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Exist watches. status = skipStrings(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Child watches. status = skipStrings(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Persistent watches. status = skipStrings(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); // Persistent recursive watches. status = skipStrings(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onSetWatches2Request(); @@ -1010,22 +691,13 @@ absl::Status DecoderImpl::parseSetWatches2Request(Buffer::Instance& data, uint64 absl::Status DecoderImpl::parseAddWatchRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr mode = helper_.peekInt32(data, offset); - if (!mode.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(mode.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(mode, mode.status().message()); callbacks_.onAddWatchRequest(path.value(), mode.value()); @@ -1035,22 +707,13 @@ absl::Status DecoderImpl::parseAddWatchRequest(Buffer::Instance& data, uint64_t& absl::Status DecoderImpl::parseXWatchesRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, OpCodes opcode) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch_type = helper_.peekInt32(data, offset); - if (!watch_type.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(watch_type.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch_type, watch_type.status().message()); if (opcode == OpCodes::CheckWatches) { callbacks_.onCheckWatchesRequest(path.value(), watch_type.value()); @@ -1063,9 +726,7 @@ absl::Status DecoderImpl::parseXWatchesRequest(Buffer::Instance& data, uint64_t& absl::Status DecoderImpl::skipString(Buffer::Instance& data, uint64_t& offset) { const absl::StatusOr slen = helper_.peekInt32(data, offset); - if (!slen.ok()) { - return absl::InvalidArgumentError(fmt::format("skipString: {}", slen.status().message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(slen, fmt::format("skipString: {}", slen.status().message())); if (slen.value() < 0) { ENVOY_LOG(trace, @@ -1081,15 +742,11 @@ absl::Status DecoderImpl::skipString(Buffer::Instance& data, uint64_t& offset) { absl::Status DecoderImpl::skipStrings(Buffer::Instance& data, uint64_t& offset) { const absl::StatusOr count = helper_.peekInt32(data, offset); - if (!count.ok()) { - return absl::InvalidArgumentError(fmt::format("skipStrings: {}", count.status().message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(count, fmt::format("skipStrings: {}", count.status().message())); for (int i = 0; i < count.value(); ++i) { absl::Status status = skipString(data, offset); - if (!status.ok()) { - return status; - } + ABSL_STATUS_RETURN_IF_STATUS_NOT_OK(status); } return absl::OkStatus(); @@ -1110,9 +767,7 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod if (zk_filter_buffer_len == 0) { status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); - if (!status.ok()) { - ENVOY_LOG(debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); - } + WRITE_ENVOY_LOG_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); return Network::FilterStatus::Continue; } @@ -1123,9 +778,7 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod data.prepend(zk_filter_buffer); status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); - if (!status.ok()) { - ENVOY_LOG(debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); - } + WRITE_ENVOY_LOG_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); // Drain the prepended ZooKeeper filter buffer. data.drain(zk_filter_buffer_len); @@ -1147,25 +800,15 @@ absl::Status DecoderImpl::decodeAndBufferHelper(Buffer::Instance& data, DecodeTy while (offset < data_len) { // Peek packet length. len = helper_.peekInt32(data, offset); - if (!len.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError( - fmt::format("peekInt32 for len: {}", len.status().message())); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(len, fmt::format("peekInt32 for len: {}", len.status().message())); status = ensureMinLength(len.value(), dtype == DecodeType::READ ? XID_LENGTH + INT_LENGTH : XID_LENGTH + ZXID_LENGTH + INT_LENGTH); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); status = ensureMaxLength(len.value()); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); offset += len.value(); if (offset <= data_len) { @@ -1248,30 +891,18 @@ absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& const std::chrono::milliseconds latency) { absl::Status status = ensureMinLength(len, PROTOCOL_VERSION_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr timeout = helper_.peekInt32(data, offset); - if (!timeout.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(timeout.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(timeout, timeout.status().message()); // Skip session id + password. offset += SESSION_LENGTH; status = skipString(data, offset); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr readonly = maybeReadBool(data, offset); - if (!readonly.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(readonly.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, readonly.status().message()); callbacks_.onConnectResponse(0, timeout.value(), readonly.value(), latency); @@ -1282,28 +913,16 @@ absl::Status DecoderImpl::parseWatchEvent(Buffer::Instance& data, uint64_t& offs const uint32_t len, const int64_t zxid, const int32_t error) { absl::Status status = ensureMinLength(len, SERVER_HEADER_LENGTH + (3 * INT_LENGTH)); - if (!status.ok()) { - callbacks_.onDecodeError(); - return status; - } + COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr event_type = helper_.peekInt32(data, offset); - if (!event_type.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(event_type.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(event_type, event_type.status().message()); const absl::StatusOr client_state = helper_.peekInt32(data, offset); - if (!client_state.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(client_state.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(client_state, client_state.status().message()); const absl::StatusOr path = helper_.peekString(data, offset); - if (!path.ok()) { - callbacks_.onDecodeError(); - return absl::InvalidArgumentError(path.status().message()); - } + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); callbacks_.onWatchEvent(event_type.value(), client_state.value(), path.value(), zxid, error); diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.cc b/source/extensions/filters/network/zookeeper_proxy/utils.cc index f60d693fa595..c8980c6e3da1 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.cc +++ b/source/extensions/filters/network/zookeeper_proxy/utils.cc @@ -9,9 +9,7 @@ namespace ZooKeeperProxy { absl::StatusOr BufferHelper::peekInt32(Buffer::Instance& buffer, uint64_t& offset) { absl::Status status = ensureMaxLen(sizeof(int32_t)); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("peekInt32: {}", status.message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("peekInt32: {}", status.message())); const int32_t val = buffer.peekBEInt(offset); offset += sizeof(int32_t); @@ -20,9 +18,7 @@ absl::StatusOr BufferHelper::peekInt32(Buffer::Instance& buffer, uint64 absl::StatusOr BufferHelper::peekInt64(Buffer::Instance& buffer, uint64_t& offset) { absl::Status status = ensureMaxLen(sizeof(int64_t)); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("peekInt64: {}", status.message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("peekInt64: {}", status.message())); const int64_t val = buffer.peekBEInt(offset); offset += sizeof(int64_t); @@ -31,9 +27,7 @@ absl::StatusOr BufferHelper::peekInt64(Buffer::Instance& buffer, uint64 absl::StatusOr BufferHelper::peekBool(Buffer::Instance& buffer, uint64_t& offset) { absl::Status status = ensureMaxLen(1); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("peekBool: {}", status.message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("peekBool: {}", status.message())); const char byte = buffer.peekInt(offset); const bool val = static_cast(byte); @@ -44,9 +38,7 @@ absl::StatusOr BufferHelper::peekBool(Buffer::Instance& buffer, uint64_t& absl::StatusOr BufferHelper::peekString(Buffer::Instance& buffer, uint64_t& offset) { std::string val; const absl::StatusOr len = peekInt32(buffer, offset); - if (!len.ok()) { - return absl::InvalidArgumentError(fmt::format("peekString: {}", len.status().message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(len, fmt::format("peekString: {}", len.status().message())); if (len.value() == 0) { return val; @@ -57,9 +49,7 @@ absl::StatusOr BufferHelper::peekString(Buffer::Instance& buffer, u } absl::Status status = ensureMaxLen(len.value()); - if (!status.ok()) { - return absl::InvalidArgumentError(fmt::format("peekString: {}", status.message())); - } + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("peekString: {}", status.message())); std::unique_ptr data(new char[len.value()]); buffer.copyOut(offset, len.value(), data.get()); diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index f85910b608e8..75901eac203f 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -19,7 +19,7 @@ namespace ZooKeeperProxy { /** * Helper for extracting ZooKeeper data from a buffer. * - * If at any point a peek is tried beyond max_len, an EnvoyException + * If at any point a peek is tried beyond max_len, an EnvoyException <---- reword this comment * will be thrown. This is important to protect Envoy against malformed * requests (e.g.: when the declared and actual length don't match). * @@ -43,6 +43,46 @@ class BufferHelper : public Logger::Loggable { uint32_t current_{}; }; +#define ABSL_STATUS_RETURN_IF_STATUS_NOT_OK(status) \ + if (!status.ok()) { \ + return status; \ + } + +#define WRITE_ENVOY_LOG_IF_STATUS_NOT_OK(status, log_level, ...) \ + if (!status.ok()) { \ + ENVOY_LOG(log_level, ##__VA_ARGS__); \ + } + +#define COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status) \ + if (!status.ok()) { \ + callbacks_.onDecodeError(); \ + return status; \ + } + +#define COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, log_level, ...) \ + if (!status.ok()) { \ + ENVOY_LOG(log_level, ##__VA_ARGS__); \ + callbacks_.onDecodeError(); \ + return status; \ + } + +#define RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ + if (!status.ok()) { \ + return absl::InvalidArgumentError(message); \ + } + +#define COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ + if (!status.ok()) { \ + callbacks_.onDecodeError(); \ + return absl::InvalidArgumentError(message); \ + } + +#define COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message, log_level, ...) \ + if (!status.ok()) { \ + ENVOY_LOG(log_level, ##__VA_ARGS__); \ + callbacks_.onDecodeError(); \ + return absl::InvalidArgumentError(message); \ + } } // namespace ZooKeeperProxy } // namespace NetworkFilters } // namespace Extensions From d3017dd5406a69acf790b75392dabfe2adcc94e4 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Mon, 6 Nov 2023 22:46:32 -0800 Subject: [PATCH 04/16] Format Signed-off-by: Zhewei Hu --- .../network/zookeeper_proxy/decoder.cc | 191 ++++++++++++------ .../filters/network/zookeeper_proxy/utils.cc | 3 +- .../filters/network/zookeeper_proxy/utils.h | 55 ++--- 3 files changed, 159 insertions(+), 90 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index 7ee17af5a14d..fd84556de3b5 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -49,16 +49,19 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // Check message length. const absl::StatusOr len = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(len, fmt::format("peekInt32 for len: {}", len.status().message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + len, fmt::format("peekInt32 for len: {}", len.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with len {} at offset {}", len.value(), offset); absl::Status status = ensureMinLength(len.value(), XID_LENGTH + INT_LENGTH); // xid + opcode - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("ensureMinLength: {}", status.message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("ensureMinLength: {}", status.message())); status = ensureMaxLength(len.value()); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("ensureMaxLength: {}", status.message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("ensureMaxLength: {}", status.message())); auto start_time = time_source_.monotonicTime(); @@ -73,7 +76,8 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // However, some client implementations might expose setWatches // as a regular data request, so we support that as well. const absl::StatusOr xid = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(xid, fmt::format("peerInt32 for xid: {}", xid.status().message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + xid, fmt::format("peerInt32 for xid: {}", xid.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with xid {} at offset {}", xid.value(), offset); @@ -81,7 +85,8 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan switch (static_cast(xid.value())) { case XidCodes::ConnectXid: status = parseConnect(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseConnect: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, + fmt::format("parseConnect: {}", status.message())); control_requests_by_xid_[xid.value()].push({OpCodes::Connect, std::move(start_time)}); return OpCodes::Connect; @@ -92,14 +97,16 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan return OpCodes::Ping; case XidCodes::AuthXid: status = parseAuthRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseAuthRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, + fmt::format("parseAuthRequest: {}", status.message())); control_requests_by_xid_[xid.value()].push({OpCodes::SetAuth, std::move(start_time)}); return OpCodes::SetAuth; case XidCodes::SetWatchesXid: offset += OPCODE_LENGTH; status = parseSetWatchesRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseSetWatchesRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseSetWatchesRequest: {}", status.message())); control_requests_by_xid_[xid.value()].push({OpCodes::SetWatches, std::move(start_time)}); return OpCodes::SetWatches; @@ -116,7 +123,8 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // must happen every 1/3 of the negotiated session timeout, to keep // the session alive. const absl::StatusOr oc = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(oc, fmt::format("peekInt32 for opcode: {}", oc.status().message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + oc, fmt::format("peekInt32 for opcode: {}", oc.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with opcode {} at offset {}", oc.value(), offset); @@ -125,77 +133,94 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan switch (opcode) { case OpCodes::GetData: status = parseGetDataRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseGetDataRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseGetDataRequest: {}", status.message())); break; case OpCodes::Create: case OpCodes::Create2: case OpCodes::CreateContainer: case OpCodes::CreateTtl: status = parseCreateRequest(data, offset, len.value(), static_cast(opcode)); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseCreateRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseCreateRequest: {}", status.message())); break; case OpCodes::SetData: status = parseSetRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseSetRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, + fmt::format("parseSetRequest: {}", status.message())); break; case OpCodes::GetChildren: status = parseGetChildrenRequest(data, offset, len.value(), false); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseGetChildrenRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseGetChildrenRequest: {}", status.message())); break; case OpCodes::GetChildren2: status = parseGetChildrenRequest(data, offset, len.value(), true); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseGetChildrenRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseGetChildrenRequest: {}", status.message())); break; case OpCodes::Delete: status = parseDeleteRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseDeleteRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseDeleteRequest: {}", status.message())); break; case OpCodes::Exists: status = parseExistsRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseExistsRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseExistsRequest: {}", status.message())); break; case OpCodes::GetAcl: status = parseGetAclRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseGetAclRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseGetAclRequest: {}", status.message())); break; case OpCodes::SetAcl: status = parseSetAclRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseSetAclRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseSetAclRequest: {}", status.message())); break; case OpCodes::Sync: callbacks_.onSyncRequest(pathOnlyRequest(data, offset, len.value())); break; case OpCodes::Check: status = parseCheckRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseCheckRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, + fmt::format("parseCheckRequest: {}", status.message())); break; case OpCodes::Multi: status = parseMultiRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseMultiRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, + fmt::format("parseMultiRequest: {}", status.message())); break; case OpCodes::Reconfig: status = parseReconfigRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseReconfigRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseReconfigRequest: {}", status.message())); break; case OpCodes::SetWatches: status = parseSetWatchesRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseSetWatchesRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseSetWatchesRequest: {}", status.message())); break; case OpCodes::SetWatches2: status = parseSetWatches2Request(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseSetWatches2Request: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseSetWatches2Request: {}", status.message())); break; case OpCodes::AddWatch: status = parseAddWatchRequest(data, offset, len.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseAddWatchRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseAddWatchRequest: {}", status.message())); break; case OpCodes::CheckWatches: status = parseXWatchesRequest(data, offset, len.value(), OpCodes::CheckWatches); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseXWatchesRequest (check watches): {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseXWatchesRequest (check watches): {}", status.message())); break; case OpCodes::RemoveWatches: status = parseXWatchesRequest(data, offset, len.value(), OpCodes::RemoveWatches); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseXWatchesRequest (remove watches): {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseXWatchesRequest (remove watches): {}", status.message())); break; case OpCodes::GetEphemerals: callbacks_.onGetEphemeralsRequest(pathOnlyRequest(data, offset, len.value())); @@ -225,20 +250,24 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta // Check message length. const absl::StatusOr len = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(len, fmt::format("peekInt32 for len: {}", len.status().message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + len, fmt::format("peekInt32 for len: {}", len.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with len.value() {} at offset {}", len.value(), offset); absl::Status status = ensureMinLength(len.value(), XID_LENGTH + ZXID_LENGTH + INT_LENGTH); // xid + zxid + err - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("ensureMinLength: {}", status.message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("ensureMinLength: {}", status.message())); status = ensureMaxLength(len.value()); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("ensureMaxLength: {}", status.message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("ensureMaxLength: {}", status.message())); const absl::StatusOr xid = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(xid, fmt::format("peekInt32 for xid: {}", xid.status().message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + xid, fmt::format("peekInt32 for xid: {}", xid.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with xid {} at offset {}", xid.value(), offset); @@ -256,14 +285,16 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta ABSL_FALLTHROUGH_INTENDED; case XidCodes::SetWatchesXid: latency = fetchControlRequestData(xid.value(), opcode); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(latency, fmt::format("fetchControlRequestData: {}", latency.status().message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + latency, fmt::format("fetchControlRequestData: {}", latency.status().message())); break; case XidCodes::WatchXid: // WATCH_XID is generated by the server, no need to fetch opcode and latency here. break; default: latency = fetchDataRequestData(xid.value(), opcode); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(latency, fmt::format("fetchDataRequestData: {}", latency.status().message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + latency, fmt::format("fetchDataRequestData: {}", latency.status().message())); } // Connect responses are special, they have no full reply header @@ -271,16 +302,19 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta // available for all other server generated messages. if (xid_code == XidCodes::ConnectXid) { status = parseConnectResponse(data, offset, len.value(), latency.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseConnectResponse: {}", status.message())) + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("parseConnectResponse: {}", status.message())) return opcode; } // Control responses that aren't connect, with XIDs <= 0. const absl::StatusOr zxid = helper_.peekInt64(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, fmt::format("peekInt64 for zxid: {}", zxid.status().message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + zxid, fmt::format("peekInt64 for zxid: {}", zxid.status().message())); const absl::StatusOr error = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, fmt::format("peekInt32 for error: {}", error.status().message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + error, fmt::format("peekInt32 for error: {}", error.status().message())); ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with zxid.value() {} and error {} at offset {}", @@ -300,7 +334,8 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta return opcode; case XidCodes::WatchXid: status = parseWatchEvent(data, offset, len.value(), zxid.value(), error.value()); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, fmt::format("parseWatchEvent: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, + fmt::format("parseWatchEvent: {}", status.message())); return absl::nullopt; // WATCH_XID is generated by the server, it has no corresponding opcode. default: @@ -340,7 +375,8 @@ absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr readonly = maybeReadBool(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, readonly.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, + readonly.status().message()); callbacks_.onConnect(readonly.value()); @@ -355,7 +391,8 @@ absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& off offset += OPCODE_LENGTH + INT_LENGTH; const absl::StatusOr scheme = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, scheme.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, + scheme.status().message()); // Skip credential. status = skipString(data, offset); @@ -384,12 +421,14 @@ absl::Status DecoderImpl::parseGetDataRequest(Buffer::Instance& data, uint64_t& absl::Status DecoderImpl::skipAcls(Buffer::Instance& data, uint64_t& offset) { const absl::StatusOr count = helper_.peekInt32(data, offset); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(count, fmt::format("skipAcls: {}", count.status().message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(count, + fmt::format("skipAcls: {}", count.status().message())); for (int i = 0; i < count.value(); ++i) { // Perms. absl::StatusOr perms = helper_.peekInt32(data, offset); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(perms, fmt::format("skipAcls: {}", perms.status().message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(perms, + fmt::format("skipAcls: {}", perms.status().message())); // Skip scheme. absl::Status status = skipString(data, offset); ABSL_STATUS_RETURN_IF_STATUS_NOT_OK(status); @@ -417,7 +456,8 @@ absl::Status DecoderImpl::parseCreateRequest(Buffer::Instance& data, uint64_t& o COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); absl::StatusOr flag_data = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(flag_data, flag_data.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(flag_data, + flag_data.status().message()); const CreateFlags flags = static_cast(flag_data.value()); status = callbacks_.onCreateRequest(path.value(), flags, opcode); @@ -439,7 +479,8 @@ absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offs // Ignore version. absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, + version.status().message()); callbacks_.onSetRequest(path.value()); @@ -471,7 +512,8 @@ absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& o COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, + version.status().message()); callbacks_.onDeleteRequest(path.value(), version.value()); @@ -519,7 +561,8 @@ absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& o COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, + version.status().message()); callbacks_.onSetAclRequest(path.value(), version.value()); @@ -529,7 +572,9 @@ absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& o absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); - COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, status.message(), debug, "[zookeeper_proxy] path only request decoding exception {}", status.message()); + COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, status.message(), debug, "[zookeeper_proxy] path only request decoding exception {}", + status.message()); return helper_.peekString(data, offset); } @@ -543,7 +588,8 @@ absl::Status DecoderImpl::parseCheckRequest(Buffer::Instance& data, uint64_t& of COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, + version.status().message()); callbacks_.onCheckRequest(path.value(), version.value()); @@ -558,14 +604,16 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of while (true) { const absl::StatusOr opcode = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(opcode, opcode.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(opcode, + opcode.status().message()); const absl::StatusOr done = helper_.peekBool(data, offset); COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(done, done.status().message()); // Ignore error field. const absl::StatusOr error = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, error.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, + error.status().message()); if (done.value()) { break; @@ -574,19 +622,27 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of switch (static_cast(opcode.value())) { case OpCodes::Create: status = parseCreateRequest(data, offset, len, OpCodes::Create); - COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] multi request (create) decoding exception {}", status.message()); + COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( + status, debug, "[zookeeper_proxy] multi request (create) decoding exception {}", + status.message()); break; case OpCodes::SetData: status = parseSetRequest(data, offset, len); - COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] multi request (set) decoding exception {}", status.message()); + COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( + status, debug, "[zookeeper_proxy] multi request (set) decoding exception {}", + status.message()); break; case OpCodes::Check: status = parseCheckRequest(data, offset, len); - COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] multi request (check) decoding exception {}", status.message()); + COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( + status, debug, "[zookeeper_proxy] multi request (check) decoding exception {}", + status.message()); break; case OpCodes::Delete: status = parseDeleteRequest(data, offset, len); - COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] multi request (delete) decoding exception {}", status.message()); + COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( + status, debug, "[zookeeper_proxy] multi request (delete) decoding exception {}", + status.message()); break; default: callbacks_.onDecodeError(); @@ -619,7 +675,8 @@ absl::Status DecoderImpl::parseReconfigRequest(Buffer::Instance& data, uint64_t& // Read config id. absl::StatusOr config_id = helper_.peekInt64(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(config_id, config_id.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(config_id, + config_id.status().message()); callbacks_.onReconfigRequest(); @@ -713,7 +770,8 @@ absl::Status DecoderImpl::parseXWatchesRequest(Buffer::Instance& data, uint64_t& COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch_type = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch_type, watch_type.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch_type, + watch_type.status().message()); if (opcode == OpCodes::CheckWatches) { callbacks_.onCheckWatchesRequest(path.value(), watch_type.value()); @@ -726,7 +784,8 @@ absl::Status DecoderImpl::parseXWatchesRequest(Buffer::Instance& data, uint64_t& absl::Status DecoderImpl::skipString(Buffer::Instance& data, uint64_t& offset) { const absl::StatusOr slen = helper_.peekInt32(data, offset); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(slen, fmt::format("skipString: {}", slen.status().message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(slen, + fmt::format("skipString: {}", slen.status().message())); if (slen.value() < 0) { ENVOY_LOG(trace, @@ -742,7 +801,8 @@ absl::Status DecoderImpl::skipString(Buffer::Instance& data, uint64_t& offset) { absl::Status DecoderImpl::skipStrings(Buffer::Instance& data, uint64_t& offset) { const absl::StatusOr count = helper_.peekInt32(data, offset); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(count, fmt::format("skipStrings: {}", count.status().message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(count, + fmt::format("skipStrings: {}", count.status().message())); for (int i = 0; i < count.value(); ++i) { absl::Status status = skipString(data, offset); @@ -767,7 +827,8 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod if (zk_filter_buffer_len == 0) { status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); - WRITE_ENVOY_LOG_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); + WRITE_ENVOY_LOG_IF_STATUS_NOT_OK( + status, debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); return Network::FilterStatus::Continue; } @@ -778,7 +839,8 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod data.prepend(zk_filter_buffer); status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); - WRITE_ENVOY_LOG_IF_STATUS_NOT_OK(status, debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); + WRITE_ENVOY_LOG_IF_STATUS_NOT_OK( + status, debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); // Drain the prepended ZooKeeper filter buffer. data.drain(zk_filter_buffer_len); @@ -800,7 +862,8 @@ absl::Status DecoderImpl::decodeAndBufferHelper(Buffer::Instance& data, DecodeTy while (offset < data_len) { // Peek packet length. len = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(len, fmt::format("peekInt32 for len: {}", len.status().message())); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + len, fmt::format("peekInt32 for len: {}", len.status().message())); status = ensureMinLength(len.value(), dtype == DecodeType::READ ? XID_LENGTH + INT_LENGTH @@ -894,7 +957,8 @@ absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr timeout = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(timeout, timeout.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(timeout, + timeout.status().message()); // Skip session id + password. offset += SESSION_LENGTH; @@ -902,7 +966,8 @@ absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr readonly = maybeReadBool(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, readonly.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, + readonly.status().message()); callbacks_.onConnectResponse(0, timeout.value(), readonly.value(), latency); @@ -916,10 +981,12 @@ absl::Status DecoderImpl::parseWatchEvent(Buffer::Instance& data, uint64_t& offs COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr event_type = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(event_type, event_type.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(event_type, + event_type.status().message()); const absl::StatusOr client_state = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(client_state, client_state.status().message()); + COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(client_state, + client_state.status().message()); const absl::StatusOr path = helper_.peekString(data, offset); COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.cc b/source/extensions/filters/network/zookeeper_proxy/utils.cc index c8980c6e3da1..b16a6cdc163c 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.cc +++ b/source/extensions/filters/network/zookeeper_proxy/utils.cc @@ -38,7 +38,8 @@ absl::StatusOr BufferHelper::peekBool(Buffer::Instance& buffer, uint64_t& absl::StatusOr BufferHelper::peekString(Buffer::Instance& buffer, uint64_t& offset) { std::string val; const absl::StatusOr len = peekInt32(buffer, offset); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(len, fmt::format("peekString: {}", len.status().message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(len, + fmt::format("peekString: {}", len.status().message())); if (len.value() == 0) { return val; diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index 75901eac203f..b98cbb22ea97 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -43,45 +43,46 @@ class BufferHelper : public Logger::Loggable { uint32_t current_{}; }; -#define ABSL_STATUS_RETURN_IF_STATUS_NOT_OK(status) \ - if (!status.ok()) { \ - return status; \ +#define ABSL_STATUS_RETURN_IF_STATUS_NOT_OK(status) \ + if (!status.ok()) { \ + return status; \ } -#define WRITE_ENVOY_LOG_IF_STATUS_NOT_OK(status, log_level, ...) \ - if (!status.ok()) { \ - ENVOY_LOG(log_level, ##__VA_ARGS__); \ +#define WRITE_ENVOY_LOG_IF_STATUS_NOT_OK(status, log_level, ...) \ + if (!status.ok()) { \ + ENVOY_LOG(log_level, ##__VA_ARGS__); \ } -#define COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status) \ - if (!status.ok()) { \ - callbacks_.onDecodeError(); \ - return status; \ +#define COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status) \ + if (!status.ok()) { \ + callbacks_.onDecodeError(); \ + return status; \ } -#define COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, log_level, ...) \ - if (!status.ok()) { \ - ENVOY_LOG(log_level, ##__VA_ARGS__); \ - callbacks_.onDecodeError(); \ - return status; \ +#define COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, log_level, ...) \ + if (!status.ok()) { \ + ENVOY_LOG(log_level, ##__VA_ARGS__); \ + callbacks_.onDecodeError(); \ + return status; \ } -#define RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ - if (!status.ok()) { \ - return absl::InvalidArgumentError(message); \ +#define RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ + if (!status.ok()) { \ + return absl::InvalidArgumentError(message); \ } -#define COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ - if (!status.ok()) { \ - callbacks_.onDecodeError(); \ - return absl::InvalidArgumentError(message); \ +#define COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ + if (!status.ok()) { \ + callbacks_.onDecodeError(); \ + return absl::InvalidArgumentError(message); \ } -#define COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message, log_level, ...) \ - if (!status.ok()) { \ - ENVOY_LOG(log_level, ##__VA_ARGS__); \ - callbacks_.onDecodeError(); \ - return absl::InvalidArgumentError(message); \ +#define COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message, \ + log_level, ...) \ + if (!status.ok()) { \ + ENVOY_LOG(log_level, ##__VA_ARGS__); \ + callbacks_.onDecodeError(); \ + return absl::InvalidArgumentError(message); \ } } // namespace ZooKeeperProxy } // namespace NetworkFilters From ee5df1248eeeacc4d8baf31cd4cfa8fece0cb5fa Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Sat, 11 Nov 2023 22:32:27 -0800 Subject: [PATCH 05/16] Rename Signed-off-by: Zhewei Hu --- .../network/zookeeper_proxy/decoder.cc | 230 +++++++++--------- .../filters/network/zookeeper_proxy/utils.h | 10 +- 2 files changed, 120 insertions(+), 120 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index fd84556de3b5..512003928a9f 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -44,23 +44,23 @@ const char* createFlagsToString(CreateFlags flags) { absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instance& data, uint64_t& offset) { - ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with {} bytes at offset {}", data.length(), + ENVOY_LOG(trace, "zookeeper_proxy: decoding request with {} bytes at offset {}", data.length(), offset); // Check message length. const absl::StatusOr len = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( len, fmt::format("peekInt32 for len: {}", len.status().message())); - ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with len {} at offset {}", len.value(), + ENVOY_LOG(trace, "zookeeper_proxy: decoding request with len {} at offset {}", len.value(), offset); absl::Status status = ensureMinLength(len.value(), XID_LENGTH + INT_LENGTH); // xid + opcode - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("ensureMinLength: {}", status.message())); status = ensureMaxLength(len.value()); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("ensureMaxLength: {}", status.message())); auto start_time = time_source_.monotonicTime(); @@ -76,10 +76,10 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // However, some client implementations might expose setWatches // as a regular data request, so we support that as well. const absl::StatusOr xid = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( xid, fmt::format("peerInt32 for xid: {}", xid.status().message())); - ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with xid {} at offset {}", xid.value(), + ENVOY_LOG(trace, "zookeeper_proxy: decoding request with xid {} at offset {}", xid.value(), offset); switch (static_cast(xid.value())) { @@ -123,10 +123,10 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // must happen every 1/3 of the negotiated session timeout, to keep // the session alive. const absl::StatusOr oc = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( oc, fmt::format("peekInt32 for opcode: {}", oc.status().message())); - ENVOY_LOG(trace, "[zookeeper_proxy] decoding request with opcode {} at offset {}", oc.value(), + ENVOY_LOG(trace, "zookeeper_proxy: decoding request with opcode {} at offset {}", oc.value(), offset); const auto opcode = static_cast(oc.value()); @@ -232,7 +232,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan callbacks_.onCloseRequest(); break; default: - ENVOY_LOG(debug, "[zookeeper_proxy] decodeOnData exception: unknown opcode {}", + ENVOY_LOG(debug, "zookeeper_proxy: decodeOnData exception: unknown opcode {}", enumToSignedInt(opcode)); callbacks_.onDecodeError(); return absl::nullopt; @@ -245,31 +245,31 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Instance& data, uint64_t& offset) { - ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with {} bytes at offset {}", data.length(), + ENVOY_LOG(trace, "zookeeper_proxy: decoding response with {} bytes at offset {}", data.length(), offset); // Check message length. const absl::StatusOr len = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( len, fmt::format("peekInt32 for len: {}", len.status().message())); - ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with len.value() {} at offset {}", + ENVOY_LOG(trace, "zookeeper_proxy: decoding response with len.value() {} at offset {}", len.value(), offset); absl::Status status = ensureMinLength(len.value(), XID_LENGTH + ZXID_LENGTH + INT_LENGTH); // xid + zxid + err - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("ensureMinLength: {}", status.message())); status = ensureMaxLength(len.value()); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("ensureMaxLength: {}", status.message())); const absl::StatusOr xid = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( xid, fmt::format("peekInt32 for xid: {}", xid.status().message())); - ENVOY_LOG(trace, "[zookeeper_proxy] decoding response with xid {} at offset {}", xid.value(), + ENVOY_LOG(trace, "zookeeper_proxy: decoding response with xid {} at offset {}", xid.value(), offset); const auto xid_code = static_cast(xid.value()); @@ -285,7 +285,7 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta ABSL_FALLTHROUGH_INTENDED; case XidCodes::SetWatchesXid: latency = fetchControlRequestData(xid.value(), opcode); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( latency, fmt::format("fetchControlRequestData: {}", latency.status().message())); break; case XidCodes::WatchXid: @@ -293,12 +293,12 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta break; default: latency = fetchDataRequestData(xid.value(), opcode); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( latency, fmt::format("fetchDataRequestData: {}", latency.status().message())); } // Connect responses are special, they have no full reply header - // but just an xid with no zxid nor error fields like the ones + // but just an XID with no zxid nor error fields like the ones // available for all other server generated messages. if (xid_code == XidCodes::ConnectXid) { status = parseConnectResponse(data, offset, len.value(), latency.value()); @@ -309,15 +309,15 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta // Control responses that aren't connect, with XIDs <= 0. const absl::StatusOr zxid = helper_.peekInt64(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( zxid, fmt::format("peekInt64 for zxid: {}", zxid.status().message())); const absl::StatusOr error = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( error, fmt::format("peekInt32 for error: {}", error.status().message())); ENVOY_LOG(trace, - "[zookeeper_proxy] decoding response with zxid.value() {} and error {} at offset {}", + "zookeeper_proxy: decoding response with zxid.value() {} and error {} at offset {}", zxid.value(), error.value(), offset); switch (xid_code) { @@ -365,17 +365,17 @@ absl::Status DecoderImpl::ensureMaxLength(const int32_t len) const { absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip zxid, timeout, and session id. offset += ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH; // Skip password. status = skipString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr readonly = maybeReadBool(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, readonly.status().message()); callbacks_.onConnect(readonly.value()); @@ -386,17 +386,17 @@ absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + INT_LENGTH + INT_LENGTH); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip opcode + type. offset += OPCODE_LENGTH + INT_LENGTH; const absl::StatusOr scheme = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, scheme.status().message()); // Skip credential. status = skipString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onAuthRequest(scheme.value()); @@ -406,13 +406,13 @@ absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& off absl::Status DecoderImpl::parseGetDataRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onGetDataRequest(path.value(), watch.value()); @@ -443,43 +443,43 @@ absl::Status DecoderImpl::skipAcls(Buffer::Instance& data, uint64_t& offset) { absl::Status DecoderImpl::parseCreateRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, OpCodes opcode) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (4 * INT_LENGTH)); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); // Skip data. status = skipString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); status = skipAcls(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); absl::StatusOr flag_data = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(flag_data, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(flag_data, flag_data.status().message()); const CreateFlags flags = static_cast(flag_data.value()); status = callbacks_.onCreateRequest(path.value(), flags, opcode); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); return absl::OkStatus(); } absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH)); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); // Skip data. status = skipString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore version. absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onSetRequest(path.value()); @@ -490,13 +490,13 @@ absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offs absl::Status DecoderImpl::parseGetChildrenRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, const bool two) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onGetChildrenRequest(path.value(), watch.value(), two); @@ -506,13 +506,13 @@ absl::Status DecoderImpl::parseGetChildrenRequest(Buffer::Instance& data, uint64 absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onDeleteRequest(path.value(), version.value()); @@ -523,13 +523,13 @@ absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseExistsRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onExistsRequest(path.value(), watch.value()); @@ -539,10 +539,10 @@ absl::Status DecoderImpl::parseExistsRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); callbacks_.onGetAclRequest(path.value()); @@ -552,16 +552,16 @@ absl::Status DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); status = skipAcls(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onSetAclRequest(path.value(), version.value()); @@ -572,8 +572,8 @@ absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& o absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); - COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - status, status.message(), debug, "[zookeeper_proxy] path only request decoding exception {}", + COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, status.message(), debug, "zookeeper_proxy: path only request decoding exception {}", status.message()); return helper_.peekString(data, offset); @@ -582,13 +582,13 @@ absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, absl::Status DecoderImpl::parseCheckRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onCheckRequest(path.value(), version.value()); @@ -600,19 +600,19 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of uint32_t len) { // Treat empty transactions as a decoding error, there should be at least 1 header. absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + MULTI_HEADER_LENGTH); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); while (true) { const absl::StatusOr opcode = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(opcode, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(opcode, opcode.status().message()); const absl::StatusOr done = helper_.peekBool(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(done, done.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(done, done.status().message()); // Ignore error field. const absl::StatusOr error = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, error.status().message()); if (done.value()) { @@ -622,26 +622,26 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of switch (static_cast(opcode.value())) { case OpCodes::Create: status = parseCreateRequest(data, offset, len, OpCodes::Create); - COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( - status, debug, "[zookeeper_proxy] multi request (create) decoding exception {}", + COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( + status, debug, "zookeeper_proxy: multi request (create) decoding exception {}", status.message()); break; case OpCodes::SetData: status = parseSetRequest(data, offset, len); - COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( - status, debug, "[zookeeper_proxy] multi request (set) decoding exception {}", + COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( + status, debug, "zookeeper_proxy: multi request (set) decoding exception {}", status.message()); break; case OpCodes::Check: status = parseCheckRequest(data, offset, len); - COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( - status, debug, "[zookeeper_proxy] multi request (check) decoding exception {}", + COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( + status, debug, "zookeeper_proxy: multi request (check) decoding exception {}", status.message()); break; case OpCodes::Delete: status = parseDeleteRequest(data, offset, len); - COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( - status, debug, "[zookeeper_proxy] multi request (delete) decoding exception {}", + COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( + status, debug, "zookeeper_proxy: multi request (delete) decoding exception {}", status.message()); break; default: @@ -660,22 +660,22 @@ absl::Status DecoderImpl::parseReconfigRequest(Buffer::Instance& data, uint64_t& uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH) + LONG_LENGTH); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip joining. status = skipString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip leaving. status = skipString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip new members. status = skipString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Read config id. absl::StatusOr config_id = helper_.peekInt64(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(config_id, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(config_id, config_id.status().message()); callbacks_.onReconfigRequest(); @@ -687,23 +687,23 @@ absl::Status DecoderImpl::parseSetWatchesRequest(Buffer::Instance& data, uint64_ uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (3 * INT_LENGTH)); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore relative Zxid. absl::StatusOr zxid = helper_.peekInt64(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); // Data watches. status = skipStrings(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Exist watches. status = skipStrings(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Child watches. status = skipStrings(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onSetWatchesRequest(); @@ -714,31 +714,31 @@ absl::Status DecoderImpl::parseSetWatches2Request(Buffer::Instance& data, uint64 uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (5 * INT_LENGTH)); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore relative Zxid. absl::StatusOr zxid = helper_.peekInt64(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); // Data watches. status = skipStrings(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Exist watches. status = skipStrings(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Child watches. status = skipStrings(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Persistent watches. status = skipStrings(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Persistent recursive watches. status = skipStrings(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onSetWatches2Request(); @@ -748,13 +748,13 @@ absl::Status DecoderImpl::parseSetWatches2Request(Buffer::Instance& data, uint64 absl::Status DecoderImpl::parseAddWatchRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr mode = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(mode, mode.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(mode, mode.status().message()); callbacks_.onAddWatchRequest(path.value(), mode.value()); @@ -764,13 +764,13 @@ absl::Status DecoderImpl::parseAddWatchRequest(Buffer::Instance& data, uint64_t& absl::Status DecoderImpl::parseXWatchesRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, OpCodes opcode) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch_type = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch_type, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch_type, watch_type.status().message()); if (opcode == OpCodes::CheckWatches) { @@ -789,7 +789,7 @@ absl::Status DecoderImpl::skipString(Buffer::Instance& data, uint64_t& offset) { if (slen.value() < 0) { ENVOY_LOG(trace, - "[zookeeper_proxy] decoding response with negative string length {} at offset {}", + "zookeeper_proxy: decoding response with negative string length {} at offset {}", slen.value(), offset); return absl::OkStatus(); } @@ -827,8 +827,8 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod if (zk_filter_buffer_len == 0) { status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); - WRITE_ENVOY_LOG_IF_STATUS_NOT_OK( - status, debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); + WRITE_LOG_IF_STATUS_NOT_OK( + status, debug, "zookeeper_proxy: decodeAndBufferHelper exception: {}", status.message()); return Network::FilterStatus::Continue; } @@ -839,8 +839,8 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod data.prepend(zk_filter_buffer); status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); - WRITE_ENVOY_LOG_IF_STATUS_NOT_OK( - status, debug, "[zookeeper_proxy] decodeAndBufferHelper exception: {}", status.message()); + WRITE_LOG_IF_STATUS_NOT_OK( + status, debug, "zookeeper_proxy: decodeAndBufferHelper exception: {}", status.message()); // Drain the prepended ZooKeeper filter buffer. data.drain(zk_filter_buffer_len); @@ -862,16 +862,16 @@ absl::Status DecoderImpl::decodeAndBufferHelper(Buffer::Instance& data, DecodeTy while (offset < data_len) { // Peek packet length. len = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( len, fmt::format("peekInt32 for len: {}", len.status().message())); status = ensureMinLength(len.value(), dtype == DecodeType::READ ? XID_LENGTH + INT_LENGTH : XID_LENGTH + ZXID_LENGTH + INT_LENGTH); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); status = ensureMaxLength(len.value()); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); offset += len.value(); if (offset <= data_len) { @@ -931,7 +931,7 @@ void DecoderImpl::decode(Buffer::Instance& data, DecodeType dtype, uint64_t full callbacks_.onRequestBytes(opcode.value(), offset - current); break; } else { - ENVOY_LOG(debug, "[zookeeper_proxy] decodeOnData exception: {}", opcode.status().message()); + ENVOY_LOG(debug, "zookeeper_proxy: decodeOnData exception: {}", opcode.status().message()); goto exit_decode_loop; } case DecodeType::WRITE: @@ -940,7 +940,7 @@ void DecoderImpl::decode(Buffer::Instance& data, DecodeType dtype, uint64_t full callbacks_.onResponseBytes(opcode.value(), offset - current); break; } else { - ENVOY_LOG(debug, "[zookeeper_proxy] decodeOnWrite exception: {}", + ENVOY_LOG(debug, "zookeeper_proxy: decodeOnWrite exception: {}", opcode.status().message()); goto exit_decode_loop; } @@ -954,19 +954,19 @@ absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& const std::chrono::milliseconds latency) { absl::Status status = ensureMinLength(len, PROTOCOL_VERSION_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr timeout = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(timeout, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(timeout, timeout.status().message()); // Skip session id + password. offset += SESSION_LENGTH; status = skipString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr readonly = maybeReadBool(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, readonly.status().message()); callbacks_.onConnectResponse(0, timeout.value(), readonly.value(), latency); @@ -978,18 +978,18 @@ absl::Status DecoderImpl::parseWatchEvent(Buffer::Instance& data, uint64_t& offs const uint32_t len, const int64_t zxid, const int32_t error) { absl::Status status = ensureMinLength(len, SERVER_HEADER_LENGTH + (3 * INT_LENGTH)); - COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr event_type = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(event_type, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(event_type, event_type.status().message()); const absl::StatusOr client_state = helper_.peekInt32(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(client_state, + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(client_state, client_state.status().message()); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); callbacks_.onWatchEvent(event_type.value(), client_state.value(), path.value(), zxid, error); diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index b98cbb22ea97..57ac36dad949 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -48,18 +48,18 @@ class BufferHelper : public Logger::Loggable { return status; \ } -#define WRITE_ENVOY_LOG_IF_STATUS_NOT_OK(status, log_level, ...) \ +#define WRITE_LOG_IF_STATUS_NOT_OK(status, log_level, ...) \ if (!status.ok()) { \ ENVOY_LOG(log_level, ##__VA_ARGS__); \ } -#define COUNT_DECODER_ERROR_AND_RETURN_IF_STATUS_NOT_OK(status) \ +#define COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status) \ if (!status.ok()) { \ callbacks_.onDecodeError(); \ return status; \ } -#define COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, log_level, ...) \ +#define COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, log_level, ...) \ if (!status.ok()) { \ ENVOY_LOG(log_level, ##__VA_ARGS__); \ callbacks_.onDecodeError(); \ @@ -71,13 +71,13 @@ class BufferHelper : public Logger::Loggable { return absl::InvalidArgumentError(message); \ } -#define COUNT_DECODER_ERROR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ +#define COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ if (!status.ok()) { \ callbacks_.onDecodeError(); \ return absl::InvalidArgumentError(message); \ } -#define COUNT_DECODER_ERROR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message, \ +#define COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message, \ log_level, ...) \ if (!status.ok()) { \ ENVOY_LOG(log_level, ##__VA_ARGS__); \ From 0b8a2dac225efbfd13cd0b6ac33ff21542343942 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Sat, 11 Nov 2023 23:01:45 -0800 Subject: [PATCH 06/16] Update comment Signed-off-by: Zhewei Hu --- source/extensions/filters/network/zookeeper_proxy/utils.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index 57ac36dad949..e184ec20dc62 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -19,8 +19,8 @@ namespace ZooKeeperProxy { /** * Helper for extracting ZooKeeper data from a buffer. * - * If at any point a peek is tried beyond max_len, an EnvoyException <---- reword this comment - * will be thrown. This is important to protect Envoy against malformed + * If at any point a peek is tried beyond max_len, an invalid argument error + * will be returned. This is important to protect Envoy against malformed * requests (e.g.: when the declared and actual length don't match). * * Note: ZooKeeper's protocol uses network byte ordering (big-endian). From 8c44bf176af7510f768dc68d3f55df74cbc2d97f Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Sat, 11 Nov 2023 23:23:02 -0800 Subject: [PATCH 07/16] Linting Signed-off-by: Zhewei Hu --- .../network/zookeeper_proxy/decoder.cc | 39 +++++++++---------- .../filters/network/zookeeper_proxy/utils.h | 2 +- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index 512003928a9f..d5085b16dffc 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -376,7 +376,7 @@ absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, const absl::StatusOr readonly = maybeReadBool(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, - readonly.status().message()); + readonly.status().message()); callbacks_.onConnect(readonly.value()); @@ -391,8 +391,7 @@ absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& off offset += OPCODE_LENGTH + INT_LENGTH; const absl::StatusOr scheme = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, - scheme.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, scheme.status().message()); // Skip credential. status = skipString(data, offset); @@ -457,7 +456,7 @@ absl::Status DecoderImpl::parseCreateRequest(Buffer::Instance& data, uint64_t& o absl::StatusOr flag_data = helper_.peekInt32(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(flag_data, - flag_data.status().message()); + flag_data.status().message()); const CreateFlags flags = static_cast(flag_data.value()); status = callbacks_.onCreateRequest(path.value(), flags, opcode); @@ -480,7 +479,7 @@ absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offs // Ignore version. absl::StatusOr version = helper_.peekInt32(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, - version.status().message()); + version.status().message()); callbacks_.onSetRequest(path.value()); @@ -513,7 +512,7 @@ absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& o const absl::StatusOr version = helper_.peekInt32(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, - version.status().message()); + version.status().message()); callbacks_.onDeleteRequest(path.value(), version.value()); @@ -562,7 +561,7 @@ absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& o const absl::StatusOr version = helper_.peekInt32(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, - version.status().message()); + version.status().message()); callbacks_.onSetAclRequest(path.value(), version.value()); @@ -589,7 +588,7 @@ absl::Status DecoderImpl::parseCheckRequest(Buffer::Instance& data, uint64_t& of const absl::StatusOr version = helper_.peekInt32(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, - version.status().message()); + version.status().message()); callbacks_.onCheckRequest(path.value(), version.value()); @@ -605,15 +604,14 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of while (true) { const absl::StatusOr opcode = helper_.peekInt32(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(opcode, - opcode.status().message()); + opcode.status().message()); const absl::StatusOr done = helper_.peekBool(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(done, done.status().message()); // Ignore error field. const absl::StatusOr error = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, - error.status().message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, error.status().message()); if (done.value()) { break; @@ -676,7 +674,7 @@ absl::Status DecoderImpl::parseReconfigRequest(Buffer::Instance& data, uint64_t& // Read config id. absl::StatusOr config_id = helper_.peekInt64(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(config_id, - config_id.status().message()); + config_id.status().message()); callbacks_.onReconfigRequest(); @@ -771,7 +769,7 @@ absl::Status DecoderImpl::parseXWatchesRequest(Buffer::Instance& data, uint64_t& const absl::StatusOr watch_type = helper_.peekInt32(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch_type, - watch_type.status().message()); + watch_type.status().message()); if (opcode == OpCodes::CheckWatches) { callbacks_.onCheckWatchesRequest(path.value(), watch_type.value()); @@ -839,8 +837,8 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod data.prepend(zk_filter_buffer); status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); - WRITE_LOG_IF_STATUS_NOT_OK( - status, debug, "zookeeper_proxy: decodeAndBufferHelper exception: {}", status.message()); + WRITE_LOG_IF_STATUS_NOT_OK(status, debug, "zookeeper_proxy: decodeAndBufferHelper exception: {}", + status.message()); // Drain the prepended ZooKeeper filter buffer. data.drain(zk_filter_buffer_len); @@ -940,8 +938,7 @@ void DecoderImpl::decode(Buffer::Instance& data, DecodeType dtype, uint64_t full callbacks_.onResponseBytes(opcode.value(), offset - current); break; } else { - ENVOY_LOG(debug, "zookeeper_proxy: decodeOnWrite exception: {}", - opcode.status().message()); + ENVOY_LOG(debug, "zookeeper_proxy: decodeOnWrite exception: {}", opcode.status().message()); goto exit_decode_loop; } } @@ -958,7 +955,7 @@ absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& const absl::StatusOr timeout = helper_.peekInt32(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(timeout, - timeout.status().message()); + timeout.status().message()); // Skip session id + password. offset += SESSION_LENGTH; @@ -967,7 +964,7 @@ absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& const absl::StatusOr readonly = maybeReadBool(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, - readonly.status().message()); + readonly.status().message()); callbacks_.onConnectResponse(0, timeout.value(), readonly.value(), latency); @@ -982,11 +979,11 @@ absl::Status DecoderImpl::parseWatchEvent(Buffer::Instance& data, uint64_t& offs const absl::StatusOr event_type = helper_.peekInt32(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(event_type, - event_type.status().message()); + event_type.status().message()); const absl::StatusOr client_state = helper_.peekInt32(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(client_state, - client_state.status().message()); + client_state.status().message()); const absl::StatusOr path = helper_.peekString(data, offset); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index e184ec20dc62..ec88406b5aaf 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -78,7 +78,7 @@ class BufferHelper : public Logger::Loggable { } #define COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message, \ - log_level, ...) \ + log_level, ...) \ if (!status.ok()) { \ ENVOY_LOG(log_level, ##__VA_ARGS__); \ callbacks_.onDecodeError(); \ From 560e263d389f91a9208ede2fad27443de6708b31 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Sun, 12 Nov 2023 11:11:31 -0800 Subject: [PATCH 08/16] Fix Signed-off-by: Zhewei Hu --- .../network/zookeeper_proxy/decoder.cc | 19 +++-- .../filters/network/zookeeper_proxy/decoder.h | 6 +- .../filters/network/zookeeper_proxy/filter.cc | 27 ++++--- .../filters/network/zookeeper_proxy/filter.h | 6 +- .../filters/network/zookeeper_proxy/utils.h | 8 -- .../network/zookeeper_proxy/filter_test.cc | 73 +++++++++++++++++++ 6 files changed, 106 insertions(+), 33 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index d5085b16dffc..e650a13518eb 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -180,7 +180,9 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan status, fmt::format("parseSetAclRequest: {}", status.message())); break; case OpCodes::Sync: - callbacks_.onSyncRequest(pathOnlyRequest(data, offset, len.value())); + status = callbacks_.onSyncRequest(pathOnlyRequest(data, offset, len.value())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("onSyncRequest: {}", status.message())); break; case OpCodes::Check: status = parseCheckRequest(data, offset, len.value()); @@ -223,10 +225,14 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan status, fmt::format("parseXWatchesRequest (remove watches): {}", status.message())); break; case OpCodes::GetEphemerals: - callbacks_.onGetEphemeralsRequest(pathOnlyRequest(data, offset, len.value())); + status = callbacks_.onGetEphemeralsRequest(pathOnlyRequest(data, offset, len.value())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("onGetEphemeralsRequest: {}", status.message())); break; case OpCodes::GetAllChildrenNumber: - callbacks_.onGetAllChildrenNumberRequest(pathOnlyRequest(data, offset, len.value())); + status = callbacks_.onGetAllChildrenNumberRequest(pathOnlyRequest(data, offset, len.value())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("onGetAllChildrenNumberRequest: {}", status.message())); break; case OpCodes::Close: callbacks_.onCloseRequest(); @@ -550,7 +556,7 @@ absl::Status DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { - absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); + absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH)); COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); @@ -571,9 +577,8 @@ absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& o absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); - COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - status, status.message(), debug, "zookeeper_proxy: path only request decoding exception {}", - status.message()); + COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + status, fmt::format("zookeeper_proxy: path only request decoding exception {}", status.message())); return helper_.peekString(data, offset); } diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.h b/source/extensions/filters/network/zookeeper_proxy/decoder.h index 8f20f8c6be7a..8718a8d9d6b1 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.h +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.h @@ -90,13 +90,13 @@ class DecoderCallbacks { OpCodes opcode) PURE; virtual void onSetRequest(const std::string& path) PURE; virtual void onGetChildrenRequest(const std::string& path, bool watch, bool v2) PURE; - virtual void onGetEphemeralsRequest(const absl::StatusOr& path) PURE; - virtual void onGetAllChildrenNumberRequest(const absl::StatusOr& path) PURE; + virtual absl::Status onGetEphemeralsRequest(const absl::StatusOr& path) PURE; + virtual absl::Status onGetAllChildrenNumberRequest(const absl::StatusOr& path) PURE; virtual void onDeleteRequest(const std::string& path, int32_t version) PURE; virtual void onExistsRequest(const std::string& path, bool watch) PURE; virtual void onGetAclRequest(const std::string& path) PURE; virtual void onSetAclRequest(const std::string& path, int32_t version) PURE; - virtual void onSyncRequest(const absl::StatusOr& path) PURE; + virtual absl::Status onSyncRequest(const absl::StatusOr& path) PURE; virtual void onCheckRequest(const std::string& path, int32_t version) PURE; virtual void onMultiRequest() PURE; virtual void onReconfigRequest() PURE; diff --git a/source/extensions/filters/network/zookeeper_proxy/filter.cc b/source/extensions/filters/network/zookeeper_proxy/filter.cc index 6b9842c1edb0..f7d942074c70 100644 --- a/source/extensions/filters/network/zookeeper_proxy/filter.cc +++ b/source/extensions/filters/network/zookeeper_proxy/filter.cc @@ -364,12 +364,13 @@ void ZooKeeperFilter::onSetAclRequest(const std::string& path, const int32_t ver setDynamicMetadata({{"opname", "setacl"}, {"path", path}, {"version", std::to_string(version)}}); } -void ZooKeeperFilter::onSyncRequest(const absl::StatusOr& path) { - if (!path.ok()) { - return; - } +absl::Status ZooKeeperFilter::onSyncRequest(const absl::StatusOr& path) { + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + config_->stats_.sync_rq_.inc(); setDynamicMetadata({{"opname", "sync"}, {"path", path.value()}}); + + return absl::OkStatus(); } void ZooKeeperFilter::onCheckRequest(const std::string&, const int32_t) { @@ -411,20 +412,22 @@ void ZooKeeperFilter::onAddWatchRequest(const std::string& path, const int32_t m setDynamicMetadata({{"opname", "addwatch"}, {"path", path}, {"mode", std::to_string(mode)}}); } -void ZooKeeperFilter::onGetEphemeralsRequest(const absl::StatusOr& path) { - if (!path.ok()) { - return; - } +absl::Status ZooKeeperFilter::onGetEphemeralsRequest(const absl::StatusOr& path) { + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + config_->stats_.getephemerals_rq_.inc(); setDynamicMetadata({{"opname", "getephemerals"}, {"path", path.value()}}); + + return absl::OkStatus(); } -void ZooKeeperFilter::onGetAllChildrenNumberRequest(const absl::StatusOr& path) { - if (!path.ok()) { - return; - } +absl::Status ZooKeeperFilter::onGetAllChildrenNumberRequest(const absl::StatusOr& path) { + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + config_->stats_.getallchildrennumber_rq_.inc(); setDynamicMetadata({{"opname", "getallchildrennumber"}, {"path", path.value()}}); + + return absl::OkStatus(); } void ZooKeeperFilter::onCloseRequest() { diff --git a/source/extensions/filters/network/zookeeper_proxy/filter.h b/source/extensions/filters/network/zookeeper_proxy/filter.h index 4c7acb19fc35..94f47506b211 100644 --- a/source/extensions/filters/network/zookeeper_proxy/filter.h +++ b/source/extensions/filters/network/zookeeper_proxy/filter.h @@ -349,7 +349,7 @@ class ZooKeeperFilter : public Network::Filter, void onExistsRequest(const std::string& path, bool watch) override; void onGetAclRequest(const std::string& path) override; void onSetAclRequest(const std::string& path, int32_t version) override; - void onSyncRequest(const absl::StatusOr& path) override; + absl::Status onSyncRequest(const absl::StatusOr& path) override; void onCheckRequest(const std::string& path, int32_t version) override; void onMultiRequest() override; void onReconfigRequest() override; @@ -358,8 +358,8 @@ class ZooKeeperFilter : public Network::Filter, void onAddWatchRequest(const std::string& path, const int32_t mode) override; void onCheckWatchesRequest(const std::string& path, int32_t type) override; void onRemoveWatchesRequest(const std::string& path, int32_t type) override; - void onGetEphemeralsRequest(const absl::StatusOr& path) override; - void onGetAllChildrenNumberRequest(const absl::StatusOr& path) override; + absl::Status onGetEphemeralsRequest(const absl::StatusOr& path) override; + absl::Status onGetAllChildrenNumberRequest(const absl::StatusOr& path) override; void onCloseRequest() override; void onResponseBytes(const absl::optional opcode, const uint64_t bytes) override; void onConnectResponse(int32_t proto_version, int32_t timeout, bool readonly, diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index ec88406b5aaf..faa61434173c 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -76,14 +76,6 @@ class BufferHelper : public Logger::Loggable { callbacks_.onDecodeError(); \ return absl::InvalidArgumentError(message); \ } - -#define COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message, \ - log_level, ...) \ - if (!status.ok()) { \ - ENVOY_LOG(log_level, ##__VA_ARGS__); \ - callbacks_.onDecodeError(); \ - return absl::InvalidArgumentError(message); \ - } } // namespace ZooKeeperProxy } // namespace NetworkFilters } // namespace Extensions diff --git a/test/extensions/filters/network/zookeeper_proxy/filter_test.cc b/test/extensions/filters/network/zookeeper_proxy/filter_test.cc index f22f9f14cee4..ffd653e69d73 100644 --- a/test/extensions/filters/network/zookeeper_proxy/filter_test.cc +++ b/test/extensions/filters/network/zookeeper_proxy/filter_test.cc @@ -271,6 +271,19 @@ class ZooKeeperFilterTest : public testing::Test { return buffer; } + Buffer::OwnedImpl encodeTooShortPath(const std::string& path, const int32_t opcode) const { + Buffer::OwnedImpl buffer; + + buffer.writeBEInt(1); + buffer.writeBEInt(1000); + // Opcode. + buffer.writeBEInt(opcode); + // Path. + addString(buffer, path); + + return buffer; + } + Buffer::OwnedImpl encodePathLongerThanBuffer(const std::string& path, const int32_t opcode) const { Buffer::OwnedImpl buffer; @@ -1190,6 +1203,19 @@ TEST_F(ZooKeeperFilterTest, SetAclRequest) { testResponse({{{"opname", "setacl_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); } +TEST_F(ZooKeeperFilterTest, SetAclTooBigRequest) { + initialize(); + + std::string schema = std::string(1048544, '*') + Buffer::OwnedImpl data = encodeSetAclRequest("/foo", schema, "passwd", -1); + + EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.setacl_rq").value()); + EXPECT_EQ(0, config_->stats().request_bytes_.value()); + EXPECT_EQ(0, store_.counter("test.zookeeper.setacl_rq_bytes")).value()); + EXPECT_EQ(1UL, config_->stats().decoder_error_.value()); +} + TEST_F(ZooKeeperFilterTest, SyncRequest) { initialize(); @@ -1199,6 +1225,20 @@ TEST_F(ZooKeeperFilterTest, SyncRequest) { testResponse({{{"opname", "sync_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); } +TEST_F(ZooKeeperFilterTest, TooSmallSyncRequest) { + initialize(); + + Buffer::OwnedImpl data = encodeTooShortPath("/foo", enumToSignedInt(OpCodes::Sync)); + + // testRequest(data, {{{"opname", "sync"}, {"path", "/foo"}}, {{"bytes", "20"}}}); + + EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.sync_rq").value()); + EXPECT_EQ(0, config_->stats().request_bytes_.value()); + EXPECT_EQ(0, store_.counter("test.zookeeper.sync_rq_bytes")).value(); + EXPECT_EQ(1UL, config_->stats().decoder_error_.value()); +} + TEST_F(ZooKeeperFilterTest, GetEphemeralsRequest) { initialize(); @@ -1270,6 +1310,39 @@ TEST_F(ZooKeeperFilterTest, MultiRequest) { testResponse({{{"opname", "multi_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); } +TEST_F(ZooKeeperFilterTest, MultiRequestWithBadCreateRequest) { + initialize(); + + Buffer::OwnedImpl create1 = encodeCreateRequest("/foo", "1", 0, true); + Buffer::OwnedImpl create2 = encodeCreateRequestWithPartialData("/foo", "bar", true); + Buffer::OwnedImpl create3 = encodeCreateRequestWithNegativeDataLen("/baz", 0, true); + Buffer::OwnedImpl check1 = encodePathVersion("/foo", 100, enumToSignedInt(OpCodes::Check), true); + Buffer::OwnedImpl set1 = encodeSetRequest("/bar", "2", -1, true); + Buffer::OwnedImpl delete1 = encodeDeleteRequest("/abcd", 1, true); + Buffer::OwnedImpl delete2 = encodeDeleteRequest("/efg", 2, true); + + std::vector> ops; + ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Create), std::move(create1))); + ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Create), std::move(create2))); + ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Create), std::move(create3))); + ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Check), std::move(check1))); + ops.push_back(std::make_pair(enumToSignedInt(OpCodes::SetData), std::move(set1))); + ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Delete), std::move(delete1))); + ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Delete), std::move(delete2))); + + Buffer::OwnedImpl data = encodeMultiRequest(ops); + + EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); + EXPECT_EQ(0UL, config_->stats().multi_rq_.value()); + EXPECT_EQ(0UL, config_->stats().request_bytes_.value()); + EXPECT_EQ(0UL, store_.counter("test.zookeeper.multi_rq_bytes").value()); + EXPECT_EQ(0UL, config_->stats().create_rq_.value()); + EXPECT_EQ(0UL, config_->stats().setdata_rq_.value()); + EXPECT_EQ(0UL, config_->stats().check_rq_.value()); + EXPECT_EQ(0UL, config_->stats().delete_rq_.value()); + EXPECT_EQ(1UL, config_->stats().decoder_error_.value()); +} + TEST_F(ZooKeeperFilterTest, ReconfigRequest) { initialize(); From cafb63a76a5b1c72bdcc1094a886b0c69e58f8b0 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Sun, 12 Nov 2023 11:18:06 -0800 Subject: [PATCH 09/16] Linting Signed-off-by: Zhewei Hu --- .../extensions/filters/network/zookeeper_proxy/decoder.cc | 7 ++++--- .../extensions/filters/network/zookeeper_proxy/filter.cc | 3 ++- .../filters/network/zookeeper_proxy/filter_test.cc | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index e650a13518eb..fa5876925bd3 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -181,8 +181,8 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan break; case OpCodes::Sync: status = callbacks_.onSyncRequest(pathOnlyRequest(data, offset, len.value())); - RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - status, fmt::format("onSyncRequest: {}", status.message())); + RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, + fmt::format("onSyncRequest: {}", status.message())); break; case OpCodes::Check: status = parseCheckRequest(data, offset, len.value()); @@ -578,7 +578,8 @@ absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( - status, fmt::format("zookeeper_proxy: path only request decoding exception {}", status.message())); + status, + fmt::format("zookeeper_proxy: path only request decoding exception {}", status.message())); return helper_.peekString(data, offset); } diff --git a/source/extensions/filters/network/zookeeper_proxy/filter.cc b/source/extensions/filters/network/zookeeper_proxy/filter.cc index f7d942074c70..817fa14077d4 100644 --- a/source/extensions/filters/network/zookeeper_proxy/filter.cc +++ b/source/extensions/filters/network/zookeeper_proxy/filter.cc @@ -421,7 +421,8 @@ absl::Status ZooKeeperFilter::onGetEphemeralsRequest(const absl::StatusOr& path) { +absl::Status +ZooKeeperFilter::onGetAllChildrenNumberRequest(const absl::StatusOr& path) { RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); config_->stats_.getallchildrennumber_rq_.inc(); diff --git a/test/extensions/filters/network/zookeeper_proxy/filter_test.cc b/test/extensions/filters/network/zookeeper_proxy/filter_test.cc index ffd653e69d73..70843ae1a136 100644 --- a/test/extensions/filters/network/zookeeper_proxy/filter_test.cc +++ b/test/extensions/filters/network/zookeeper_proxy/filter_test.cc @@ -1206,13 +1206,13 @@ TEST_F(ZooKeeperFilterTest, SetAclRequest) { TEST_F(ZooKeeperFilterTest, SetAclTooBigRequest) { initialize(); - std::string schema = std::string(1048544, '*') + std::string schema = std::string(1048544, '*'); Buffer::OwnedImpl data = encodeSetAclRequest("/foo", schema, "passwd", -1); EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(0UL, store_.counter("test.zookeeper.setacl_rq").value()); EXPECT_EQ(0, config_->stats().request_bytes_.value()); - EXPECT_EQ(0, store_.counter("test.zookeeper.setacl_rq_bytes")).value()); + EXPECT_EQ(0, store_.counter("test.zookeeper.setacl_rq_bytes").value()); EXPECT_EQ(1UL, config_->stats().decoder_error_.value()); } From 3ada8cebaba70da3d8b2a9897363f7060a8d1d62 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Sun, 12 Nov 2023 11:41:18 -0800 Subject: [PATCH 10/16] Fix Signed-off-by: Zhewei Hu --- .../network/zookeeper_proxy/filter_test.cc | 37 +------------------ 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/test/extensions/filters/network/zookeeper_proxy/filter_test.cc b/test/extensions/filters/network/zookeeper_proxy/filter_test.cc index 70843ae1a136..96567f7e52ec 100644 --- a/test/extensions/filters/network/zookeeper_proxy/filter_test.cc +++ b/test/extensions/filters/network/zookeeper_proxy/filter_test.cc @@ -1230,12 +1230,10 @@ TEST_F(ZooKeeperFilterTest, TooSmallSyncRequest) { Buffer::OwnedImpl data = encodeTooShortPath("/foo", enumToSignedInt(OpCodes::Sync)); - // testRequest(data, {{{"opname", "sync"}, {"path", "/foo"}}, {{"bytes", "20"}}}); - EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); EXPECT_EQ(0UL, store_.counter("test.zookeeper.sync_rq").value()); EXPECT_EQ(0, config_->stats().request_bytes_.value()); - EXPECT_EQ(0, store_.counter("test.zookeeper.sync_rq_bytes")).value(); + EXPECT_EQ(0, store_.counter("test.zookeeper.sync_rq_bytes").value()); EXPECT_EQ(1UL, config_->stats().decoder_error_.value()); } @@ -1310,39 +1308,6 @@ TEST_F(ZooKeeperFilterTest, MultiRequest) { testResponse({{{"opname", "multi_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); } -TEST_F(ZooKeeperFilterTest, MultiRequestWithBadCreateRequest) { - initialize(); - - Buffer::OwnedImpl create1 = encodeCreateRequest("/foo", "1", 0, true); - Buffer::OwnedImpl create2 = encodeCreateRequestWithPartialData("/foo", "bar", true); - Buffer::OwnedImpl create3 = encodeCreateRequestWithNegativeDataLen("/baz", 0, true); - Buffer::OwnedImpl check1 = encodePathVersion("/foo", 100, enumToSignedInt(OpCodes::Check), true); - Buffer::OwnedImpl set1 = encodeSetRequest("/bar", "2", -1, true); - Buffer::OwnedImpl delete1 = encodeDeleteRequest("/abcd", 1, true); - Buffer::OwnedImpl delete2 = encodeDeleteRequest("/efg", 2, true); - - std::vector> ops; - ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Create), std::move(create1))); - ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Create), std::move(create2))); - ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Create), std::move(create3))); - ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Check), std::move(check1))); - ops.push_back(std::make_pair(enumToSignedInt(OpCodes::SetData), std::move(set1))); - ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Delete), std::move(delete1))); - ops.push_back(std::make_pair(enumToSignedInt(OpCodes::Delete), std::move(delete2))); - - Buffer::OwnedImpl data = encodeMultiRequest(ops); - - EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); - EXPECT_EQ(0UL, config_->stats().multi_rq_.value()); - EXPECT_EQ(0UL, config_->stats().request_bytes_.value()); - EXPECT_EQ(0UL, store_.counter("test.zookeeper.multi_rq_bytes").value()); - EXPECT_EQ(0UL, config_->stats().create_rq_.value()); - EXPECT_EQ(0UL, config_->stats().setdata_rq_.value()); - EXPECT_EQ(0UL, config_->stats().check_rq_.value()); - EXPECT_EQ(0UL, config_->stats().delete_rq_.value()); - EXPECT_EQ(1UL, config_->stats().decoder_error_.value()); -} - TEST_F(ZooKeeperFilterTest, ReconfigRequest) { initialize(); From acdf2b0b3c8b97dacfe337be6f957df3872cd36a Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Mon, 13 Nov 2023 08:57:21 -0800 Subject: [PATCH 11/16] Update Signed-off-by: Zhewei Hu --- .../network/zookeeper_proxy/filter_test.cc | 38 ------------------- 1 file changed, 38 deletions(-) diff --git a/test/extensions/filters/network/zookeeper_proxy/filter_test.cc b/test/extensions/filters/network/zookeeper_proxy/filter_test.cc index 96567f7e52ec..f22f9f14cee4 100644 --- a/test/extensions/filters/network/zookeeper_proxy/filter_test.cc +++ b/test/extensions/filters/network/zookeeper_proxy/filter_test.cc @@ -271,19 +271,6 @@ class ZooKeeperFilterTest : public testing::Test { return buffer; } - Buffer::OwnedImpl encodeTooShortPath(const std::string& path, const int32_t opcode) const { - Buffer::OwnedImpl buffer; - - buffer.writeBEInt(1); - buffer.writeBEInt(1000); - // Opcode. - buffer.writeBEInt(opcode); - // Path. - addString(buffer, path); - - return buffer; - } - Buffer::OwnedImpl encodePathLongerThanBuffer(const std::string& path, const int32_t opcode) const { Buffer::OwnedImpl buffer; @@ -1203,19 +1190,6 @@ TEST_F(ZooKeeperFilterTest, SetAclRequest) { testResponse({{{"opname", "setacl_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); } -TEST_F(ZooKeeperFilterTest, SetAclTooBigRequest) { - initialize(); - - std::string schema = std::string(1048544, '*'); - Buffer::OwnedImpl data = encodeSetAclRequest("/foo", schema, "passwd", -1); - - EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); - EXPECT_EQ(0UL, store_.counter("test.zookeeper.setacl_rq").value()); - EXPECT_EQ(0, config_->stats().request_bytes_.value()); - EXPECT_EQ(0, store_.counter("test.zookeeper.setacl_rq_bytes").value()); - EXPECT_EQ(1UL, config_->stats().decoder_error_.value()); -} - TEST_F(ZooKeeperFilterTest, SyncRequest) { initialize(); @@ -1225,18 +1199,6 @@ TEST_F(ZooKeeperFilterTest, SyncRequest) { testResponse({{{"opname", "sync_resp"}, {"zxid", "2000"}, {"error", "0"}}, {{"bytes", "20"}}}); } -TEST_F(ZooKeeperFilterTest, TooSmallSyncRequest) { - initialize(); - - Buffer::OwnedImpl data = encodeTooShortPath("/foo", enumToSignedInt(OpCodes::Sync)); - - EXPECT_EQ(Envoy::Network::FilterStatus::Continue, filter_->onData(data, false)); - EXPECT_EQ(0UL, store_.counter("test.zookeeper.sync_rq").value()); - EXPECT_EQ(0, config_->stats().request_bytes_.value()); - EXPECT_EQ(0, store_.counter("test.zookeeper.sync_rq_bytes").value()); - EXPECT_EQ(1UL, config_->stats().decoder_error_.value()); -} - TEST_F(ZooKeeperFilterTest, GetEphemeralsRequest) { initialize(); From f44688d147fa1883a21e8e660886cb2a323d0953 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Tue, 14 Nov 2023 21:06:03 -0800 Subject: [PATCH 12/16] goto -> return Signed-off-by: Zhewei Hu --- source/extensions/filters/network/zookeeper_proxy/decoder.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index fa5876925bd3..e301ee2f43ff 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -936,7 +936,7 @@ void DecoderImpl::decode(Buffer::Instance& data, DecodeType dtype, uint64_t full break; } else { ENVOY_LOG(debug, "zookeeper_proxy: decodeOnData exception: {}", opcode.status().message()); - goto exit_decode_loop; + return; } case DecodeType::WRITE: opcode = decodeOnWrite(data, offset); @@ -945,11 +945,10 @@ void DecoderImpl::decode(Buffer::Instance& data, DecodeType dtype, uint64_t full break; } else { ENVOY_LOG(debug, "zookeeper_proxy: decodeOnWrite exception: {}", opcode.status().message()); - goto exit_decode_loop; + return; } } } -exit_decode_loop:; } absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& offset, From 21f634b75d6e3de64a296793336acfb16eba35f7 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Wed, 15 Nov 2023 22:41:52 -0800 Subject: [PATCH 13/16] Remove one marco Signed-off-by: Zhewei Hu --- .../filters/network/zookeeper_proxy/decoder.cc | 10 ++++++---- .../extensions/filters/network/zookeeper_proxy/utils.h | 5 ----- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index e301ee2f43ff..7e97acedb141 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -831,8 +831,9 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod if (zk_filter_buffer_len == 0) { status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); - WRITE_LOG_IF_STATUS_NOT_OK( - status, debug, "zookeeper_proxy: decodeAndBufferHelper exception: {}", status.message()); + if (!status.ok()) { + ENVOY_LOG(debug, "zookeeper_proxy: decodeAndBufferHelper exception: {}", status.message()); + } return Network::FilterStatus::Continue; } @@ -843,8 +844,9 @@ Network::FilterStatus DecoderImpl::decodeAndBuffer(Buffer::Instance& data, Decod data.prepend(zk_filter_buffer); status = decodeAndBufferHelper(data, dtype, zk_filter_buffer); - WRITE_LOG_IF_STATUS_NOT_OK(status, debug, "zookeeper_proxy: decodeAndBufferHelper exception: {}", - status.message()); + if (!status.ok()) { + ENVOY_LOG(debug, "zookeeper_proxy: decodeAndBufferHelper exception: {}", status.message()); + } // Drain the prepended ZooKeeper filter buffer. data.drain(zk_filter_buffer_len); diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index faa61434173c..b542ae7f0694 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -48,11 +48,6 @@ class BufferHelper : public Logger::Loggable { return status; \ } -#define WRITE_LOG_IF_STATUS_NOT_OK(status, log_level, ...) \ - if (!status.ok()) { \ - ENVOY_LOG(log_level, ##__VA_ARGS__); \ - } - #define COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status) \ if (!status.ok()) { \ callbacks_.onDecodeError(); \ From 490f892345fd6c10e795c181778a9078b8fc7f65 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Fri, 17 Nov 2023 09:05:47 -0800 Subject: [PATCH 14/16] Remove one more marco Signed-off-by: Zhewei Hu --- .../filters/network/zookeeper_proxy/decoder.cc | 16 ++++------------ .../filters/network/zookeeper_proxy/utils.h | 7 ------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index 7e97acedb141..ed7bb0181664 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -626,27 +626,19 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of switch (static_cast(opcode.value())) { case OpCodes::Create: status = parseCreateRequest(data, offset, len, OpCodes::Create); - COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( - status, debug, "zookeeper_proxy: multi request (create) decoding exception {}", - status.message()); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; case OpCodes::SetData: status = parseSetRequest(data, offset, len); - COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( - status, debug, "zookeeper_proxy: multi request (set) decoding exception {}", - status.message()); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; case OpCodes::Check: status = parseCheckRequest(data, offset, len); - COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( - status, debug, "zookeeper_proxy: multi request (check) decoding exception {}", - status.message()); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; case OpCodes::Delete: status = parseDeleteRequest(data, offset, len); - COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK( - status, debug, "zookeeper_proxy: multi request (delete) decoding exception {}", - status.message()); + COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; default: callbacks_.onDecodeError(); diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index b542ae7f0694..535cbe1f70bb 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -54,13 +54,6 @@ class BufferHelper : public Logger::Loggable { return status; \ } -#define COUNT_DECODER_ERR_WITH_LOG_AND_RETURN_IF_STATUS_NOT_OK(status, log_level, ...) \ - if (!status.ok()) { \ - ENVOY_LOG(log_level, ##__VA_ARGS__); \ - callbacks_.onDecodeError(); \ - return status; \ - } - #define RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ if (!status.ok()) { \ return absl::InvalidArgumentError(message); \ From fc59e2232dbff78efd27be159e9f3511d6775d48 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Sun, 19 Nov 2023 12:05:11 -0800 Subject: [PATCH 15/16] Address comments Signed-off-by: Zhewei Hu --- .../filters/network/zookeeper_proxy/decoder.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index ed7bb0181664..e5834f9ced9c 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -928,19 +928,17 @@ void DecoderImpl::decode(Buffer::Instance& data, DecodeType dtype, uint64_t full if (opcode.ok()) { callbacks_.onRequestBytes(opcode.value(), offset - current); break; - } else { - ENVOY_LOG(debug, "zookeeper_proxy: decodeOnData exception: {}", opcode.status().message()); - return; } + ENVOY_LOG(debug, "zookeeper_proxy: decodeOnData exception: {}", opcode.status().message()); + return; case DecodeType::WRITE: opcode = decodeOnWrite(data, offset); if (opcode.ok()) { callbacks_.onResponseBytes(opcode.value(), offset - current); break; - } else { - ENVOY_LOG(debug, "zookeeper_proxy: decodeOnWrite exception: {}", opcode.status().message()); - return; } + ENVOY_LOG(debug, "zookeeper_proxy: decodeOnWrite exception: {}", opcode.status().message()); + return; } } } From 9023480df26059178ecc844edcddcb8cf8022114 Mon Sep 17 00:00:00 2001 From: Zhewei Hu Date: Sun, 19 Nov 2023 12:12:19 -0800 Subject: [PATCH 16/16] Rename marcos Signed-off-by: Zhewei Hu --- .../network/zookeeper_proxy/decoder.cc | 206 +++++++++--------- .../filters/network/zookeeper_proxy/utils.h | 4 +- 2 files changed, 102 insertions(+), 108 deletions(-) diff --git a/source/extensions/filters/network/zookeeper_proxy/decoder.cc b/source/extensions/filters/network/zookeeper_proxy/decoder.cc index e5834f9ced9c..92367e55063a 100644 --- a/source/extensions/filters/network/zookeeper_proxy/decoder.cc +++ b/source/extensions/filters/network/zookeeper_proxy/decoder.cc @@ -49,18 +49,18 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // Check message length. const absl::StatusOr len = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( len, fmt::format("peekInt32 for len: {}", len.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding request with len {} at offset {}", len.value(), offset); absl::Status status = ensureMinLength(len.value(), XID_LENGTH + INT_LENGTH); // xid + opcode - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("ensureMinLength: {}", status.message())); status = ensureMaxLength(len.value()); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("ensureMaxLength: {}", status.message())); auto start_time = time_source_.monotonicTime(); @@ -76,7 +76,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // However, some client implementations might expose setWatches // as a regular data request, so we support that as well. const absl::StatusOr xid = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( xid, fmt::format("peerInt32 for xid: {}", xid.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding request with xid {} at offset {}", xid.value(), @@ -123,7 +123,7 @@ absl::StatusOr> DecoderImpl::decodeOnData(Buffer::Instan // must happen every 1/3 of the negotiated session timeout, to keep // the session alive. const absl::StatusOr oc = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( oc, fmt::format("peekInt32 for opcode: {}", oc.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding request with opcode {} at offset {}", oc.value(), @@ -256,7 +256,7 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta // Check message length. const absl::StatusOr len = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( len, fmt::format("peekInt32 for len: {}", len.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding response with len.value() {} at offset {}", @@ -264,15 +264,15 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta absl::Status status = ensureMinLength(len.value(), XID_LENGTH + ZXID_LENGTH + INT_LENGTH); // xid + zxid + err - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("ensureMinLength: {}", status.message())); status = ensureMaxLength(len.value()); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("ensureMaxLength: {}", status.message())); const absl::StatusOr xid = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( xid, fmt::format("peekInt32 for xid: {}", xid.status().message())); ENVOY_LOG(trace, "zookeeper_proxy: decoding response with xid {} at offset {}", xid.value(), @@ -291,7 +291,7 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta ABSL_FALLTHROUGH_INTENDED; case XidCodes::SetWatchesXid: latency = fetchControlRequestData(xid.value(), opcode); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( latency, fmt::format("fetchControlRequestData: {}", latency.status().message())); break; case XidCodes::WatchXid: @@ -299,7 +299,7 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta break; default: latency = fetchDataRequestData(xid.value(), opcode); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( latency, fmt::format("fetchDataRequestData: {}", latency.status().message())); } @@ -315,11 +315,11 @@ absl::StatusOr> DecoderImpl::decodeOnWrite(Buffer::Insta // Control responses that aren't connect, with XIDs <= 0. const absl::StatusOr zxid = helper_.peekInt64(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( zxid, fmt::format("peekInt64 for zxid: {}", zxid.status().message())); const absl::StatusOr error = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( error, fmt::format("peekInt32 for error: {}", error.status().message())); ENVOY_LOG(trace, @@ -371,18 +371,18 @@ absl::Status DecoderImpl::ensureMaxLength(const int32_t len) const { absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip zxid, timeout, and session id. offset += ZXID_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH; // Skip password. status = skipString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr readonly = maybeReadBool(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, - readonly.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, + readonly.status().message()); callbacks_.onConnect(readonly.value()); @@ -392,16 +392,16 @@ absl::Status DecoderImpl::parseConnect(Buffer::Instance& data, uint64_t& offset, absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + INT_LENGTH + INT_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip opcode + type. offset += OPCODE_LENGTH + INT_LENGTH; const absl::StatusOr scheme = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, scheme.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(scheme, scheme.status().message()); // Skip credential. status = skipString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onAuthRequest(scheme.value()); @@ -411,13 +411,13 @@ absl::Status DecoderImpl::parseAuthRequest(Buffer::Instance& data, uint64_t& off absl::Status DecoderImpl::parseGetDataRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onGetDataRequest(path.value(), watch.value()); @@ -448,44 +448,43 @@ absl::Status DecoderImpl::skipAcls(Buffer::Instance& data, uint64_t& offset) { absl::Status DecoderImpl::parseCreateRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, OpCodes opcode) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (4 * INT_LENGTH)); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); // Skip data. status = skipString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); status = skipAcls(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); absl::StatusOr flag_data = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(flag_data, - flag_data.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(flag_data, + flag_data.status().message()); const CreateFlags flags = static_cast(flag_data.value()); status = callbacks_.onCreateRequest(path.value(), flags, opcode); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); return absl::OkStatus(); } absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH)); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); // Skip data. status = skipString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore version. absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, - version.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onSetRequest(path.value()); @@ -495,13 +494,13 @@ absl::Status DecoderImpl::parseSetRequest(Buffer::Instance& data, uint64_t& offs absl::Status DecoderImpl::parseGetChildrenRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, const bool two) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onGetChildrenRequest(path.value(), watch.value(), two); @@ -511,14 +510,13 @@ absl::Status DecoderImpl::parseGetChildrenRequest(Buffer::Instance& data, uint64 absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, - version.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onDeleteRequest(path.value(), version.value()); @@ -528,13 +526,13 @@ absl::Status DecoderImpl::parseDeleteRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseExistsRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH + BOOL_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch = helper_.peekBool(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch, watch.status().message()); callbacks_.onExistsRequest(path.value(), watch.value()); @@ -544,10 +542,10 @@ absl::Status DecoderImpl::parseExistsRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); callbacks_.onGetAclRequest(path.value()); @@ -557,17 +555,16 @@ absl::Status DecoderImpl::parseGetAclRequest(Buffer::Instance& data, uint64_t& o absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH)); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); status = skipAcls(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, - version.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onSetAclRequest(path.value(), version.value()); @@ -577,7 +574,7 @@ absl::Status DecoderImpl::parseSetAclRequest(Buffer::Instance& data, uint64_t& o absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + INT_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( status, fmt::format("zookeeper_proxy: path only request decoding exception {}", status.message())); @@ -587,14 +584,13 @@ absl::StatusOr DecoderImpl::pathOnlyRequest(Buffer::Instance& data, absl::Status DecoderImpl::parseCheckRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr version = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, - version.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(version, version.status().message()); callbacks_.onCheckRequest(path.value(), version.value()); @@ -605,19 +601,18 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of uint32_t len) { // Treat empty transactions as a decoding error, there should be at least 1 header. absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + MULTI_HEADER_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); while (true) { const absl::StatusOr opcode = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(opcode, - opcode.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(opcode, opcode.status().message()); const absl::StatusOr done = helper_.peekBool(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(done, done.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(done, done.status().message()); // Ignore error field. const absl::StatusOr error = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, error.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(error, error.status().message()); if (done.value()) { break; @@ -626,19 +621,19 @@ absl::Status DecoderImpl::parseMultiRequest(Buffer::Instance& data, uint64_t& of switch (static_cast(opcode.value())) { case OpCodes::Create: status = parseCreateRequest(data, offset, len, OpCodes::Create); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; case OpCodes::SetData: status = parseSetRequest(data, offset, len); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; case OpCodes::Check: status = parseCheckRequest(data, offset, len); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; case OpCodes::Delete: status = parseDeleteRequest(data, offset, len); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); break; default: callbacks_.onDecodeError(); @@ -656,23 +651,23 @@ absl::Status DecoderImpl::parseReconfigRequest(Buffer::Instance& data, uint64_t& uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (3 * INT_LENGTH) + LONG_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip joining. status = skipString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip leaving. status = skipString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Skip new members. status = skipString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Read config id. absl::StatusOr config_id = helper_.peekInt64(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(config_id, - config_id.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(config_id, + config_id.status().message()); callbacks_.onReconfigRequest(); @@ -683,23 +678,23 @@ absl::Status DecoderImpl::parseSetWatchesRequest(Buffer::Instance& data, uint64_ uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (3 * INT_LENGTH)); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore relative Zxid. absl::StatusOr zxid = helper_.peekInt64(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); // Data watches. status = skipStrings(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Exist watches. status = skipStrings(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Child watches. status = skipStrings(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onSetWatchesRequest(); @@ -710,31 +705,31 @@ absl::Status DecoderImpl::parseSetWatches2Request(Buffer::Instance& data, uint64 uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + LONG_LENGTH + (5 * INT_LENGTH)); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Ignore relative Zxid. absl::StatusOr zxid = helper_.peekInt64(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(zxid, zxid.status().message()); // Data watches. status = skipStrings(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Exist watches. status = skipStrings(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Child watches. status = skipStrings(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Persistent watches. status = skipStrings(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); // Persistent recursive watches. status = skipStrings(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); callbacks_.onSetWatches2Request(); @@ -744,13 +739,13 @@ absl::Status DecoderImpl::parseSetWatches2Request(Buffer::Instance& data, uint64 absl::Status DecoderImpl::parseAddWatchRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr mode = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(mode, mode.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(mode, mode.status().message()); callbacks_.onAddWatchRequest(path.value(), mode.value()); @@ -760,14 +755,14 @@ absl::Status DecoderImpl::parseAddWatchRequest(Buffer::Instance& data, uint64_t& absl::Status DecoderImpl::parseXWatchesRequest(Buffer::Instance& data, uint64_t& offset, uint32_t len, OpCodes opcode) { absl::Status status = ensureMinLength(len, XID_LENGTH + OPCODE_LENGTH + (2 * INT_LENGTH)); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); const absl::StatusOr watch_type = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch_type, - watch_type.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(watch_type, + watch_type.status().message()); if (opcode == OpCodes::CheckWatches) { callbacks_.onCheckWatchesRequest(path.value(), watch_type.value()); @@ -860,16 +855,16 @@ absl::Status DecoderImpl::decodeAndBufferHelper(Buffer::Instance& data, DecodeTy while (offset < data_len) { // Peek packet length. len = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK( len, fmt::format("peekInt32 for len: {}", len.status().message())); status = ensureMinLength(len.value(), dtype == DecodeType::READ ? XID_LENGTH + INT_LENGTH : XID_LENGTH + ZXID_LENGTH + INT_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); status = ensureMaxLength(len.value()); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); offset += len.value(); if (offset <= data_len) { @@ -948,20 +943,19 @@ absl::Status DecoderImpl::parseConnectResponse(Buffer::Instance& data, uint64_t& const std::chrono::milliseconds latency) { absl::Status status = ensureMinLength(len, PROTOCOL_VERSION_LENGTH + TIMEOUT_LENGTH + SESSION_LENGTH + INT_LENGTH); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr timeout = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(timeout, - timeout.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(timeout, timeout.status().message()); // Skip session id + password. offset += SESSION_LENGTH; status = skipString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr readonly = maybeReadBool(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, - readonly.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(readonly, + readonly.status().message()); callbacks_.onConnectResponse(0, timeout.value(), readonly.value(), latency); @@ -972,18 +966,18 @@ absl::Status DecoderImpl::parseWatchEvent(Buffer::Instance& data, uint64_t& offs const uint32_t len, const int64_t zxid, const int32_t error) { absl::Status status = ensureMinLength(len, SERVER_HEADER_LENGTH + (3 * INT_LENGTH)); - COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); + EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status); const absl::StatusOr event_type = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(event_type, - event_type.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(event_type, + event_type.status().message()); const absl::StatusOr client_state = helper_.peekInt32(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(client_state, - client_state.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(client_state, + client_state.status().message()); const absl::StatusOr path = helper_.peekString(data, offset); - COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); + EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(path, path.status().message()); callbacks_.onWatchEvent(event_type.value(), client_state.value(), path.value(), zxid, error); diff --git a/source/extensions/filters/network/zookeeper_proxy/utils.h b/source/extensions/filters/network/zookeeper_proxy/utils.h index 535cbe1f70bb..76b43661dcfb 100644 --- a/source/extensions/filters/network/zookeeper_proxy/utils.h +++ b/source/extensions/filters/network/zookeeper_proxy/utils.h @@ -48,7 +48,7 @@ class BufferHelper : public Logger::Loggable { return status; \ } -#define COUNT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status) \ +#define EMIT_DECODER_ERR_AND_RETURN_IF_STATUS_NOT_OK(status) \ if (!status.ok()) { \ callbacks_.onDecodeError(); \ return status; \ @@ -59,7 +59,7 @@ class BufferHelper : public Logger::Loggable { return absl::InvalidArgumentError(message); \ } -#define COUNT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ +#define EMIT_DECODER_ERR_AND_RETURN_INVALID_ARG_ERR_IF_STATUS_NOT_OK(status, message) \ if (!status.ok()) { \ callbacks_.onDecodeError(); \ return absl::InvalidArgumentError(message); \