res_pjsip_pubsub: Add some type safety when generating NOTIFY bodies.

res_pjsip_pubsub has two separate checks that it makes when a SUBSCRIBE
arrives.
* It checks that there is a subscription handler for the Event
* It checks that there are body generators for the types in the Accept header

The problem is, there's nothing that ensures that these two things will
actually mesh with each other. For instance, Asterisk will accept a subscription
to MWI that accepts pidf+xml bodies. That doesn't make sense.

With this commit, we add some type information to the mix. Subscription
handlers state they generate data of type X, and body generators state
that they consume data of type X. This way, Asterisk doesn't end up in
some hilariously mismatched situation like the one in the previous paragraph.

ASTERISK-24136 #close
Reported by Mark Michelson

Review: https://reviewboard.asterisk.org/r/3877
Review: https://reviewboard.asterisk.org/r/3878

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/12@423344 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/include/asterisk/res_pjsip_pubsub.h b/include/asterisk/res_pjsip_pubsub.h
index d46cbae30d38896c5e5d73dda7ee7c16296a2279..128379d078db75ed0900a59b6179c0c3867c0602 100644 (file)
--- a/include/asterisk/res_pjsip_pubsub.h
+++ b/include/asterisk/res_pjsip_pubsub.h
@@ -221,11 +221,18 @@ struct ast_sip_subscription_response_data {
        struct ast_sip_body *body;
 };
 
+/*!
+ * \brief Type information for NOTIFY/PUBLISH body data
+ */
+#define AST_SIP_EXTEN_STATE_DATA "ast_sip_exten_state_data"
+#define AST_SIP_MESSAGE_ACCUMULATOR "ast_sip_message_accumulator"
+
 #define AST_SIP_MAX_ACCEPT 32
 
 struct ast_sip_subscription_handler {
        /*! The name of the event this handler deals with */
        const char *event_name;
+       const char *body_type;
        /*! The types of body this handler accepts.
         *
         * \note This option has no bearing when the handler is used in the
@@ -566,6 +573,8 @@ struct ast_sip_pubsub_body_generator {
         * In "plain/text", "text" is the subtype
         */
        const char *subtype;
+       /*! The type of body that the body generator takes as input */
+       const char *body_type;
        /*!
         * \brief allocate body structure.
         *
@@ -645,6 +654,16 @@ struct ast_sip_pubsub_body_supplement {
        AST_LIST_ENTRY(ast_sip_pubsub_body_supplement) list;
 };
 
+/*!
+ * \brief Data used to create bodies for NOTIFY/PUBLISH requests.
+ */
+struct ast_sip_body_data {
+       /*! The type of the data */
+       const char *body_type;
+       /*! The actual data from which the body is generated */
+       void *body_data;
+};
+
 /*!
  * \since 13.0.0
  * \brief Generate body content for a PUBLISH or NOTIFY
@@ -662,7 +681,7 @@ struct ast_sip_pubsub_body_supplement {
  * \retval non-zero Failure
  */
 int ast_sip_pubsub_generate_body_content(const char *content_type,
-               const char *content_subtype, void *data, struct ast_str **str);
+               const char *content_subtype, struct ast_sip_body_data *data, struct ast_str **str);
 
 /*!
  * \since 13.0.0

diff --git a/res/res_pjsip_dialog_info_body_generator.c b/res/res_pjsip_dialog_info_body_generator.c
index c6d53dc7b3b4b087e886c51e2ccf6ba9bcdef99b..f7045e594bd58f9286dcac06a24a2bc58a74ff04 100644 (file)
--- a/res/res_pjsip_dialog_info_body_generator.c
+++ b/res/res_pjsip_dialog_info_body_generator.c
@@ -187,6 +187,7 @@ static void dialog_info_to_string(void *body, struct ast_str **str)
 static struct ast_sip_pubsub_body_generator dialog_info_body_generator = {
        .type = "application",
        .subtype = "dialog-info+xml",
+       .body_type = AST_SIP_EXTEN_STATE_DATA,
        .allocate_body = dialog_info_allocate_body,
        .generate_body_content = dialog_info_generate_body_content,
        .to_string = dialog_info_to_string,

diff --git a/res/res_pjsip_exten_state.c b/res/res_pjsip_exten_state.c
index e452b240d74fa938103af17c62f0e7d1d74b514c..2e133f926e8476294fd0622d72ba442ed5886002 100644 (file)
--- a/res/res_pjsip_exten_state.c
+++ b/res/res_pjsip_exten_state.c
@@ -81,6 +81,7 @@ static void to_ami(struct ast_sip_subscription *sub,
 
 struct ast_sip_subscription_handler presence_handler = {
        .event_name = "presence",
+       .body_type = AST_SIP_EXTEN_STATE_DATA,
        .accept = { DEFAULT_PRESENCE_BODY, },
        .default_accept = DEFAULT_PRESENCE_BODY,
        .subscription_shutdown = subscription_shutdown,
@@ -93,6 +94,7 @@ struct ast_sip_subscription_handler presence_handler = {
 
 struct ast_sip_subscription_handler dialog_handler = {
        .event_name = "dialog",
+       .body_type = AST_SIP_EXTEN_STATE_DATA,
        .accept = { DEFAULT_DIALOG_BODY, },
        .default_accept = DEFAULT_DIALOG_BODY,
        .subscription_shutdown = subscription_shutdown,
@@ -180,12 +182,16 @@ static void create_send_notify(struct exten_state_subscription *exten_state_sub,
        const pj_str_t *reason_str_ptr = NULL;
        pjsip_tx_data *tdata;
        struct ast_sip_body body;
+       struct ast_sip_body_data body_data = {
+               .body_type = AST_SIP_EXTEN_STATE_DATA,
+               .body_data = exten_state_data,
+       };
 
        body.type = ast_sip_subscription_get_body_type(exten_state_sub->sip_sub);
        body.subtype = ast_sip_subscription_get_body_subtype(exten_state_sub->sip_sub);
 
        if (ast_sip_pubsub_generate_body_content(body.type, body.subtype,
-                               exten_state_data, &body_text)) {
+                               &body_data, &body_text)) {
                ast_log(LOG_ERROR, "Unable to create body on NOTIFY request\n");
                return;
        }

diff --git a/res/res_pjsip_mwi.c b/res/res_pjsip_mwi.c
index f25a7c48f9714fd7f7221f0d9f578344c0fe5df2..f30e49447875d09996bd8f510fd9df376827a247 100644 (file)
--- a/res/res_pjsip_mwi.c
+++ b/res/res_pjsip_mwi.c
@@ -63,6 +63,7 @@ static void mwi_to_ami(struct ast_sip_subscription *sub, struct ast_str **buf);
 
 static struct ast_sip_subscription_handler mwi_handler = {
        .event_name = "message-summary",
+       .body_type = AST_SIP_MESSAGE_ACCUMULATOR,
        .accept = { MWI_TYPE"/"MWI_SUBTYPE, },
        .default_accept =  MWI_TYPE"/"MWI_SUBTYPE,
        .subscription_shutdown = mwi_subscription_shutdown,
@@ -432,6 +433,10 @@ static void send_mwi_notify(struct mwi_subscription *sub, pjsip_evsub_state stat
        pjsip_tx_data *tdata;
        pj_str_t reason_str;
        struct ast_sip_body body;
+       struct ast_sip_body_data body_data = {
+               .body_type = AST_SIP_MESSAGE_ACCUMULATOR,
+               .body_data = &counter,
+       };
 
        body.type = sub->is_solicited ?
                ast_sip_subscription_get_body_type(sub->sip_sub) :
@@ -448,7 +453,7 @@ static void send_mwi_notify(struct mwi_subscription *sub, pjsip_evsub_state stat
                reason_str_ptr = &reason_str;
        }
 
-       if (ast_sip_pubsub_generate_body_content(body.type, body.subtype, &counter, &body_text)) {
+       if (ast_sip_pubsub_generate_body_content(body.type, body.subtype, &body_data, &body_text)) {
                ast_log(LOG_WARNING, "Unable to generate SIP MWI NOTIFY body.\n");
                return;
        }

diff --git a/res/res_pjsip_mwi_body_generator.c b/res/res_pjsip_mwi_body_generator.c
index 9a721dbf47d7441492e41bf72ef70b1708fba1e1..4e6f814d808564e73d2ef7a409547ecef81c8159 100644 (file)
--- a/res/res_pjsip_mwi_body_generator.c
+++ b/res/res_pjsip_mwi_body_generator.c
@@ -85,6 +85,7 @@ static void mwi_destroy_body(void *body)
 static struct ast_sip_pubsub_body_generator mwi_generator = {
        .type = MWI_TYPE,
        .subtype = MWI_SUBTYPE,
+       .body_type = AST_SIP_MESSAGE_ACCUMULATOR,
        .allocate_body = mwi_allocate_body,
        .generate_body_content = mwi_generate_body_content,
        .to_string = mwi_to_string,

diff --git a/res/res_pjsip_pidf_body_generator.c b/res/res_pjsip_pidf_body_generator.c
index 2c99f5cfdafd3d21f55cc22f690825a24754d734..cd4840e13a4b4c31a1294449c9c5aff0a9be7bea 100644 (file)
--- a/res/res_pjsip_pidf_body_generator.c
+++ b/res/res_pjsip_pidf_body_generator.c
@@ -109,6 +109,7 @@ static void pidf_to_string(void *body, struct ast_str **str)
 static struct ast_sip_pubsub_body_generator pidf_body_generator = {
        .type = "application",
        .subtype = "pidf+xml",
+       .body_type = AST_SIP_EXTEN_STATE_DATA,
        .allocate_body = pidf_allocate_body,
        .generate_body_content = pidf_generate_body_content,
        .to_string = pidf_to_string,

diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index be97991baa94a7b80b6b0e7cdc1175b69c7e40a9..f4d9dbc2534a11f1065d8c359b19e788392f5636 100644 (file)
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -366,7 +366,7 @@ static void subscription_persistence_remove(struct ast_sip_subscription *sub)
 
 static struct ast_sip_subscription_handler *find_sub_handler_for_event_name(const char *event_name);
 static struct ast_sip_pubsub_body_generator *find_body_generator(char accept[AST_SIP_MAX_ACCEPT][64],
-               size_t num_accept);
+               size_t num_accept, const char *body_type);
 
 /*! \brief Retrieve a handler using the Event header of an rdata message */
 static struct ast_sip_subscription_handler *subscription_get_handler_from_rdata(pjsip_rx_data *rdata)
@@ -414,7 +414,7 @@ static struct ast_sip_pubsub_body_generator *subscription_get_generator_from_rda
                num_accept_headers = 1;
        }
 
-       return find_body_generator(accept, num_accept_headers);
+       return find_body_generator(accept, num_accept_headers, handler->body_type);
 }
 
 /*! \brief Callback function to perform the actual recreation of a subscription */
@@ -1092,7 +1092,7 @@ static struct ast_sip_pubsub_body_generator *find_body_generator_accept(const ch
 }
 
 static struct ast_sip_pubsub_body_generator *find_body_generator(char accept[AST_SIP_MAX_ACCEPT][64],
-               size_t num_accept)
+               size_t num_accept, const char *body_type)
 {
        int i;
        struct ast_sip_pubsub_body_generator *generator = NULL;
@@ -1101,6 +1101,12 @@ static struct ast_sip_pubsub_body_generator *find_body_generator(char accept[AST
                generator = find_body_generator_accept(accept[i]);
                if (generator) {
                        ast_debug(3, "Body generator %p found for accept type %s\n", generator, accept[i]);
+                       if (strcmp(generator->body_type, body_type)) {
+                               ast_log(LOG_WARNING, "Body generator '%s/%s'(%p) does not accept the type of data this event generates\n",
+                                               generator->type, generator->subtype, generator);
+                               generator = NULL;
+                               continue;
+                       }
                        break;
                } else {
                        ast_debug(3, "No body generator found for accept type %s\n", accept[i]);
@@ -1511,7 +1517,7 @@ const char *ast_sip_subscription_get_body_subtype(struct ast_sip_subscription *s
 }
 
 int ast_sip_pubsub_generate_body_content(const char *type, const char *subtype,
-               void *data, struct ast_str **str)
+               struct ast_sip_body_data *data, struct ast_str **str)
 {
        struct ast_sip_pubsub_body_supplement *supplement;
        struct ast_sip_pubsub_body_generator *generator;
@@ -1525,14 +1531,19 @@ int ast_sip_pubsub_generate_body_content(const char *type, const char *subtype,
                return -1;
        }
 
-       body = generator->allocate_body(data);
+       if (strcmp(data->body_type, generator->body_type)) {
+               ast_log(LOG_WARNING, "Body generator does not accept the type of data provided\n");
+               return -1;
+       }
+
+       body = generator->allocate_body(data->body_data);
        if (!body) {
                ast_log(LOG_WARNING, "Unable to allocate a NOTIFY body of type %s/%s\n",
                                type, subtype);
                return -1;
        }
 
-       if (generator->generate_body_content(body, data)) {
+       if (generator->generate_body_content(body, data->body_data)) {
                res = -1;
                goto end;
        }
@@ -1541,7 +1552,7 @@ int ast_sip_pubsub_generate_body_content(const char *type, const char *subtype,
        AST_RWLIST_TRAVERSE(&body_supplements, supplement, list) {
                if (!strcmp(generator->type, supplement->type) &&
                                !strcmp(generator->subtype, supplement->subtype)) {
-                       res = supplement->supplement_body(body, data);
+                       res = supplement->supplement_body(body, data->body_data);
                        if (res) {
                                break;
                        }

diff --git a/res/res_pjsip_xpidf_body_generator.c b/res/res_pjsip_xpidf_body_generator.c
index a844cab9369d72153a9ed6012cc2ddaa648b1307..42158ef39c72c385a14b5fb8510c0114374511d7 100644 (file)
--- a/res/res_pjsip_xpidf_body_generator.c
+++ b/res/res_pjsip_xpidf_body_generator.c
@@ -126,6 +126,7 @@ static void xpidf_to_string(void *body, struct ast_str **str)
 static struct ast_sip_pubsub_body_generator xpidf_body_generator = {
        .type = "application",
        .subtype = "xpidf+xml",
+       .body_type = AST_SIP_EXTEN_STATE_DATA,
        .allocate_body = xpidf_allocate_body,
        .generate_body_content = xpidf_generate_body_content,
        .to_string = xpidf_to_string,
@@ -135,6 +136,7 @@ static struct ast_sip_pubsub_body_generator xpidf_body_generator = {
 static struct ast_sip_pubsub_body_generator cpim_pidf_body_generator = {
        .type = "application",
        .subtype = "cpim-pidf+xml",
+       .body_type = AST_SIP_EXTEN_STATE_DATA,
        .allocate_body = xpidf_allocate_body,
        .generate_body_content = xpidf_generate_body_content,
        .to_string = xpidf_to_string,