[asterisk-commits] dlee: branch 1.8 r379392 - in /branches/1.8/channels: ./ sip/ sip/include/
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Thu Jan 17 23:24:02 CST 2013
Author: dlee
Date: Thu Jan 17 23:23:57 2013
New Revision: 379392
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=379392
Log:
Fix Record-Route parsing for large headers.
Record-Route parsing copied the header into a char[256] array, which can
be a problem if the header is longer than that. This patch parses the
header in place, without the copy, avoiding the issue.
In addition to the original patch, I added a unit test for the new
get_in_brackets_const function.
(closes issue ASTERISK-20837)
Reported by: Corey Farrell
Patches:
chan_sip-build_route-optimized-rev1.patch uploaded by Corey Farrell (license 5909)
(with minor changes by dlee)
Modified:
branches/1.8/channels/chan_sip.c
branches/1.8/channels/sip/include/reqresp_parser.h
branches/1.8/channels/sip/reqresp_parser.c
Modified: branches/1.8/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_sip.c?view=diff&rev=379392&r1=379391&r2=379392
==============================================================================
--- branches/1.8/channels/chan_sip.c (original)
+++ branches/1.8/channels/chan_sip.c Thu Jan 17 23:23:57 2013
@@ -14727,21 +14727,25 @@
/* 1st we pass through all the hops in any Record-Route headers */
for (;;) {
/* Each Record-Route header */
- char rr_copy[256];
- char *rr_copy_ptr;
- char *rr_iter;
+ int len = 0;
+ const char *uri;
rr = __get_header(req, "Record-Route", &start);
if (*rr == '\0') {
break;
}
- ast_copy_string(rr_copy, rr, sizeof(rr_copy));
- rr_copy_ptr = rr_copy;
- while ((rr_iter = strsep(&rr_copy_ptr, ","))) { /* Each route entry */
- char *uri = get_in_brackets(rr_iter);
- len = strlen(uri) + 1;
- /* Make a struct route */
+ while (!get_in_brackets_const(rr, &uri, &len)) {
+ len++;
+ rr = strchr(rr, ',');
+ if(rr >= uri && rr < (uri + len)) {
+ /* comma inside brackets*/
+ const char *next_br = strchr(rr, '<');
+ if (next_br && next_br < (uri + len)) {
+ rr++;
+ continue;
+ }
+ continue;
+ }
if ((thishop = ast_malloc(sizeof(*thishop) + len))) {
- /* ast_calloc is not needed because all fields are initialized in this block */
ast_copy_string(thishop->hop, uri, len);
ast_debug(2, "build_route: Record-Route hop: <%s>\n", thishop->hop);
/* Link in */
@@ -14764,6 +14768,13 @@
tail = thishop;
}
}
+ rr = strchr(uri + len, ',');
+ if (rr == NULL) {
+ /* No more field-values, we're done with this header */
+ break;
+ }
+ /* Advance past comma */
+ rr++;
}
}
@@ -31343,6 +31354,59 @@
return res;
}
+AST_TEST_DEFINE(get_in_brackets_const_test)
+{
+ const char *input;
+ const char *start = NULL;
+ int len = 0;
+ int res;
+
+#define CHECK_RESULTS(in, expected_res, expected_start, expected_len) do { \
+ input = (in); \
+ res = get_in_brackets_const(input, &start, &len); \
+ if ((expected_res) != res) { \
+ ast_test_status_update(test, "Unexpected result: %d != %d\n", expected_res, res); \
+ return AST_TEST_FAIL; \
+ } \
+ if ((expected_start) != start) { \
+ const char *e = expected_start ? expected_start : "(null)"; \
+ const char *a = start ? start : "(null)"; \
+ ast_test_status_update(test, "Unexpected start: %s != %s\n", e, a); \
+ return AST_TEST_FAIL; \
+ } \
+ if ((expected_len) != len) { \
+ ast_test_status_update(test, "Unexpected len: %d != %d\n", expected_len, len); \
+ return AST_TEST_FAIL; \
+ } \
+ } while(0)
+
+ switch (cmd) {
+ case TEST_INIT:
+ info->name = __func__;
+ info->category = "/channels/chan_sip/";
+ info->summary = "get_in_brackets_const test";
+ info->description =
+ "Tests the get_in_brackets_const function";
+ return AST_TEST_NOT_RUN;
+ case TEST_EXECUTE:
+ break;
+ }
+
+ CHECK_RESULTS("", 1, NULL, -1);
+ CHECK_RESULTS("normal <test>", 0, input + 8, 4);
+ CHECK_RESULTS("\"normal\" <test>", 0, input + 10, 4);
+ CHECK_RESULTS("not normal <test", -1, NULL, -1);
+ CHECK_RESULTS("\"yes < really\" <test>", 0, input + 16, 4);
+ CHECK_RESULTS("\"even > this\" <test>", 0, input + 15, 4);
+ CHECK_RESULTS("<sip:id1 at 10.10.10.10;lr>", 0, input + 1, 22);
+ CHECK_RESULTS("<sip:id1 at 10.10.10.10;lr>, <sip:id1 at 10.10.10.20;lr>", 0, input + 1, 22);
+ CHECK_RESULTS("<sip:id1,id2 at 10.10.10.10;lr>", 0, input + 1, 26);
+ CHECK_RESULTS("<sip:id1 at 10., <sip:id2 at 10.10.10.10;lr>", 0, input + 1, 36);
+ CHECK_RESULTS("\"quoted text\" <sip:dlg1 at 10.10.10.10;lr>", 0, input + 15, 23);
+
+ return AST_TEST_PASS;
+}
+
#endif
#define DATA_EXPORT_SIP_PEER(MEMBER) \
@@ -31566,6 +31630,7 @@
AST_TEST_REGISTER(test_sip_peers_get);
AST_TEST_REGISTER(test_sip_mwi_subscribe_parse);
AST_TEST_REGISTER(test_tcp_message_fragmentation);
+ AST_TEST_REGISTER(get_in_brackets_const_test);
#endif
/* Register AstData providers */
@@ -31679,6 +31744,7 @@
AST_TEST_UNREGISTER(test_sip_peers_get);
AST_TEST_UNREGISTER(test_sip_mwi_subscribe_parse);
AST_TEST_UNREGISTER(test_tcp_message_fragmentation);
+ AST_TEST_UNREGISTER(get_in_brackets_const_test);
#endif
/* Unregister all the AstData providers */
ast_data_unregister(NULL);
Modified: branches/1.8/channels/sip/include/reqresp_parser.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/sip/include/reqresp_parser.h?view=diff&rev=379392&r1=379391&r2=379392
==============================================================================
--- branches/1.8/channels/sip/include/reqresp_parser.h (original)
+++ branches/1.8/channels/sip/include/reqresp_parser.h Thu Jan 17 23:23:57 2013
@@ -89,6 +89,17 @@
* \endverbatim
*/
char *get_in_brackets(char *tmp);
+
+/*! \brief Get text in brackets on a const without copy
+ *
+ * \param src String to search
+ * \param[out] start Set to first character inside left bracket.
+ * \param[out] length Set to lenght of string inside brackets
+ * \retval 0 success
+ * \retval -1 failure
+ * \retval 1 no brackets so got all
+ */
+int get_in_brackets_const(const char *src,const char **start,int *length);
/*! \brief Get text in brackets and any trailing residue
*
Modified: branches/1.8/channels/sip/reqresp_parser.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/sip/reqresp_parser.c?view=diff&rev=379392&r1=379391&r2=379392
==============================================================================
--- branches/1.8/channels/sip/reqresp_parser.c (original)
+++ branches/1.8/channels/sip/reqresp_parser.c Thu Jan 17 23:23:57 2013
@@ -946,6 +946,59 @@
ast_free(number);
return res;
+}
+
+int get_in_brackets_const(const char *src,const char **start,int *length)
+{
+ const char *parse = src;
+ const char *first_bracket;
+ const char *second_bracket;
+
+ if (start == NULL) {
+ return -1;
+ }
+ if (length == NULL) {
+ return -1;
+ }
+ *start = NULL;
+ *length = -1;
+ if (ast_strlen_zero(src)) {
+ return 1;
+ }
+
+ /*
+ * Skip any quoted text until we find the part in brackets.
+ * On any error give up and return -1
+ */
+ while ( (first_bracket = strchr(parse, '<')) ) {
+ const char *first_quote = strchr(parse, '"');
+ first_bracket++;
+ if (!first_quote || first_quote >= first_bracket) {
+ break; /* no need to look at quoted part */
+ }
+ /* the bracket is within quotes, so ignore it */
+ parse = find_closing_quote(first_quote + 1, NULL);
+ if (!*parse) {
+ ast_log(LOG_WARNING, "No closing quote found in '%s'\n", src);
+ return -1;
+ }
+ parse++;
+ }
+
+ /* Require a first bracket. Unlike get_in_brackets_full, this procedure is passed a const,
+ * so it can expect a pointer to an original value */
+ if (!first_bracket) {
+ ast_log(LOG_WARNING, "No opening bracket found in '%s'\n", src);
+ return 1;
+ }
+
+ if ((second_bracket = strchr(first_bracket, '>'))) {
+ *start = first_bracket;
+ *length = second_bracket - first_bracket;
+ return 0;
+ }
+ ast_log(LOG_WARNING, "No closing bracket found in '%s'\n", src);
+ return -1;
}
int get_in_brackets_full(char *tmp,char **out,char **residue)
More information about the asterisk-commits
mailing list