afb-xreq: Forbids (un)subscribes after reply

Allowing to subscribe to a request that was replied
was possible and lead to problem revealed by issues
SPEC-2542 and SPEC-2599 (these issues are more related
to processing unordered incoming messages).

The choice was
 - fix the bug and authorize (un)subscribe after reply
 - forbids to (un)subscribe after reply

Second solution was chosen for its simplicity and
its networking efficiency.

Bug-AGL: SPEC-2542
Bug-AGL: SPEC-2599

Change-Id: I09f48b760b1fd6f70d42b80df3c8053696c45966
Signed-off-by: Jose Bollo <jose.bollo@iot.bzh>

diff --git a/docs/reference-v3/func-req.md b/docs/reference-v3/func-req.md
index 2564405..fb61575 100644 (file)
--- a/docs/reference-v3/func-req.md
+++ b/docs/reference-v3/func-req.md
@@ -528,6 +528,8 @@ int afb_req_subcall_sync(
  * Establishes for the client link identified by 'req' a subscription
  * to the 'event'.
  *
+ * Establishing subscription MUST be called BEFORE replying to the request.
+ *
  * @param req the request
  * @param event the event to subscribe
  *
@@ -546,6 +548,8 @@ int afb_req_subscribe(
  * link identified by 'req'.
  * Returns 0 in case of successful subscription or -1 in case of error.
  *
+ * Revoking subscription MUST be called BEFORE replying to the request.
+ *
  * @param req the request
  * @param event the event to revoke
  *

diff --git a/include/afb/afb-req-x1.h b/include/afb/afb-req-x1.h
index d9fa1be..0cbefc6 100644 (file)
--- a/include/afb/afb-req-x1.h
+++ b/include/afb/afb-req-x1.h
@@ -274,6 +274,7 @@ static inline int afb_req_x1_session_set_LOA(struct afb_req_x1 req, unsigned lev
  *
  * Establishes for the client link identified by 'req' a subscription
  * to the 'event'.
+ * Establishing subscriptions MUST be called BEFORE replying to the request.
  * Returns 0 in case of successful subscription or -1 in case of error.
  */
 static inline int afb_req_x1_subscribe(struct afb_req_x1 req, struct afb_event_x1 event)
@@ -286,6 +287,7 @@ static inline int afb_req_x1_subscribe(struct afb_req_x1 req, struct afb_event_x
  *
  * Revokes the subscription established to the 'event' for the client
  * link identified by 'req'.
+ * Revoking subscription MUST be called BEFORE replying to the request.
  * Returns 0 in case of successful subscription or -1 in case of error.
  */
 static inline int afb_req_x1_unsubscribe(struct afb_req_x1 req, struct afb_event_x1 event)

diff --git a/include/afb/afb-req-x2.h b/include/afb/afb-req-x2.h
index 4ae1af7..2d4a2f2 100644 (file)
--- a/include/afb/afb-req-x2.h
+++ b/include/afb/afb-req-x2.h
@@ -458,6 +458,8 @@ int afb_req_x2_session_set_LOA(
  * Establishes for the client link identified by 'req' a subscription
  * to the 'event'.
  *
+ * Establishing subscription MUST be called BEFORE replying to the request.
+ *
  * @param req the request
  * @param event the event to subscribe
  *
@@ -476,6 +478,8 @@ int afb_req_x2_subscribe(
  * link identified by 'req'.
  * Returns 0 in case of successful subscription or -1 in case of error.
  *
+ * Revoking subscription MUST be called BEFORE replying to the request.
+ *
  * @param req the request
  * @param event the event to revoke
  *

diff --git a/src/afb-xreq.c b/src/afb-xreq.c
index ba6660e..9c089bc 100644 (file)
--- a/src/afb-xreq.c
+++ b/src/afb-xreq.c
@@ -200,12 +200,17 @@ static int xreq_legacy_subscribe_event_x1_cb(struct afb_req_x2 *closure, struct
 
 int afb_xreq_subscribe(struct afb_xreq *xreq, struct afb_event_x2 *event)
 {
-       if (xreq->listener)
-               return afb_evt_event_x2_add_watch(xreq->listener, event);
-       if (xreq->queryitf->subscribe)
-               return xreq->queryitf->subscribe(xreq, event);
-       ERROR("no event listener, subscription impossible");
-       errno = EINVAL;
+       if (xreq->replied) {
+               ERROR("request replied, subscription impossible");
+               errno = EINVAL;
+       } else {
+               if (xreq->listener)
+                       return afb_evt_event_x2_add_watch(xreq->listener, event);
+               if (xreq->queryitf->subscribe)
+                       return xreq->queryitf->subscribe(xreq, event);
+               ERROR("no event listener, subscription impossible");
+               errno = ENOTSUP;
+       }
        return -1;
 }
 
@@ -222,12 +227,17 @@ static int xreq_legacy_unsubscribe_event_x1_cb(struct afb_req_x2 *closure, struc
 
 int afb_xreq_unsubscribe(struct afb_xreq *xreq, struct afb_event_x2 *event)
 {
-       if (xreq->listener)
-               return afb_evt_event_x2_remove_watch(xreq->listener, event);
-       if (xreq->queryitf->unsubscribe)
-               return xreq->queryitf->unsubscribe(xreq, event);
-       ERROR("no event listener, unsubscription impossible");
-       errno = EINVAL;
+       if (xreq->replied) {
+               ERROR("request replied, unsubscription impossible");
+               errno = EINVAL;
+       } else {
+               if (xreq->listener)
+                       return afb_evt_event_x2_remove_watch(xreq->listener, event);
+               if (xreq->queryitf->unsubscribe)
+                       return xreq->queryitf->unsubscribe(xreq, event);
+               ERROR("no event listener, unsubscription impossible");
+               errno = ENOTSUP;
+       }
        return -1;
 }