[Asterisk-code-review] res_format_attr_*: Parameter Names are Case-Insensitive. (asterisk[18])

Joshua Colp asteriskteam at digium.com
Wed Mar 10 04:22:20 CST 2021


Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15571 )

Change subject: res_format_attr_*: Parameter Names are Case-Insensitive.
......................................................................

res_format_attr_*: Parameter Names are Case-Insensitive.

see RFC 4855:
parameter names are case-insensitive both in media type strings and
in the default mapping to the SDP a=fmtp attribute.

This change is required for H.263+ because some implementations are
known to use even mixed-case. This does not fix ASTERISK~29268 because
H.264 was not fixed. This approach here lowers/uppers both parameter
names and parameter values. H.264 needs a different approach because
one of its parameter values is not case-insensitive:
sprop-parameter-sets is Base64.

Change-Id: Idf2a73457be231647aed3c87b1da197afba86892
---
M res/res_format_attr_celt.c
M res/res_format_attr_h263.c
M res/res_format_attr_ilbc.c
M res/res_format_attr_opus.c
M res/res_format_attr_silk.c
M res/res_format_attr_siren14.c
M res/res_format_attr_siren7.c
M res/res_format_attr_vp8.c
8 files changed, 100 insertions(+), 21 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
  George Joseph: Looks good to me, approved



diff --git a/res/res_format_attr_celt.c b/res/res_format_attr_celt.c
index 4b92376..24e6670 100644
--- a/res/res_format_attr_celt.c
+++ b/res/res_format_attr_celt.c
@@ -29,8 +29,14 @@
 
 #include "asterisk.h"
 
+#include <ctype.h>                      /* for tolower */
+
 #include "asterisk/module.h"
 #include "asterisk/format.h"
+#include "asterisk/astobj2.h"           /* for ao2_ref */
+#include "asterisk/logger.h"            /* for ast_log, LOG_WARNING */
+#include "asterisk/strings.h"           /* for ast_str_append */
+#include "asterisk/utils.h"             /* for MIN */
 
 /*!
  * \brief CELT attribute structure.
@@ -70,6 +76,7 @@
 
 static struct ast_format *celt_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
 {
+	char *attribs = ast_strdupa(attributes), *attrib;
 	struct ast_format *cloned;
 	struct celt_attr *attr;
 	unsigned int val;
@@ -80,7 +87,12 @@
 	}
 	attr = ast_format_get_attribute_data(cloned);
 
-	if (sscanf(attributes, "framesize=%30u", &val) == 1) {
+	/* lower-case everything, so we are case-insensitive */
+	for (attrib = attribs; *attrib; ++attrib) {
+		*attrib = tolower(*attrib);
+	} /* based on channels/chan_sip.c:process_a_sdp_image() */
+
+	if (sscanf(attribs, "framesize=%30u", &val) == 1) {
 		attr->framesize = val;
 	}
 
diff --git a/res/res_format_attr_h263.c b/res/res_format_attr_h263.c
index 5246725..70d7591 100644
--- a/res/res_format_attr_h263.c
+++ b/res/res_format_attr_h263.c
@@ -33,8 +33,12 @@
 
 #include "asterisk.h"
 
+#include <ctype.h>                      /* for toupper */
+
 #include "asterisk/module.h"
 #include "asterisk/format.h"
+#include "asterisk/strings.h"           /* for ast_str_append */
+#include "asterisk/utils.h"             /* for ast_strip */
 
 /*! \brief Value that indicates an attribute is actually unset */
 #define H263_ATTR_KEY_UNSET UINT8_MAX
@@ -174,6 +178,11 @@
 	}
 	attr = ast_format_get_attribute_data(cloned);
 
+	/* upper-case everything, so we are case-insensitive */
+	for (attrib = attribs; *attrib; ++attrib) {
+		*attrib = toupper(*attrib);
+	} /* based on channels/chan_sip.c:process_a_sdp_image() */
+
 	attr->BPP = H263_ATTR_KEY_UNSET;
 	attr->MaxBR = H263_ATTR_KEY_UNSET;
 	attr->PAR_WIDTH = H263_ATTR_KEY_UNSET;
diff --git a/res/res_format_attr_ilbc.c b/res/res_format_attr_ilbc.c
index 0fac55c..ec33599 100644
--- a/res/res_format_attr_ilbc.c
+++ b/res/res_format_attr_ilbc.c
@@ -31,10 +31,12 @@
 
 #include "asterisk.h"
 
+#include <ctype.h>                      /* for tolower */
+
 #include "asterisk/module.h"
 #include "asterisk/format.h"
 #include "asterisk/strings.h"           /* for ast_str_append */
-#include "asterisk/utils.h"             /* for ast_calloc, ast_free */
+#include "asterisk/utils.h"             /* for ast_free */
 
 #include "asterisk/ilbc.h"
 
@@ -71,6 +73,7 @@
 
 static struct ast_format *ilbc_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
 {
+	char *attribs = ast_strdupa(attributes), *attrib;
 	struct ast_format *cloned;
 	struct ilbc_attr *attr;
 	const char *kvp;
@@ -82,7 +85,12 @@
 	}
 	attr = ast_format_get_attribute_data(cloned);
 
-	if ((kvp = strstr(attributes, "mode")) && sscanf(kvp, "mode=%30u", &val) == 1) {
+	/* lower-case everything, so we are case-insensitive */
+	for (attrib = attribs; *attrib; ++attrib) {
+		*attrib = tolower(*attrib);
+	} /* based on channels/chan_sip.c:process_a_sdp_image() */
+
+	if ((kvp = strstr(attribs, "mode")) && sscanf(kvp, "mode=%30u", &val) == 1) {
 		attr->mode = val;
 	} else {
 		attr->mode = 30; /* optional attribute; 30 is default value */
diff --git a/res/res_format_attr_opus.c b/res/res_format_attr_opus.c
index cf30cf5..be6b611 100644
--- a/res/res_format_attr_opus.c
+++ b/res/res_format_attr_opus.c
@@ -29,8 +29,11 @@
 
 #include "asterisk.h"
 
+#include <ctype.h>
+
 #include "asterisk/module.h"
 #include "asterisk/format.h"
+#include "asterisk/astobj2.h"
 #include "asterisk/logger.h"
 #include "asterisk/strings.h"
 #include "asterisk/utils.h"
@@ -135,6 +138,7 @@
 
 static struct ast_format *opus_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
 {
+	char *attribs = ast_strdupa(attributes), *attrib;
 	struct ast_format *cloned;
 	struct opus_attr *attr;
 
@@ -145,22 +149,27 @@
 
 	attr = ast_format_get_attribute_data(cloned);
 
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_MAX_PLAYBACK_RATE, &attr->maxplayrate);
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_MAX_CODED_AUDIO_BANDWIDTH,
+	/* lower-case everything, so we are case-insensitive */
+	for (attrib = attribs; *attrib; ++attrib) {
+		*attrib = tolower(*attrib);
+	} /* based on channels/chan_sip.c:process_a_sdp_image() */
+
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_MAX_PLAYBACK_RATE, &attr->maxplayrate);
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_MAX_CODED_AUDIO_BANDWIDTH,
 		&attr->maxplayrate);
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_SPROP_MAX_CAPTURE_RATE,
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_SPROP_MAX_CAPTURE_RATE,
 		&attr->spropmaxcapturerate);
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_MAX_PTIME, &attr->maxptime);
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_PTIME, &attr->ptime);
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_MAX_AVERAGE_BITRATE, &attr->maxbitrate);
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_STEREO, &attr->stereo);
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_MAX_PTIME, &attr->maxptime);
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_PTIME, &attr->ptime);
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_MAX_AVERAGE_BITRATE, &attr->maxbitrate);
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_STEREO, &attr->stereo);
 	if (attr->stereo) {
 		ast_format_set_channel_count(cloned, 2);
 	}
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_SPROP_STEREO, &attr->spropstereo);
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_CBR, &attr->cbr);
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_FEC, &attr->fec);
-	sdp_fmtp_get(attributes, CODEC_OPUS_ATTR_DTX, &attr->dtx);
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_SPROP_STEREO, &attr->spropstereo);
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_CBR, &attr->cbr);
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_FEC, &attr->fec);
+	sdp_fmtp_get(attribs, CODEC_OPUS_ATTR_DTX, &attr->dtx);
 
 	return cloned;
 }
diff --git a/res/res_format_attr_silk.c b/res/res_format_attr_silk.c
index e2c9ca1..c0a4885 100644
--- a/res/res_format_attr_silk.c
+++ b/res/res_format_attr_silk.c
@@ -29,8 +29,13 @@
 
 #include "asterisk.h"
 
+#include <ctype.h>                      /* for tolower */
+
 #include "asterisk/module.h"
 #include "asterisk/format.h"
+#include "asterisk/logger.h"            /* for ast_log, LOG_WARNING */
+#include "asterisk/strings.h"           /* for ast_str_append */
+#include "asterisk/utils.h"             /* for MAX, MIN */
 
 /*!
  * \brief SILK attribute structure.
@@ -78,6 +83,7 @@
 
 static struct ast_format *silk_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
 {
+	char *attribs = ast_strdupa(attributes), *attrib;
 	struct ast_format *cloned;
 	struct silk_attr *attr;
 	unsigned int val;
@@ -88,13 +94,18 @@
 	}
 	attr = ast_format_get_attribute_data(cloned);
 
-	if (sscanf(attributes, "maxaveragebitrate=%30u", &val) == 1) {
+	/* lower-case everything, so we are case-insensitive */
+	for (attrib = attribs; *attrib; ++attrib) {
+		*attrib = tolower(*attrib);
+	} /* based on channels/chan_sip.c:process_a_sdp_image() */
+
+	if (sscanf(attribs, "maxaveragebitrate=%30u", &val) == 1) {
 		attr->maxbitrate = val;
 	}
-	if (sscanf(attributes, "usedtx=%30u", &val) == 1) {
+	if (sscanf(attribs, "usedtx=%30u", &val) == 1) {
 		attr->dtx = val;
 	}
-	if (sscanf(attributes, "useinbandfec=%30u", &val) == 1) {
+	if (sscanf(attribs, "useinbandfec=%30u", &val) == 1) {
 		attr->fec = val;
 	}
 
diff --git a/res/res_format_attr_siren14.c b/res/res_format_attr_siren14.c
index 2cc2a2f..111ec47 100644
--- a/res/res_format_attr_siren14.c
+++ b/res/res_format_attr_siren14.c
@@ -29,8 +29,13 @@
 
 #include "asterisk.h"
 
+#include <ctype.h>                      /* for tolower */
+
 #include "asterisk/module.h"
 #include "asterisk/format.h"
+#include "asterisk/astobj2.h"           /* for ao2_bump */
+#include "asterisk/logger.h"            /* for ast_log, LOG_WARNING */
+#include "asterisk/strings.h"           /* for ast_str_append */
 
 /* Destroy is a required callback and must exist */
 static void siren14_destroy(struct ast_format *format)
@@ -45,9 +50,15 @@
 
 static struct ast_format *siren14_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
 {
+	char *attribs = ast_strdupa(attributes), *attrib;
 	unsigned int val;
 
-	if (sscanf(attributes, "bitrate=%30u", &val) == 1) {
+	/* lower-case everything, so we are case-insensitive */
+	for (attrib = attribs; *attrib; ++attrib) {
+		*attrib = tolower(*attrib);
+	} /* based on channels/chan_sip.c:process_a_sdp_image() */
+
+	if (sscanf(attribs, "bitrate=%30u", &val) == 1) {
 		if (val != 48000) {
 			ast_log(LOG_WARNING, "Got siren14 offer at %u bps, but only 48000 bps supported; ignoring.\n", val);
 			return NULL;
diff --git a/res/res_format_attr_siren7.c b/res/res_format_attr_siren7.c
index 7f4f165..72b0ff2 100644
--- a/res/res_format_attr_siren7.c
+++ b/res/res_format_attr_siren7.c
@@ -29,8 +29,13 @@
 
 #include "asterisk.h"
 
+#include <ctype.h>                      /* for tolower */
+
 #include "asterisk/module.h"
 #include "asterisk/format.h"
+#include "asterisk/astobj2.h"           /* for ao2_bump */
+#include "asterisk/logger.h"            /* for ast_log, LOG_WARNING */
+#include "asterisk/strings.h"           /* for ast_str_append */
 
 /* Destroy is a required callback and must exist */
 static void siren7_destroy(struct ast_format *format)
@@ -45,9 +50,15 @@
 
 static struct ast_format *siren7_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
 {
+	char *attribs = ast_strdupa(attributes), *attrib;
 	unsigned int val;
 
-	if (sscanf(attributes, "bitrate=%30u", &val) == 1) {
+	/* lower-case everything, so we are case-insensitive */
+	for (attrib = attribs; *attrib; ++attrib) {
+		*attrib = tolower(*attrib);
+	} /* based on channels/chan_sip.c:process_a_sdp_image() */
+
+	if (sscanf(attribs, "bitrate=%30u", &val) == 1) {
 		if (val != 32000) {
 			ast_log(LOG_WARNING, "Got Siren7 offer at %u bps, but only 32000 bps supported; ignoring.\n", val);
 			return NULL;
diff --git a/res/res_format_attr_vp8.c b/res/res_format_attr_vp8.c
index f9babd8..c54e4ac 100644
--- a/res/res_format_attr_vp8.c
+++ b/res/res_format_attr_vp8.c
@@ -31,6 +31,8 @@
 
 #include "asterisk.h"
 
+#include <ctype.h>                      /* for tolower */
+
 #include "asterisk/module.h"
 #include "asterisk/format.h"
 #include "asterisk/logger.h"            /* for ast_log, LOG_WARNING */
@@ -76,6 +78,7 @@
 
 static struct ast_format *vp8_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
 {
+	char *attribs = ast_strdupa(attributes), *attrib;
 	struct ast_format *cloned;
 	struct vp8_attr *attr;
 	const char *kvp;
@@ -87,13 +90,18 @@
 	}
 	attr = ast_format_get_attribute_data(cloned);
 
-	if ((kvp = strstr(attributes, "max-fr")) && sscanf(kvp, "max-fr=%30u", &val) == 1) {
+	/* lower-case everything, so we are case-insensitive */
+	for (attrib = attribs; *attrib; ++attrib) {
+		*attrib = tolower(*attrib);
+	} /* based on channels/chan_sip.c:process_a_sdp_image() */
+
+	if ((kvp = strstr(attribs, "max-fr")) && sscanf(kvp, "max-fr=%30u", &val) == 1) {
 		attr->maximum_frame_rate = val;
 	} else {
 		attr->maximum_frame_rate = UINT_MAX;
 	}
 
-	if ((kvp = strstr(attributes, "max-fs")) && sscanf(kvp, "max-fs=%30u", &val) == 1) {
+	if ((kvp = strstr(attribs, "max-fs")) && sscanf(kvp, "max-fs=%30u", &val) == 1) {
 		attr->maximum_frame_size = val;
 	} else {
 		attr->maximum_frame_size = UINT_MAX;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15571
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: Idf2a73457be231647aed3c87b1da197afba86892
Gerrit-Change-Number: 15571
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210310/9ce96f35/attachment-0001.html>


More information about the asterisk-code-review mailing list