Skip to content

Commit

Permalink
Fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@erjiaqing
erjiaqing committed Nov 4, 2021
1 parent 91e9d9b commit 485ad7a
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 34 deletions.
38 changes: 19 additions & 19 deletions src/app/ClusterInfo.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ namespace app {

/**
* ClusterInfo is the representation of an attribute path or an event path used by ReadHandler, ReadClient, WriteHandler,
* Report::Engine etc, tt uses some invalid values for representing the wildcard value for its field and contains a mpNext field so
* it can be used as a linked list.
* Report::Engine etc, it uses some invalid values for representing the wildcard values for its fields and contains a mpNext field
* so it can be used as a linked list.
*/
// TODO: The cluster info should be separated into AttributeInfo and EventInfo.
// Note: The change will happen after #11171 with a better linked list.
Expand All @@ -46,39 +46,39 @@ struct ClusterInfo

bool IsAttributePathSupersetOf(const ClusterInfo & other) const
{
VerifyOrReturnError(!HasWildcardEndpointId() || mEndpointId == other.mEndpointId, false);
VerifyOrReturnError(!HasWildcardClusterId() || mClusterId == other.mClusterId, false);
VerifyOrReturnError(!HasWildcardAttributeId() || mFieldId == other.mFieldId, false);
VerifyOrReturnError(!HasWildcardListIndex() || mListIndex == other.mListIndex, false);
VerifyOrReturnError(HasWildcardEndpointId() || mEndpointId == other.mEndpointId, false);
VerifyOrReturnError(HasWildcardClusterId() || mClusterId == other.mClusterId, false);
VerifyOrReturnError(HasWildcardAttributeId() || mFieldId == other.mFieldId, false);
VerifyOrReturnError(HasWildcardListIndex() || mListIndex == other.mListIndex, false);

return true;
}

bool HasWildcard() const { return !HasWildcardEndpointId() || !HasWildcardClusterId() || !HasWildcardAttributeId(); }
bool HasWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardAttributeId(); }

/**
* Verify if it mets some basic constrants of a attribute path: If list index is valid, then field id must be valid too.
* There are other conditions that the path is not a valid path, e.g. the path does not exists, wildcard cluster id with
* non-wildcard attribute id and valid list index on a non-array attributes etc.
* Check that the path meets some basic constraints of an attribute path: If list index is not wildcard, then field id must not
* be wildcard. This does not verify that the attribute being targeted is actually of list type when the list index is not
* wildcard.
*/
bool IsValidAttributePath() const { return mListIndex == kInvalidListIndex || mFieldId != kInvalidAttributeId; }
bool IsValidAttributePath() const { return HasWildcardListIndex() || !HasWildcardAttributeId(); }

inline bool HasWildcardNodeId() const { return mNodeId != kUndefinedNodeId; }
inline bool HasWildcardEndpointId() const { return mEndpointId != kInvalidEndpointId; }
inline bool HasWildcardClusterId() const { return mClusterId != kInvalidClusterId; }
inline bool HasWildcardAttributeId() const { return mFieldId != kInvalidAttributeId; }
inline bool HasWildcardListIndex() const { return mListIndex != kInvalidListIndex; }
inline bool HasWildcardEventId() const { return mEventId != kInvalidEventId; }
inline bool HasWildcardNodeId() const { return mNodeId == kUndefinedNodeId; }
inline bool HasWildcardEndpointId() const { return mEndpointId == kInvalidEndpointId; }
inline bool HasWildcardClusterId() const { return mClusterId == kInvalidClusterId; }
inline bool HasWildcardAttributeId() const { return mFieldId == kInvalidAttributeId; }
inline bool HasWildcardListIndex() const { return mListIndex == kInvalidListIndex; }
inline bool HasWildcardEventId() const { return mEventId == kInvalidEventId; }

ClusterInfo() {}
/*
* For better structure alignment
* Above ordering is by bit-size to ensure least amount of memory alignment padding.
* Below ordering is by bit-size to ensure least amount of memory alignment padding.
* Changing order to something more natural (e.g. endpoint id before cluster id) will result
* in extra memory alignment padding.
*/
ClusterInfo * mpNext = nullptr; // pointer width (32/64 bits)
NodeId mNodeId = kUndefinedNodeId; // uint64
ClusterInfo * mpNext = nullptr; // pointer width (32/64 bits)
ClusterId mClusterId = kInvalidClusterId; // uint32
AttributeId mFieldId = kInvalidAttributeId; // uint32
EventId mEventId = kInvalidEventId; // uint32
Expand Down
15 changes: 10 additions & 5 deletions src/app/MessageDef/AttributePath.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class Parser : public chip::app::Parser
CHIP_ERROR CheckSchemaValidity() const;
#endif
/**
* @brief Get a TLVReader for the NodeId. Next() must be called before accessing them.
* @brief Get a TLVReader for the NodeId. Next() must be called before accessing them. If the node id field does not exist, the
* nodeid will not be touched.
*
* @param [in] apNodeId A pointer to apNodeId
*
Expand All @@ -85,7 +86,8 @@ class Parser : public chip::app::Parser
CHIP_ERROR GetNodeId(chip::NodeId * const apNodeId) const;

/**
* @brief Get a TLVReader for the EndpointId. Next() must be called before accessing them.
* @brief Get a TLVReader for the EndpointId. Next() must be called before accessing them. If the endpoint id field does not
* exist, the value will not be touched.
*
* @param [in] apEndpointId A pointer to apEndpointId
*
Expand All @@ -96,7 +98,8 @@ class Parser : public chip::app::Parser
CHIP_ERROR GetEndpointId(chip::EndpointId * const apEndpointId) const;

/**
* @brief Get a TLVReader for the ClusterId. Next() must be called before accessing them.
* @brief Get a TLVReader for the ClusterId. Next() must be called before accessing them. If the cluster id field does not
* exist, the value will not be touched.
*
* @param [in] apClusterId A pointer to apClusterId
*
Expand All @@ -107,7 +110,8 @@ class Parser : public chip::app::Parser
CHIP_ERROR GetClusterId(chip::ClusterId * const apClusterId) const;

/**
* @brief Get a TLVReader for the FieldId. Next() must be called before accessing them.
* @brief Get a TLVReader for the FieldId. Next() must be called before accessing them. If the field id field does not exist,
* the value will not be touched.
*
* @param [in] apFieldId A pointer to apFieldId
*
Expand All @@ -118,7 +122,8 @@ class Parser : public chip::app::Parser
CHIP_ERROR GetFieldId(chip::AttributeId * const apFieldId) const;

/**
* @brief Get a TLVReader for the ListIndex. Next() must be called before accessing them.
* @brief Get a TLVReader for the ListIndex. Next() must be called before accessing them. If the list index field does not
* exist, the value will not be touched.
*
* @param [in] apListIndex A pointer to apListIndex
*
Expand Down
14 changes: 14 additions & 0 deletions src/app/MessageDef/Parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,26 @@ class Parser
chip::TLV::TLVType mOuterContainerType;
Parser();

/**
* Gets a unsigned integer value with the given tag, the value is not touched when the tag is not found in the TLV.
*
* @return #CHIP_NO_ERROR on success
* #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
* #CHIP_END_OF_TLV if there is no such element
*/
template <typename T>
CHIP_ERROR GetUnsignedInteger(const uint8_t aContextTag, T * const apLValue) const
{
return GetSimpleValue(aContextTag, chip::TLV::kTLVType_UnsignedInteger, apLValue);
};

/**
* Gets a scaler value with the given tag, the value is not touched when the tag is not found in the TLV.
*
* @return #CHIP_NO_ERROR on success
* #CHIP_ERROR_WRONG_TLV_TYPE if there is such element but it's not any of the defined unsigned integer types
* #CHIP_END_OF_TLV if there is no such element
*/
template <typename T>
CHIP_ERROR GetSimpleValue(const uint8_t aContextTag, const chip::TLV::TLVType aTLVType, T * const apLValue) const
{
Expand Down
21 changes: 11 additions & 10 deletions src/app/ReadClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,32 +473,37 @@ CHIP_ERROR ReadClient::ProcessAttributeDataList(TLV::TLVReader & aAttributeDataL
SuccessOrExit(err);

err = element.GetAttributePath(&attributePathParser);
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

// We are using the feature that the parser won't touch the value if the field does not exist, since all fields in the
// cluster info will be invalid / wildcard, it is safe ignore CHIP_END_OF_TLV directly.

err = attributePathParser.GetNodeId(&(clusterInfo.mNodeId));
if (err == CHIP_END_OF_TLV)
{
err = CHIP_NO_ERROR;
}
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

// The ReportData must contain a concrete attribute path

err = attributePathParser.GetEndpointId(&(clusterInfo.mEndpointId));
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

err = attributePathParser.GetClusterId(&(clusterInfo.mClusterId));
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

err = attributePathParser.GetFieldId(&(clusterInfo.mFieldId));
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

err = attributePathParser.GetListIndex(&(clusterInfo.mListIndex));
if (CHIP_END_OF_TLV == err)
{
err = CHIP_NO_ERROR;
}
SuccessOrExit(err);
VerifyOrExit(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

VerifyOrExit(clusterInfo.IsValidAttributePath(), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

err = element.GetData(&dataReader);
if (err == CHIP_END_OF_TLV)
Expand All @@ -523,10 +528,6 @@ CHIP_ERROR ReadClient::ProcessAttributeDataList(TLV::TLVReader & aAttributeDataL
}

exit:
if (err != CHIP_NO_ERROR)
{
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
}
return err;
}

Expand Down
4 changes: 4 additions & 0 deletions src/app/WriteHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataList(TLV::TLVReader & aAttributeDat
err = element.GetAttributePath(&attributePath);
SuccessOrExit(err);

// We are using the feature that the parser won't touch the value if the field does not exist, since all fields in the
// cluster info will be invalid / wildcard, it is safe ignore CHIP_END_OF_TLV directly.

err = attributePath.GetNodeId(&(clusterInfo.mNodeId));
if (CHIP_END_OF_TLV == err)
{
Expand All @@ -144,6 +147,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataList(TLV::TLVReader & aAttributeDat
err = CHIP_NO_ERROR;
}

// We do not support Wildcard writes for now, reject all wildcard write requests.
VerifyOrExit(clusterInfo.IsValidAttributePath() && !clusterInfo.HasWildcard(),
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

Expand Down

0 comments on commit 485ad7a

Please sign in to comment.