Message ID duplication not detected about mbed-client-c HOT 14 CLOSED

ARM Internal Ref: IOTCLT-760

from mbed-client-c.

Hi, thanks for your report. We will look into this and get back to you when we have reproduced and investigated this issue.

from mbed-client-c.

@EduardPon have you set SN_COAP_DUPLICATION_MAX_MSGS_COUNT? this looks to me, that it enables message de-duplication. see https://github.com/ARMmbed/mbed-client-c/blob/master/source/libCoap/src/sn_coap_protocol.c#L50

from mbed-client-c.

No I haven't. I was not aware about this definition.

Strange that this is disabled by default because it violates the CoAP standard. I would suggest to remove this switchable option and enable the duplication detection.

from mbed-client-c.

Hi,
This is an option we can enable from configuration so that application can turn it on or off based on their requirement. I will add this to our backlog to provide configuration for enabling this flag. We have similar configuration for BLOCKWISE transfer.

from mbed-client-c.

Hello,
@yogpan01, can you confirm this configuration option deletes duplicates and fixes the reason why I have opened this issue report.

What I don't understand is why it has a maximum count value? I would expect to enable or disable duplicate discarding.

Why would you allow to disable it and hereby violate the CoAP specification?

The recipient SHOULD acknowledge each
duplicate copy of a Confirmable message using the same
Acknowledgement or Reset message but SHOULD process any request or
response in the message only once.

from mbed-client-c.

Hi,
I checked and found that configuration is already present for enabling/disabling duplication logic.
https://docs.mbed.com/docs/mbed-client-guide/en/latest/Introduction/
Check section "CoAP message deduplication"
Defining non-zero value for this setting enables this feature, the count is mainly coap library implementation for keeping track of "n" number of duplicate messages.

from mbed-client-c.

Thanks for the info. But what I have understood, this relates to the duplication of messages to be transmitted, and not detecting duplicates at receiver side. These must only be acknowledged and don't be processed again. The current implementation processes every duplicate at the receiver side (CoAP spec violation).

from mbed-client-c.

Hi,
So in case the duplicate message is detected , this is how it is currently handled
https://github.com/ARMmbed/mbed-client-c/blob/master/source/libCoap/src/sn_coap_protocol.c#L664

/* Set returned status to User /
returned_dst_coap_msg_ptr->coap_status = COAP_STATUS_PARSER_DUPLICATED_MSG;
/ Because duplicate message, return with coap_status set */
return returned_dst_coap_msg_ptr;

Perhaps , we should check this status in our end before invoking callback to application.
Are you using mbed Client C++ API for your application ? We can add logic of checking this status code and avoid processing of these messages.

from mbed-client-c.

Hello,

Yes we are using the C++ API.

from mbed-client-c.

Thanks ! We will check now, how to handle this case safely on C++ side. I will keep you updated, hopefully we need not merge your PR if we are able to make the fix working from our end.

from mbed-client-c.

Hi @EduardPon , if you can try this PR ARMmbed/mbed-client#251 to verify that duplication detection is working for you. We will merge and release the update.

from mbed-client-c.

Hello,

I tested the implementation change https://github.com/ARMmbed/mbed-client/pull/251, but not in the M2MNsdlInterface but in our NSDL CoAP resource_callback function. I did not get a COAP_STATUS_PARSER_DUPLICATED_MSG coap_status. So enabled the SN_COAP_DUPLICATION_MAX_MSGS_COUNT. Still the COAP_STATUS_PARSER_DUPLICATED_MSG coap_status is not returned, BUT the duplicates are discarded. So I would suggest to enable the COAP_STATUS_PARSER_DUPLICATED_MSG by default to be compliant with the CoAP specification. I know there is a resource penalty. I would suggest to add a remark about the CoAP non conformance when disabling the COAP_STATUS_PARSER_DUPLICATED_MSG.

Summary:

• The change https://github.com/ARMmbed/mbed-client/pull/251 is not needed.
• Enable the SN_COAP_DUPLICATION_MAX_MSGS_COUNT to discard duplicates.
• Add remark CoAP non conformance when SN_COAP_DUPLICATION_MAX_MSGS_COUNT is not enabled.

from mbed-client-c.

Hi,
Thanks for the confirmation. mbed-client-c is internally handling the duplicated message so we will not need to handle it on resource level.
We will check if there is any issue with enabling the duplication by default, hopefully it should not break anything but we would need to confirm this.

from mbed-client-c.