Change in osmo-mgw[master]: mgcp_conn_get(): compare conn Id ('I:') case insensitively
Neels Hofmeyr has submitted this change and it was merged. (
https://gerrit.osmocom.org/10677 )
Change subject: mgcp_conn_get(): compare conn Id ('I:') case insensitively
..
mgcp_conn_get(): compare conn Id ('I:') case insensitively
The Connection Identifier is defined as a hex string, so clients may send the
ID back in lower case. Convert to upper case before comparing.
A specific SCCPlite MSC is observed to DLCX with Connection Identifier in lower
case, which would mismatch pefore this patch.
Add test_conn_id_matching() in mgcp_test.c to verify case insensitivity.
Cosmetic: use strcmp(), not strncmp(). In the presence of a terminating nul as
we can assume here, this makes no functional difference, but it clarifies the
code.
Related: OS#3508
Depends: Ib0ee1206b9f31d7ba25c31f8008119ac55440797 (libosmocore)
Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
---
M src/libosmo-mgcp/mgcp_conn.c
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
3 files changed, 49 insertions(+), 1 deletion(-)
Approvals:
Harald Welte: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index 0126c7c..0918b8b 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -189,9 +189,18 @@
struct mgcp_conn *mgcp_conn_get(struct mgcp_endpoint *endp, const char *id)
{
struct mgcp_conn *conn;
+ const char *id_upper;
+
+ if (!id || !*id)
+ return NULL;
+
+ /* Use uppercase to compare identifiers, to avoid mismatches: RFC3435
2.1.3.2 "Names of
+* Connections" defines the id as a hex string, so clients may return
lower case hex even though
+* we sent upper case hex in the CRCX response. */
+ id_upper = osmo_str_toupper(id);
llist_for_each_entry(conn, >conns, entry) {
- if (strncmp(conn->id, id, sizeof(conn->id)) == 0)
+ if (strcmp(conn->id, id_upper) == 0)
return conn;
}
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index c40eabc..99ddd71 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -1751,6 +1751,40 @@
OSMO_ASSERT(pt_dst == -EINVAL);
}
+void test_conn_id_matching()
+{
+ struct mgcp_endpoint endp = {};
+ struct mgcp_conn *conn;
+ struct mgcp_conn *conn_match;
+ int i;
+ const char *conn_id_generated = "23AB";
+ const char *conn_id_request[] = {
+ "23AB",
+ "23ab",
+ };
+
+ printf("\nTesting %s\n", __func__);
+
+ INIT_LLIST_HEAD();
+
+ conn = talloc_zero(NULL, struct mgcp_conn);
+ OSMO_ASSERT(conn);
+ osmo_strlcpy(conn->id, conn_id_generated, sizeof(conn->id));
+ llist_add(>entry, );
+
+ for (i = 0; i < ARRAY_SIZE(conn_id_request); i++) {
+ const char *needle = conn_id_request[i];
+ printf("needle='%s' ", needle);
+ conn_match = mgcp_conn_get(, needle);
+ OSMO_ASSERT(conn_match);
+ printf("found '%s'\n", conn_match->id);
+ OSMO_ASSERT(conn_match == conn);
+ }
+
+ llist_del(>entry);
+ talloc_free(conn);
+}
+
int main(int argc, char **argv)
{
void *ctx = talloc_named_const(NULL, 0, "mgcp_test");
@@ -1775,6 +1809,7 @@
test_get_lco_identifier();
test_check_local_cx_options(ctx);
test_mgcp_codec_pt_translate();
+ test_conn_id_matching();
OSMO_ASSERT(talloc_total_size(msgb_ctx) == 0);
OSMO_ASSERT(talloc_total_blocks(msgb_ctx) == 1);
diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok
index ddda751..f50f487 100644
--- a/tests/mgcp/mgcp_test.ok
+++ b/tests/mgcp/mgcp_test.ok
@@ -1169,4 +1169,8 @@
'' -> '(null)'
p10, aPCMU -> (null)
'10,a :PCMU' -> '(null)'
+
+Testing test_conn_id_matching
+needle='23AB' found '23AB'
+needle='23ab' found '23AB'
Done
--
To view, visit https://gerrit.osmocom.org/10677
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
Gerrit-Change-Number: 10677
Gerrit-PatchSet: 5
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Change in osmo-mgw[master]: mgcp_conn_get(): compare conn Id ('I:') case insensitively
Harald Welte has posted comments on this change. (
https://gerrit.osmocom.org/10677 )
Change subject: mgcp_conn_get(): compare conn Id ('I:') case insensitively
..
Patch Set 5: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/10677
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
Gerrit-Change-Number: 10677
Gerrit-PatchSet: 5
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Comment-Date: Fri, 07 Sep 2018 09:10:16 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes
Change in osmo-mgw[master]: mgcp_conn_get(): compare conn Id ('I:') case insensitively
Neels Hofmeyr has posted comments on this change. (
https://gerrit.osmocom.org/10677 )
Change subject: mgcp_conn_get(): compare conn Id ('I:') case insensitively
..
Patch Set 5:
note the added "Depends:" on a libosmocore patch adding osmo_str_toupper()
--
To view, visit https://gerrit.osmocom.org/10677
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
Gerrit-Change-Number: 10677
Gerrit-PatchSet: 5
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Comment-Date: Fri, 07 Sep 2018 02:34:43 +
Gerrit-HasComments: No
Gerrit-HasLabels: No
Change in osmo-mgw[master]: mgcp_conn_get(): compare conn Id ('I:') case insensitively
Neels Hofmeyr has posted comments on this change. (
https://gerrit.osmocom.org/10677 )
Change subject: mgcp_conn_get(): compare conn Id ('I:') case insensitively
..
Set Ready For Review
--
To view, visit https://gerrit.osmocom.org/10677
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
Gerrit-Change-Number: 10677
Gerrit-PatchSet: 5
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Comment-Date: Fri, 07 Sep 2018 02:34:10 +
Gerrit-HasComments: No
Gerrit-HasLabels: No
Change in osmo-mgw[master]: mgcp_conn_get(): compare conn Id ('I:') case insensitively
Neels Hofmeyr has posted comments on this change. (
https://gerrit.osmocom.org/10677 )
Change subject: mgcp_conn_get(): compare conn Id ('I:') case insensitively
..
Patch Set 3: Code-Review-1
(1 comment)
https://gerrit.osmocom.org/#/c/10677/3/src/libosmo-mgcp/mgcp_conn.c
File src/libosmo-mgcp/mgcp_conn.c:
https://gerrit.osmocom.org/#/c/10677/3/src/libosmo-mgcp/mgcp_conn.c@202
PS3, Line 202: rc = osmo_strlcpy(id_upper, id, sizeof(id_upper));
this should use osmo_str2upper(), or maybe a new osmo_str_to_upper() with a
length check which I have in a libosmocore branch.
--
To view, visit https://gerrit.osmocom.org/10677
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
Gerrit-Change-Number: 10677
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Neels Hofmeyr
Gerrit-Comment-Date: Tue, 04 Sep 2018 14:45:47 +
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
Change in osmo-mgw[master]: mgcp_conn_get(): compare conn Id ('I:') case insensitively
Hello Harald Welte, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/10677
to look at the new patch set (#3).
Change subject: mgcp_conn_get(): compare conn Id ('I:') case insensitively
..
mgcp_conn_get(): compare conn Id ('I:') case insensitively
The Connection Identifier is defined as a hex string, so clients may send the
ID back in lower case. Convert to upper case before comparing.
A specific SCCPlite MSC is observed to DLCX with Connection Identifier in lower
case, which would mismatch pefore this patch.
Add test_conn_id_matching() in mgcp_test.c to verify case insensitivity.
Cosmetic: use strcmp(), not strncmp(). In the presence of a terminating nul as
we can assume here, this makes no functional difference, but it clarifies the
code.
Related: OS#3508
Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
---
M src/libosmo-mgcp/mgcp_conn.c
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
3 files changed, 57 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/77/10677/3
--
To view, visit https://gerrit.osmocom.org/10677
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
Gerrit-Change-Number: 10677
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Change in osmo-mgw[master]: mgcp_conn_get(): compare conn Id ('I:') case insensitively
Harald Welte has posted comments on this change. (
https://gerrit.osmocom.org/10677 )
Change subject: mgcp_conn_get(): compare conn Id ('I:') case insensitively
..
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/10677
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
Gerrit-Change-Number: 10677
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Comment-Date: Wed, 29 Aug 2018 07:16:40 +
Gerrit-HasComments: No
Gerrit-HasLabels: Yes
Change in osmo-mgw[master]: mgcp_conn_get(): compare conn Id ('I:') case insensitively
Neels Hofmeyr has uploaded this change for review. (
https://gerrit.osmocom.org/10677
Change subject: mgcp_conn_get(): compare conn Id ('I:') case insensitively
..
mgcp_conn_get(): compare conn Id ('I:') case insensitively
The Connection Identifier is defined as a hex string, so clients may send the
ID back in lower case. Convert to upper case before comparing.
A specific SCCPlite MSC is observed to DLCX with Connection Identifier in lower
case, which would mismatch pefore this patch.
Add test_conn_id_matching() in mgcp_test.c to verify case insensitivity.
Related: OS#3508
Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
---
M src/libosmo-mgcp/mgcp_conn.c
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
3 files changed, 52 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/77/10677/1
diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index 8d855f2..89c55ed 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -188,9 +188,21 @@
struct mgcp_conn *mgcp_conn_get(struct mgcp_endpoint *endp, const char *id)
{
struct mgcp_conn *conn;
+ char id_upper[MGCP_CONN_ID_LENGTH];
+ int i;
+
+ if (!id || !*id)
+ return NULL;
+
+ /* Use uppercase to compare identifiers, to avoid mismatches: RFC3435
2.1.3.2 "Names of
+* Connections" defines the id as a hex string, so clients may return
lower case hex even though
+* we sent upper case hex in the CRCX response. */
+ osmo_strlcpy(id_upper, id, sizeof(id_upper));
+ for (i = 0; i < MGCP_CONN_ID_LENGTH; i++)
+ id_upper[i] = toupper(id_upper[i]);
llist_for_each_entry(conn, >conns, entry) {
- if (strncmp(conn->id, id, sizeof(conn->id)) == 0)
+ if (strncmp(conn->id, id_upper, sizeof(conn->id)) == 0)
return conn;
}
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 2f3a8a7..bb1cd98 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -1744,6 +1744,40 @@
OSMO_ASSERT(pt_dst == -EINVAL);
}
+void test_conn_id_matching()
+{
+ struct mgcp_endpoint endp = {};
+ struct mgcp_conn *conn;
+ struct mgcp_conn *conn_match;
+ int i;
+ const char *conn_id_generated = "23AB";
+ const char *conn_id_request[] = {
+ "23AB",
+ "23ab",
+ };
+
+ printf("\nTesting %s\n", __func__);
+
+ INIT_LLIST_HEAD();
+
+ conn = talloc_zero(NULL, struct mgcp_conn);
+ OSMO_ASSERT(conn);
+ osmo_strlcpy(conn->id, conn_id_generated, sizeof(conn->id));
+ llist_add(>entry, );
+
+ for (i = 0; i < ARRAY_SIZE(conn_id_request); i++) {
+ const char *needle = conn_id_request[i];
+ printf("needle='%s' ", needle);
+ conn_match = mgcp_conn_get(, needle);
+ OSMO_ASSERT(conn_match);
+ printf("found '%s'\n", conn_match->id);
+ OSMO_ASSERT(conn_match == conn);
+ }
+
+ llist_del(>entry);
+ talloc_free(conn);
+}
+
int main(int argc, char **argv)
{
void *ctx = talloc_named_const(NULL, 0, "mgcp_test");
@@ -1768,6 +1802,7 @@
test_get_lco_identifier();
test_check_local_cx_options(ctx);
test_mgcp_codec_pt_translate();
+ test_conn_id_matching();
OSMO_ASSERT(talloc_total_size(msgb_ctx) == 0);
OSMO_ASSERT(talloc_total_blocks(msgb_ctx) == 1);
diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok
index 915d45e..34dcd14 100644
--- a/tests/mgcp/mgcp_test.ok
+++ b/tests/mgcp/mgcp_test.ok
@@ -1156,4 +1156,8 @@
'' -> '(null)'
p10, aPCMU -> (null)
'10,a :PCMU' -> '(null)'
+
+Testing test_conn_id_matching
+needle='23AB' found '23AB'
+needle='23ab' found '23AB'
Done
--
To view, visit https://gerrit.osmocom.org/10677
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8e52278c3abe9e9c8c848c2b1538bce443f68a43
Gerrit-Change-Number: 10677
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr
