From 6e766ce28eb1519ef49b76d27e7c3d6985b07a9e Mon Sep 17 00:00:00 2001 From: kangping Date: Mon, 12 Apr 2021 22:03:26 +0800 Subject: [PATCH] [coap] return unexpected coap response code as error (#176) --- include/commissioner/error.hpp | 7 +- src/common/error.cpp | 2 + src/common/error_macros.hpp | 2 + src/common/error_test.cpp | 1 + src/library/coap.cpp | 126 +++++++++++++++++++++++++++++- src/library/coap.hpp | 11 ++- src/library/commissioner_impl.cpp | 56 ++++++------- src/library/commissioner_impl.hpp | 1 + 8 files changed, 170 insertions(+), 36 deletions(-) diff --git a/include/commissioner/error.hpp b/include/commissioner/error.hpp index f573e6a07..358454f0d 100644 --- a/include/commissioner/error.hpp +++ b/include/commissioner/error.hpp @@ -146,10 +146,15 @@ enum class ErrorCode : int */ kRejected = 16, + /** + * The operation was failed because with a CoAP error. + */ + kCoapError = 17, + /** * The error is out of the address space of OT Commissioner. */ - kUnknown = 17, + kUnknown = 18, }; /** diff --git a/src/common/error.cpp b/src/common/error.cpp index 6312c83f5..c197f774c 100644 --- a/src/common/error.cpp +++ b/src/common/error.cpp @@ -78,6 +78,8 @@ static std::string ErrorCodeToString(ErrorCode code) return "INVALID_STATE"; case ErrorCode::kRejected: return "REJECTED"; + case ErrorCode::kCoapError: + return "COAP_ERROR"; case ErrorCode::kUnknown: return "UNKNOWN"; diff --git a/src/common/error_macros.hpp b/src/common/error_macros.hpp index f930fcd1a..9e21493f7 100644 --- a/src/common/error_macros.hpp +++ b/src/common/error_macros.hpp @@ -72,6 +72,8 @@ Error { ErrorCode::kInvalidState, fmt::format(FMT_STRING((aFormat)), ##__VA_ARGS__) } #define ERROR_REJECTED(aFormat, ...) \ Error { ErrorCode::kRejected, fmt::format(FMT_STRING((aFormat)), ##__VA_ARGS__) } +#define ERROR_COAP_ERROR(aFormat, ...) \ + Error { ErrorCode::kCoapError, fmt::format(FMT_STRING((aFormat)), ##__VA_ARGS__) } #define ERROR_UNKNOWN(aFormat, ...) \ Error { ErrorCode::kUnknown, fmt::format(FMT_STRING((aFormat)), ##__VA_ARGS__) } diff --git a/src/common/error_test.cpp b/src/common/error_test.cpp index 005e34532..7ea9de2c3 100644 --- a/src/common/error_test.cpp +++ b/src/common/error_test.cpp @@ -83,6 +83,7 @@ TEST_CASE("error-to-string", "[error]") CHECK_ERROR_STR_STARTS_WITH_ERROR_NAME(ABORTED); CHECK_ERROR_STR_STARTS_WITH_ERROR_NAME(INVALID_STATE); CHECK_ERROR_STR_STARTS_WITH_ERROR_NAME(REJECTED); + CHECK_ERROR_STR_STARTS_WITH_ERROR_NAME(COAP_ERROR); CHECK_ERROR_STR_STARTS_WITH_ERROR_NAME(UNKNOWN); #undef CHECK_ERROR_STR_STARTS_WITH_ERROR_NAME diff --git a/src/library/coap.cpp b/src/library/coap.cpp index 847b09f7c..66dfbe2f7 100644 --- a/src/library/coap.cpp +++ b/src/library/coap.cpp @@ -49,6 +49,115 @@ namespace commissioner { namespace coap { +std::string CodeToString(Code aCode) +{ + std::string str; + + switch (aCode) + { + /* + * Methods (0.XX). + */ + case Code::kEmpty: + str = "EMPTY"; + break; + case Code::kGet: + str = "GET"; + break; + case Code::kPost: + str = "POST"; + break; + + case Code::kPut: + str = "PUT"; + break; + + case Code::kDelete: + str = "DELETE"; + break; + + /* + * Success (2.XX). + */ + case Code::kCreated: + str = "CREATED"; + break; + case Code::kDeleted: + str = "DELETED"; + break; + case Code::kValid: + str = "VALID"; + break; + case Code::kChanged: + str = "CHANGED"; + break; + case Code::kContent: + str = "CONTENT"; + break; + + /* + * Client side errors (4.XX). + */ + case Code::kBadRequest: + str = "BAD_REQUEST"; + break; + case Code::kUnauthorized: + str = "UNAUTHORIZED"; + break; + case Code::kBadOption: + str = "BAD_OPTION"; + break; + case Code::kForBidden: + str = "FORBIDDEN"; + break; + case Code::kNotFound: + str = "NOT_FOUND"; + break; + case Code::kMethodNotAllowed: + str = "METHOD_NOT_ALLOWED"; + break; + case Code::kNotAcceptable: + str = "NOT_ACCEPTABLE"; + break; + case Code::kPreconditionFailed: + str = "PRECONDITIONFAILED"; + break; + case Code::kRequestTooLarge: + str = "REQUEST_TOOL_LARGE"; + break; + case Code::kUnsupportedFormat: + str = "UNSUPPORTED_FORMAT"; + break; + + /* + * Server side errors (5.XX). + */ + case Code::kInternalError: + str = "INTERNAL_ERROR"; + break; + case Code::kNotImplemented: + str = "NOT_IMPLEMENTED"; + break; + case Code::kBadGateway: + str = "BAD_GATEWAY"; + break; + case Code::kServiceUnavailable: + str = "SERVICE_UNAVAILABLE"; + break; + case Code::kGatewayTimeout: + str = "BATEWAY_TIMEOUT"; + break; + case Code::kProxyNotSupported: + str = "PROXY_NOT_SUPPORTED"; + break; + default: + str = "UNKNOWN"; + break; + } + + return "CoAP::" + str; +} + Message::Message(Type aType, Code aCode) : Message() { @@ -567,7 +676,7 @@ void Coap::HandleRequest(const Request &aRequest) return; } -void Coap::HandleResponse(const Response &aResponse) +void Coap::HandleResponse(Response &aResponse) { const RequestHolder *requestHolder = nullptr; std::string requestUri = "UNKNOWN_URI"; @@ -583,6 +692,7 @@ void Coap::HandleResponse(const Response &aResponse) } requestHolder->mRequest->GetUriPath(requestUri).IgnoreError(); + aResponse.SetRequestUri(requestUri); switch (aResponse.GetType()) { case Type::kReset: @@ -1154,6 +1264,20 @@ void Message::SetToken(uint8_t aTokenLength) random::non_crypto::FillBuffer(mHeader.mToken, aTokenLength); } +void Message::SetRequestUri(const std::string &aUri) +{ + ASSERT(IsResponse()); + + mRequestUri = aUri; +} + +std::string Message::GetRequestUri(void) const +{ + ASSERT(IsResponse()); + + return mRequestUri; +} + } // namespace coap } // namespace commissioner diff --git a/src/library/coap.hpp b/src/library/coap.hpp index cfaf2ad32..c1fe6f2c7 100644 --- a/src/library/coap.hpp +++ b/src/library/coap.hpp @@ -127,6 +127,8 @@ enum class Code : uint8_t #undef OT_COAP_CODE +std::string CodeToString(Code aCode); + /** * CoAP Option Numbers */ @@ -508,6 +510,9 @@ class Message static Error NormalizeUriPath(std::string &uriPath); + void SetRequestUri(const std::string &aUri); + std::string GetRequestUri(void) const; + protected: Error Serialize(const Header &aHeader, ByteArray &aBuf) const; static Error Deserialize(Header &aHeader, const ByteArray &aBuf, size_t &aOffset); @@ -545,6 +550,10 @@ class Message std::map mOptions; ByteArray mPayload; + // The URI of the request to which this response messages + // is associated. Should only be set/get by a response message. + std::string mRequestUri; + MessageSubType mSubType; mutable Endpoint *mEndpoint = nullptr; @@ -746,7 +755,7 @@ class Coap void HandleRequest(const Request &aRequest); // Handle empty and response message - void HandleResponse(const Response &aResponse); + void HandleResponse(Response &aResponse); Error SendEmptyMessage(Type aType, const Request &aRequest); diff --git a/src/library/commissioner_impl.cpp b/src/library/commissioner_impl.cpp index d3a4ae024..9180fcbac 100644 --- a/src/library/commissioner_impl.cpp +++ b/src/library/commissioner_impl.cpp @@ -368,10 +368,7 @@ void CommissionerImpl::GetCommissionerDataset(Handler aHand CommissionerDataset dataset; SuccessOrExit(error = aError); - ASSERT(aResponse != nullptr); - - VerifyOrExit(aResponse->GetCode() == coap::Code::kChanged, - error = ERROR_BAD_FORMAT("expect CoAP::CHANGED for MGMT_COMM_GET.rsp message")); + SuccessOrExit(error = CheckCoapResponseCode(*aResponse)); SuccessOrExit(error = DecodeCommissionerDataset(dataset, *aResponse)); @@ -473,8 +470,7 @@ void CommissionerImpl::GetRawActiveDataset(Handler aHandler, uint16_t Error error; SuccessOrExit(error = aError); - VerifyOrExit(aResponse->GetCode() == coap::Code::kChanged, - error = ERROR_BAD_FORMAT("expect CoAP::CHANGED for MGMT_ACTIVE_GET.rsp message")); + SuccessOrExit(error = CheckCoapResponseCode(*aResponse)); aHandler(&aResponse->GetPayload(), error); @@ -579,8 +575,7 @@ void CommissionerImpl::GetPendingDataset(Handler aHan PendingOperationalDataset dataset; SuccessOrExit(error = aError); - VerifyOrExit(aResponse->GetCode() == coap::Code::kChanged, - error = ERROR_BAD_FORMAT("expect CoAP::CHANGED for MGMT_PENDING_GET.rsp message")); + SuccessOrExit(error = CheckCoapResponseCode(*aResponse)); SuccessOrExit(error = DecodePendingOperationalDataset(dataset, *aResponse)); if (dataset.mPresentFlags != 0) @@ -715,8 +710,7 @@ void CommissionerImpl::GetBbrDataset(Handler aHandler, uint16_t aDat BbrDataset dataset; SuccessOrExit(error = aError); - VerifyOrExit(aResponse->GetCode() == coap::Code::kChanged, - error = ERROR_BAD_FORMAT("expect CoAP::CHANGED for MGMT_BBR_GET.rsp message")); + SuccessOrExit(error = CheckCoapResponseCode(*aResponse)); SuccessOrExit(error = DecodeBbrDataset(dataset, *aResponse)); @@ -968,10 +962,7 @@ void CommissionerImpl::RegisterMulticastListener(Handler uint8_t status; SuccessOrExit(error = aError); - VerifyOrExit(aResponse->GetCode() != coap::Code::kUnauthorized, - error = ERROR_SECURITY("response code is CoAP::UNAUTHORIZED")); - VerifyOrExit(aResponse->GetCode() == coap::Code::kChanged, - error = ERROR_BAD_FORMAT("expect response code as CoAP::CHANGED")); + SuccessOrExit(error = CheckCoapResponseCode(*aResponse)); statusTlv = GetTlv(tlv::Type::kThreadStatus, *aResponse, tlv::Scope::kThread); VerifyOrExit(statusTlv != nullptr, error = ERROR_BAD_FORMAT("no valid State TLV found in response")); @@ -1037,10 +1028,7 @@ void CommissionerImpl::AnnounceBegin(ErrorHandler aHandler, Error error; SuccessOrExit(error = aError); - VerifyOrExit(aResponse->GetCode() != coap::Code::kUnauthorized, - error = ERROR_SECURITY("response code is CoAP::UNAUTHORIZED")); - VerifyOrExit(aResponse->GetCode() == coap::Code::kChanged, - error = ERROR_BAD_FORMAT("expect response code as CoAP::CHANGED")); + SuccessOrExit(error = CheckCoapResponseCode(*aResponse)); exit: aHandler(error); @@ -1091,10 +1079,7 @@ void CommissionerImpl::PanIdQuery(ErrorHandler aHandler, Error error; SuccessOrExit(error = aError); - VerifyOrExit(aResponse->GetCode() != coap::Code::kUnauthorized, - error = ERROR_SECURITY("response code is CoAP::UNAUTHORIZED")); - VerifyOrExit(aResponse->GetCode() == coap::Code::kChanged, - error = ERROR_BAD_FORMAT("expect response code as CoAP::CHANGED")); + SuccessOrExit(error = CheckCoapResponseCode(*aResponse)); exit: aHandler(error); @@ -1146,10 +1131,7 @@ void CommissionerImpl::EnergyScan(ErrorHandler aHandler, Error error; SuccessOrExit(error = aError); - VerifyOrExit(aResponse->GetCode() != coap::Code::kUnauthorized, - error = ERROR_SECURITY("response code is CoAP::UNAUTHORIZED")); - VerifyOrExit(aResponse->GetCode() == coap::Code::kChanged, - error = ERROR_BAD_FORMAT("expect response code as CoAP::CHANGED")); + SuccessOrExit(error = CheckCoapResponseCode(*aResponse)); exit: aHandler(error); @@ -1201,9 +1183,7 @@ void CommissionerImpl::SendPetition(PetitionHandler aHandler) std::string existingCommissionerId; SuccessOrExit(error = aError); - - VerifyOrExit(aResponse->GetCode() == coap::Code::kChanged, - error = ERROR_BAD_FORMAT("expect response code as CoAP::CHANGED")); + SuccessOrExit(error = CheckCoapResponseCode(*aResponse)); SuccessOrExit(error = GetTlvSet(tlvSet, *aResponse)); @@ -1361,16 +1341,26 @@ tlv::TlvPtr GetTlv(tlv::Type aTlvType, const coap::Message &aMessage, tlv::Scope return tlv::GetTlv(aTlvType, aMessage.GetPayload(), aScope); } +Error CommissionerImpl::CheckCoapResponseCode(const coap::Response &aResponse) +{ + Error error = ERROR_NONE; + + VerifyOrExit(aResponse.GetCode() == coap::Code::kChanged, + error = ERROR_COAP_ERROR("request for {} failed: {}", aResponse.GetRequestUri(), + coap::CodeToString(aResponse.GetCode()))); + +exit: + return error; +} + Error CommissionerImpl::HandleStateResponse(const coap::Response *aResponse, Error aError, bool aStateTlvIsMandatory) { Error error; tlv::TlvPtr stateTlv = nullptr; SuccessOrExit(error = aError); - VerifyOrExit(aResponse->GetCode() != coap::Code::kUnauthorized, - error = ERROR_SECURITY("response code is CoAP::UNAUTHORIZED")); - VerifyOrExit(aResponse->GetCode() == coap::Code::kChanged, - error = ERROR_BAD_FORMAT("expect response code as CoAP::CHANGED")); + SuccessOrExit(error = CheckCoapResponseCode(*aResponse)); + stateTlv = GetTlv(tlv::Type::kState, *aResponse); VerifyOrExit((stateTlv != nullptr || !aStateTlvIsMandatory), error = ERROR_BAD_FORMAT("no valid State TLV found in response")); diff --git a/src/library/commissioner_impl.hpp b/src/library/commissioner_impl.hpp index 64994300e..164d906b0 100644 --- a/src/library/commissioner_impl.hpp +++ b/src/library/commissioner_impl.hpp @@ -197,6 +197,7 @@ class CommissionerImpl : public Commissioner static Error ValidateConfig(const Config &aConfig); void LoggingConfig(); + static Error CheckCoapResponseCode(const coap::Response &aResponse); static Error HandleStateResponse(const coap::Response *aResponse, Error aError, bool aStateTlvIsMandatory = true); static ByteArray GetActiveOperationalDatasetTlvs(uint16_t aDatasetFlags);