espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-06-06 05:24:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

[coap] simplify Coap::TxParameters validation (#12235)

Browse Source 
This commit simplifies the validation of `Coap::TxParameters`.

The primary `SendMessage()` method is updated to accept `TxParameters`
as a pointer, where `nullptr` is mapped to the default `TxParameters`.
The user-provided `TxParameters` are now validated in the primary
`SendMessage()` method by calling `TxParameters::ValidateFor()` replacing
checks previously performed in `coap_api.cpp` source file.

This commit also adds a set of `static_assert()` checks to validate the
default `TxParameters` at compile-time, ensuring all its properties are
within valid ranges and that duration calculations will not cause
an overflow.

This approach simplifies the API by removing the `TxParameters::From()`
helper and centralizes `TxParameters` selection and validation logic
within the core `CoapBase` class.
This commit is contained in:
Abtin Keshavarzian
2025-12-23 19:25:52 -08:00
committed by GitHub
parent c222f582b4
commit 4c152b91aa
4 changed files with 67 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/api/coap_api.cpp
+6 -19
View File
@@ -209,19 +209,13 @@ otError otCoapSendRequestBlockWiseWithParameters(otInstance *aIn
otCoapBlockwiseTransmitHook aTransmitHook,
otCoapBlockwiseReceiveHook aReceiveHook)
{
Error error;
const Coap::TxParameters &txParameters = Coap::TxParameters::From(aTxParameters);
Error error;
VerifyOrExit(!AsCoreType(aMessage).IsOriginThreadNetif(), error = kErrorInvalidArgs);
if (aTxParameters != nullptr)
{
VerifyOrExit(txParameters.IsValid(), error = kErrorInvalidArgs);
}
error = AsCoreType(aInstance).Get<Coap::ApplicationCoap>().SendMessage(
AsCoapMessage(aMessage), AsCoreType(aMessageInfo), txParameters, aHandler, aContext, aTransmitHook,
aReceiveHook);
AsCoapMessage(aMessage), AsCoreType(aMessageInfo), AsCoreTypePtr(aTxParameters), aHandler, aContext,
aTransmitHook, aReceiveHook);
exit:
return error;
@@ -237,17 +231,10 @@ otError otCoapSendRequestWithParameters(otInstance *aInstance,
{
Error error;
const Coap::TxParameters &txParameters = Coap::TxParameters::From(aTxParameters);
VerifyOrExit(!AsCoreType(aMessage).IsOriginThreadNetif(), error = kErrorInvalidArgs);
if (aTxParameters != nullptr)
{
VerifyOrExit(txParameters.IsValid(), error = kErrorInvalidArgs);
}
error = AsCoreType(aInstance).Get<Coap::ApplicationCoap>().SendMessage(
AsCoapMessage(aMessage), AsCoreType(aMessageInfo), txParameters, aHandler, aContext);
AsCoapMessage(aMessage), AsCoreType(aMessageInfo), AsCoreTypePtr(aTxParameters), aHandler, aContext);
exit:
return error;
@@ -305,7 +292,7 @@ otError otCoapSendResponseBlockWiseWithParameters(otInstance *aI
VerifyOrExit(!AsCoreType(aMessage).IsOriginThreadNetif(), error = kErrorInvalidArgs);
error = AsCoreType(aInstance).Get<Coap::ApplicationCoap>().SendMessage(
AsCoapMessage(aMessage), AsCoreType(aMessageInfo), Coap::TxParameters::From(aTxParameters), nullptr, aContext,
AsCoapMessage(aMessage), AsCoreType(aMessageInfo), AsCoreTypePtr(aTxParameters), nullptr, aContext,
aTransmitHook, nullptr);
exit:
return error;
@@ -322,7 +309,7 @@ otError otCoapSendResponseWithParameters(otInstance *aInstance,
VerifyOrExit(!AsCoreType(aMessage).IsOriginThreadNetif(), error = kErrorInvalidArgs);
error = AsCoreType(aInstance).Get<Coap::ApplicationCoap>().SendMessage(
AsCoapMessage(aMessage), AsCoreType(aMessageInfo), Coap::TxParameters::From(aTxParameters), nullptr, nullptr);
AsCoapMessage(aMessage), AsCoreType(aMessageInfo), AsCoreTypePtr(aTxParameters), nullptr, nullptr);
exit:
return error;
src/core/coap/coap.cpp
+51 -32
View File
@@ -180,7 +180,7 @@ Error CoapBase::Send(ot::Message &aMessage, const Ip6::MessageInfo &aMessageInfo
#if OPENTHREAD_CONFIG_COAP_BLOCKWISE_TRANSFER_ENABLE
Error CoapBase::SendMessage(Message &aMessage,
const Ip6::MessageInfo &aMessageInfo,
const TxParameters &aTxParameters,
const TxParameters *aTxParameters,
ResponseHandler aHandler,
void *aContext,
otCoapBlockwiseTransmitHook aTransmitHook,
@@ -188,7 +188,7 @@ Error CoapBase::SendMessage(Message &aMessage,
#else
Error CoapBase::SendMessage(Message &aMessage,
const Ip6::MessageInfo &aMessageInfo,
const TxParameters &aTxParameters,
const TxParameters *aTxParameters,
ResponseHandler aHandler,
void *aContext)
#endif
@@ -202,10 +202,13 @@ Error CoapBase::SendMessage(Message &aMessage,
bool moreBlocks = false;
#endif
// fire and forget (mAckTimeout=0) is only allowed for Non-confirmable (NON) messages
if (aTxParameters.mAckTimeout == 0)
if (aTxParameters == nullptr)
{
VerifyOrExit(aMessage.IsNonConfirmable(), error = kErrorInvalidArgs);
aTxParameters = &TxParameters::GetDefault();
}
else
{
SuccessOrExit(error = aTxParameters->ValidateFor(aMessage));
}
switch (aMessage.GetType())
@@ -227,7 +230,7 @@ Error CoapBase::SendMessage(Message &aMessage,
}
#endif
mResponseCache.Add(aMessage, aMessageInfo, aTxParameters.CalculateExchangeLifetime());
mResponseCache.Add(aMessage, aMessageInfo, aTxParameters->CalculateExchangeLifetime());
break;
case kTypeReset:
OT_ASSERT(aMessage.GetCode() == kCodeEmpty);
@@ -313,8 +316,8 @@ Error CoapBase::SendMessage(Message &aMessage,
metadata.mMulticastLoop = aMessageInfo.GetMulticastLoop();
metadata.mResponseHandler = aHandler;
metadata.mResponseContext = aContext;
metadata.mRetransmissionsRemaining = aTxParameters.mMaxRetransmit;
metadata.mRetransmissionTimeout = aTxParameters.CalculateInitialRetransmissionTimeout();
metadata.mRetransmissionsRemaining = aTxParameters->mMaxRetransmit;
metadata.mRetransmissionTimeout = aTxParameters->CalculateInitialRetransmissionTimeout();
metadata.mAcknowledged = false;
metadata.mConfirmable = aMessage.IsConfirmable();
#if OPENTHREAD_CONFIG_BACKBONE_ROUTER_ENABLE
@@ -330,7 +333,7 @@ Error CoapBase::SendMessage(Message &aMessage,
#endif
metadata.mNextTimerShot =
TimerMilli::GetNow() +
(metadata.mConfirmable ? metadata.mRetransmissionTimeout : aTxParameters.CalculateMaxTransmitWait());
(metadata.mConfirmable ? metadata.mRetransmissionTimeout : aTxParameters->CalculateMaxTransmitWait());
storedCopy = CopyAndEnqueueMessage(aMessage, copyLength, metadata);
VerifyOrExit(storedCopy != nullptr, error = kErrorNoBufs);
@@ -350,7 +353,7 @@ exit:
Error CoapBase::SendMessage(Message &aMessage, const Ip6::MessageInfo &aMessageInfo, const TxParameters &aTxParameters)
{
return SendMessage(aMessage, aMessageInfo, aTxParameters, nullptr, nullptr);
return SendMessage(aMessage, aMessageInfo, &aTxParameters, nullptr, nullptr);
}
Error CoapBase::SendMessage(Message &aMessage,
@@ -358,11 +361,7 @@ Error CoapBase::SendMessage(Message &aMessage,
ResponseHandler aHandler,
void *aContext)
{
#if OPENTHREAD_CONFIG_COAP_BLOCKWISE_TRANSFER_ENABLE
return SendMessage(aMessage, aMessageInfo, TxParameters::GetDefault(), aHandler, aContext, nullptr, nullptr);
#else
return SendMessage(aMessage, aMessageInfo, TxParameters::GetDefault(), aHandler, aContext);
#endif
return SendMessage(aMessage, aMessageInfo, nullptr, aHandler, aContext);
}
Error CoapBase::SendMessage(OwnedPtr<Message> aMessage,
@@ -1276,7 +1275,7 @@ Error CoapBase::SendNextBlock1Request(Message &aRequest,
LogInfo("Send Block1 Nr. %d, Size: %d bytes, More Blocks Flag: %d", request->GetBlockWiseBlockNumber(),
otCoapBlockSizeFromExponent(request->GetBlockWiseBlockSize()), request->IsMoreBlocksFlagSet());
SuccessOrExit(error = SendMessage(*request, aMessageInfo, TxParameters::GetDefault(),
SuccessOrExit(error = SendMessage(*request, aMessageInfo, /* aTxParamters */ nullptr,
aCoapMetadata.mResponseHandler, aCoapMetadata.mResponseContext,
aCoapMetadata.mBlockwiseTransmitHook, aCoapMetadata.mBlockwiseReceiveHook));
@@ -1339,7 +1338,7 @@ Error CoapBase::SendNextBlock2Request(Message &aRequest,
otCoapBlockSizeFromExponent(request->GetBlockWiseBlockSize()));
SuccessOrExit(error =
SendMessage(*request, aMessageInfo, TxParameters::GetDefault(), aCoapMetadata.mResponseHandler,
SendMessage(*request, aMessageInfo, /* aTxParameters */ nullptr, aCoapMetadata.mResponseHandler,
aCoapMetadata.mResponseContext, nullptr, aCoapMetadata.mBlockwiseReceiveHook));
exit:
@@ -1694,16 +1693,45 @@ void CoapBase::ResponseCache::HandleTimer(void)
//---------------------------------------------------------------------------------------------------------------------
// TxParameters
bool TxParameters::IsValid(void) const
const otCoapTxParameters TxParameters::kDefaultTxParameters = {
kDefaultAckTimeout,
kDefaultAckRandomFactorNumerator,
kDefaultAckRandomFactorDenominator,
kDefaultMaxRetransmit,
};
const TxParameters &TxParameters::GetDefault(void)
{
bool isValid = false;
// Validate the default `TxParameters` at compile-time
static constexpr uint64_t kMaxDuration = static_cast<uint64_t>(kDefaultAckTimeout) *
kDefaultAckRandomFactorNumerator *
(1UL << (kDefaultMaxRetransmit + 1)) +
2 * kDefaultMaxLatency;
static_assert(kDefaultAckRandomFactorDenominator > 0, "kDefaultAckRandomFactorDenominator MUST be non-zero");
static_assert(kDefaultAckRandomFactorNumerator >= kDefaultAckRandomFactorDenominator, "Numerator is invalid");
static_assert(kMinAckTimeout > 0, "kMinAckTimeout MUST be non-zero");
static_assert(kDefaultAckTimeout >= kMinAckTimeout, "kDefaultAckTimeout is invalid");
static_assert(kMaxRetransmit > 0, "kMaxRetransmit MUST be non-zero");
static_assert(kMaxRetransmit < 31, "kMaxRetransmit is not valid");
static_assert(kDefaultMaxRetransmit <= kMaxRetransmit, "kDefaultMaxRetransmit is invalid");
static_assert(kMaxDuration < NumericLimits<uint32_t>::kMax, "Default `TxParameters` is invalid");
return AsCoreType(&kDefaultTxParameters);
}
Error TxParameters::ValidateFor(const Message &aMessage) const
{
Error error = kErrorInvalidArgs;
uint32_t duration;
uint32_t retryFactor;
if (mAckTimeout == 0)
{
// Support fire and forget requests
isValid = true;
// Fire and forget is only allowed for non-confirmable messages.
VerifyOrExit(aMessage.IsNonConfirmable());
error = kErrorNone;
ExitNow();
}
@@ -1714,8 +1742,6 @@ bool TxParameters::IsValid(void) const
// Calculate exchange lifetime max duration step by step and verify no overflow.
static_assert(kMaxRetransmit < 31, "kMaxRetransmit is not valid");
retryFactor = static_cast<uint32_t>((1U << (mMaxRetransmit + 1)) - 1);
SuccessOrExit(SafeMultiply<uint32_t>(mAckTimeout, retryFactor, duration));
@@ -1726,10 +1752,10 @@ bool TxParameters::IsValid(void) const
VerifyOrExit(CanAddSafely<uint32_t>(mAckTimeout, 2 * kDefaultMaxLatency));
VerifyOrExit(CanAddSafely<uint32_t>(duration, mAckTimeout + 2 * kDefaultMaxLatency));
isValid = true;
error = kErrorNone;
exit:
return isValid;
return error;
}
uint32_t TxParameters::CalculateInitialRetransmissionTimeout(void) const
@@ -1752,13 +1778,6 @@ uint32_t TxParameters::CalculateSpan(uint8_t aMaxRetx) const
mAckRandomFactorNumerator);
}
const otCoapTxParameters TxParameters::kDefaultTxParameters = {
kDefaultAckTimeout,
kDefaultAckRandomFactorNumerator,
kDefaultAckRandomFactorDenominator,
kDefaultMaxRetransmit,
};
//---------------------------------------------------------------------------------------------------------------------
// Resource
src/core/coap/coap.hpp
+9 -30
View File
@@ -95,33 +95,11 @@ class TxParameters : public otCoapTxParameters
public:
/**
* Converts a pointer to `otCoapTxParameters` to `Coap::TxParamters`
*
* If the pointer is `nullptr`, the default parameters are used instead.
*
* @param[in] aTxParameters A pointer to tx parameter.
*
* @returns A reference to corresponding `TxParamters` if @p aTxParameters is not `nullptr`, otherwise the default
* tx parameters.
*/
static const TxParameters &From(const otCoapTxParameters *aTxParameters)
{
return aTxParameters ? *static_cast<const TxParameters *>(aTxParameters) : GetDefault();
}
/**
* Validates whether the CoAP transmission parameters are valid.
*
* @returns Whether the parameters are valid.
*/
bool IsValid(void) const;
/**
* Returns default CoAP tx parameters.
* Returns the default CoAP tx parameters.
*
* @returns The default tx parameters.
*/
static const TxParameters &GetDefault(void) { return static_cast<const TxParameters &>(kDefaultTxParameters); }
static const TxParameters &GetDefault(void);
private:
static constexpr uint32_t kDefaultAckTimeout = 2000; // in msec
@@ -132,6 +110,7 @@ private:
static constexpr uint8_t kMaxRetransmit = OT_COAP_MAX_RETRANSMIT;
static constexpr uint32_t kMinAckTimeout = OT_COAP_MIN_ACK_TIMEOUT;
Error ValidateFor(const Message &aMessage) const;
uint32_t CalculateInitialRetransmissionTimeout(void) const;
uint32_t CalculateExchangeLifetime(void) const;
uint32_t CalculateMaxTransmitWait(void) const;
@@ -465,7 +444,7 @@ public:
*
* @param[in] aMessage A reference to the message to send.
* @param[in] aMessageInfo A reference to the message info associated with @p aMessage.
* @param[in] aTxParameters A reference to transmission parameters for this message.
* @param[in] aTxParameters A pointer to `TxParameters`. If `nullptr`, default `TxParameters` will be used.
* @param[in] aHandler A function pointer that shall be called on response reception or time-out.
* @param[in] aContext A pointer to arbitrary context information.
* @param[in] aTransmitHook A pointer to a hook function for outgoing block-wise transfer.
@@ -476,9 +455,9 @@ public:
*/
Error SendMessage(Message &aMessage,
const Ip6::MessageInfo &aMessageInfo,
const TxParameters &aTxParameters,
otCoapResponseHandler aHandler = nullptr,
void *aContext = nullptr,
const TxParameters *aTxParameters,
otCoapResponseHandler aHandler,
void *aContext,
otCoapBlockwiseTransmitHook aTransmitHook = nullptr,
otCoapBlockwiseReceiveHook aReceiveHook = nullptr);
#else // OPENTHREAD_CONFIG_COAP_BLOCKWISE_TRANSFER_ENABLE
@@ -492,7 +471,7 @@ public:
*
* @param[in] aMessage A reference to the message to send.
* @param[in] aMessageInfo A reference to the message info associated with @p aMessage.
* @param[in] aTxParameters A reference to transmission parameters for this message.
* @param[in] aTxParameters A pointer to `TxParameters`. If `nullptr`, default `TxParameters` will be used.
* @param[in] aHandler A function pointer that shall be called on response reception or time-out.
* @param[in] aContext A pointer to arbitrary context information.
*
@@ -501,7 +480,7 @@ public:
*/
Error SendMessage(Message &aMessage,
const Ip6::MessageInfo &aMessageInfo,
const TxParameters &aTxParameters,
const TxParameters *aTxParameters,
ResponseHandler aHandler,
void *aContext);
#endif // OPENTHREAD_CONFIG_COAP_BLOCKWISE_TRANSFER_ENABLE
src/core/coap/coap_secure.cpp
+1 -1
View File
@@ -66,7 +66,7 @@ Error SecureSession::SendMessage(Message &aMessage,
otCoapBlockwiseTransmitHook aTransmitHook,
otCoapBlockwiseReceiveHook aReceiveHook)
{
return IsConnected() ? CoapBase::SendMessage(aMessage, GetMessageInfo(), TxParameters::GetDefault(), aHandler,
return IsConnected() ? CoapBase::SendMessage(aMessage, GetMessageInfo(), /* aTxParameters */ nullptr, aHandler,
aContext, aTransmitHook, aReceiveHook)
: kErrorInvalidState;
}