From 293d0d4c663e22c481c9ff44b78a89a063286dc4 Mon Sep 17 00:00:00 2001
From: Robert Lubos <Robert.Lubos@nordicsemi.no>
Date: Thu, 24 Aug 2017 20:26:36 +0200
Subject: [PATCH] [coap] fix secure coap retransmission issue (#2128)

This commit fixes a bug in SecureCoap encountered when testing commissioning.

A little bit of background - after GetOwner method in CoAP was introduced, a problem with a static handler of mRetransmissionTimer showed up. In single instance mode, whenever retransmission timer striked, static GetOwner method from Coap class was called, regardless if the timer was a member of a Coap or SecureCoap class.

As there are separate instances for Coap and SecureCoap: mCoap and mCoapSecure, the former one was always returned. In result, there was no chance to handle messages queued for retransmission in an instance of SecureCoap. This resulted in commissionig failure in case a retransmission of JoinerFinalize message was needed.

The same pattern would apply for any class that uses inheritance and uses GetOwner in static handlers.

This fix extends the Coap constructor with an optional parameter -aRetransmissionHandler. This way, SecureCoap instances can register their own static timer handler and thereby use correct GetOwner funcion.
---
 src/core/coap/coap.cpp        |  4 ++--
 src/core/coap/coap.hpp        | 13 ++++++++++---
 src/core/coap/coap_secure.cpp |  7 ++++++-
 src/core/coap/coap_secure.hpp |  2 ++
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/src/core/coap/coap.cpp b/src/core/coap/coap.cpp
index d2136ea03..c02d63b3b 100644
--- a/src/core/coap/coap.cpp
+++ b/src/core/coap/coap.cpp
@@ -49,10 +49,10 @@
 namespace ot {
 namespace Coap {
 
-Coap::Coap(ThreadNetif &aNetif):
+Coap::Coap(ThreadNetif &aNetif, Timer::Handler aRetransmissionHandler):
     ThreadNetifLocator(aNetif),
     mSocket(aNetif.GetIp6().mUdp),
-    mRetransmissionTimer(aNetif.GetInstance(), &Coap::HandleRetransmissionTimer, this),
+    mRetransmissionTimer(aNetif.GetInstance(), aRetransmissionHandler, this),
     mResources(NULL),
     mContext(NULL),
     mInterceptor(NULL),
diff --git a/src/core/coap/coap.hpp b/src/core/coap/coap.hpp
index 05637c261..442aac11c 100644
--- a/src/core/coap/coap.hpp
+++ b/src/core/coap/coap.hpp
@@ -443,10 +443,11 @@ public:
     /**
      * This constructor initializes the object.
      *
-     * @param[in]  aNetif    A reference to the Netif object.
+     * @param[in]  aNetif                   A reference to the Netif object.
+     * @param[in]  aRetransmissionHandler   An optional pointer to the retransmission handler, used by CoapSecure.
      *
      */
-    Coap(ThreadNetif &aNetif);
+    Coap(ThreadNetif &aNetif, Timer::Handler aRetransmissionHandler = &Coap::HandleRetransmissionTimer);
 
     /**
      * This method starts the CoAP service.
@@ -646,6 +647,12 @@ public:
     const MessageQueue &GetCachedResponses(void) const { return mResponsesQueue.GetResponses(); }
 
 protected:
+    /**
+     * Retransmission timer handler.
+     *
+     */
+    void HandleRetransmissionTimer(void);
+
     /**
      * This method send a message.
      *
@@ -688,8 +695,8 @@ private:
     otError SendCopy(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo);
     otError SendEmptyMessage(Header::Type aType, const Header &aRequestHeader,
                              const Ip6::MessageInfo &aMessageInfo);
+
     static void HandleRetransmissionTimer(Timer &aTimer);
-    void HandleRetransmissionTimer(void);
 
     static Coap &GetOwner(const Context &aContext);
 
diff --git a/src/core/coap/coap_secure.cpp b/src/core/coap/coap_secure.cpp
index 25f5117a8..379b2d8cc 100644
--- a/src/core/coap/coap_secure.cpp
+++ b/src/core/coap/coap_secure.cpp
@@ -48,7 +48,7 @@ namespace ot {
 namespace Coap {
 
 CoapSecure::CoapSecure(ThreadNetif &aNetif):
-    Coap(aNetif),
+    Coap(aNetif, &CoapSecure::HandleRetransmissionTimer),
     mConnectedCallback(NULL),
     mConnectedContext(NULL),
     mTransportCallback(NULL),
@@ -320,6 +320,11 @@ CoapSecure &CoapSecure::GetOwner(const Context &aContext)
     return coap;
 }
 
+void CoapSecure::HandleRetransmissionTimer(Timer &aTimer)
+{
+    GetOwner(aTimer).Coap::HandleRetransmissionTimer();
+}
+
 }  // namespace Coap
 }  // namespace ot
 
diff --git a/src/core/coap/coap_secure.hpp b/src/core/coap/coap_secure.hpp
index 8ef72ea8d..6a98ac36e 100644
--- a/src/core/coap/coap_secure.hpp
+++ b/src/core/coap/coap_secure.hpp
@@ -214,6 +214,8 @@ private:
     static void HandleUdpTransmit(Tasklet &aTasklet);
     void HandleUdpTransmit(void);
 
+    static void HandleRetransmissionTimer(Timer &aTimer);
+
     static CoapSecure &GetOwner(const Context &aContext);
 
     Ip6::MessageInfo mPeerAddress;