res_pjsip_pubsub: re-re-fix persistent subscription storage.
authorMark Michelson <mmichelson@digium.com>
Tue, 1 Sep 2015 14:05:34 +0000 (09:05 -0500)
committerMark Michelson <mmichelson@digium.com>
Tue, 1 Sep 2015 14:41:10 +0000 (09:41 -0500)
A recent change to res_pjsip_pubsub switched to using pjsip_msg_print as
a means of writing an appropriate packet to persistent storage. While
this partially solved the issue, it had its own problems.
pjsip_msg_print will always add a Content-Length header to the message
it prints. Frequent restarts of Asterisk can result in persistent
subscriptions being written with five or more Content-Length headers. In
addition, sometimes some apparent corruption of individual headers could
be seen.

This aims to fix the problem by not running a parsed message through an
interpreter but rather by taking the raw message and saving it. The
logic for what to save is going to be different depending on whether a
SUBSCRIBE was received from the wire or if it was pulled from
persistence. When receiving a packet from the wire, when using a
streaming transport, the rdata->pkt_info.packet may contain multiple SIP
messages or fragments. However, the rdata->msg_info.msg_buf will always
contain the current SIP message to be processed. When pulling from
persistence, though, the rdata->msg_info.msg_buf will be NULL since no
transport actually handled the packet. However, since we know that we
will always ever pull one SIP message from persistence, we are free to
save directly from rdata->pkt_info.packet instead.

ASTERISK-25365 #close
Reported by Mark Michelson

Change-Id: I33153b10d0b4dc8e3801aaaee2f48173b867855b

res/res_pjsip_pubsub.c

index 2b42ad6..6c3d101 100644 (file)
@@ -561,8 +561,19 @@ static void subscription_persistence_update(struct sip_subscription_tree *sub_tr
                expires = expires_hdr ? expires_hdr->ivalue : DEFAULT_PUBLISH_EXPIRES;
                sub_tree->persistence->expires = ast_tvadd(ast_tvnow(), ast_samp2tv(expires, 1));
 
-               pjsip_msg_print(rdata->msg_info.msg, sub_tree->persistence->packet,
-                               sizeof(sub_tree->persistence->packet));
+               /* When receiving a packet on an streaming transport, it's possible to receive more than one SIP
+                * message at a time into the rdata->pkt_info.packet buffer. However, the rdata->msg_info.msg_buf
+                * will always point to the proper SIP message that is to be processed. When updating subscription
+                * persistence that is pulled from persistent storage, though, the rdata->pkt_info.packet will
+                * only ever have a single SIP message on it, and so we base persistence on that.
+                */
+               if (rdata->msg_info.msg_buf) {
+                       ast_copy_string(sub_tree->persistence->packet, rdata->msg_info.msg_buf,
+                                       MIN(sizeof(sub_tree->persistence->packet), rdata->msg_info.len));
+               } else {
+                       ast_copy_string(sub_tree->persistence->packet, rdata->pkt_info.packet,
+                                       sizeof(sub_tree->persistence->packet));
+               }
                ast_copy_string(sub_tree->persistence->src_name, rdata->pkt_info.src_name,
                                sizeof(sub_tree->persistence->src_name));
                sub_tree->persistence->src_port = rdata->pkt_info.src_port;