pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39240?usp=email )
Change subject: mgw: DLCX: Split mgcp header pars parsing into a previous step ......................................................................
mgw: DLCX: Split mgcp header pars parsing into a previous step
Do most of the initial parsing and verification in a prior step, filling in a "parsed" struct which simplifies logic coming after for different message types.
Change-Id: I557a3a257ddefedc479a4aff974a74c4e4e2c85d --- M src/libosmo-mgcp/mgcp_protocol.c 1 file changed, 38 insertions(+), 49 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/40/39240/1
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index c82911c..d066f29 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -1222,13 +1222,12 @@ struct mgcp_parse_data *pdata = rq->pdata; struct mgcp_trunk *trunk = rq->trunk; struct mgcp_endpoint *endp = rq->endp; + struct mgcp_parse_hdr_pars *hpars = &pdata->hpars; struct rate_ctr_group *rate_ctrs; - int error_code = 400; - char *line; char stats[1048]; - const char *conn_id = NULL; struct mgcp_conn *conn = NULL; unsigned int i; + int rc;
/* NOTE: In this handler we can not take it for granted that the endp * pointer will be populated, however a trunk is always guaranteed (except for 'null' endp). @@ -1270,49 +1269,42 @@ return create_ok_response(trunk, NULL, 200, "DLCX", pdata->trans); }
- for_each_line(line, pdata->save) { - if (!mgcp_check_param(line)) - continue; + rc = mgcp_parse_hdr_pars(pdata); + switch (rc) { + case 0: + break; /* all good, continue below */ + case 539: + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); + return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans); + default: + return create_err_response(rq->trunk, NULL, rc, "DLCX", pdata->trans); + }
- switch (toupper(line[0])) { - case 'C': - /* If we have no endpoint, but a call id in the request, - then this request cannot be handled */ - if (!endp) { - LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, - "cannot handle requests with call-id (C) without endpoint -- abort!"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); - return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans); - } - - if (mgcp_verify_call_id(endp, line + 3) != 0) { - error_code = 516; - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CALLID)); - goto error3; - } - break; - case 'I': - /* If we have no endpoint, but a connection id in the request, - then this request cannot be handled */ - if (!endp) { - LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, - "cannot handle requests with conn-id (I) without endpoint -- abort!"); - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); - return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans); - } - - conn_id = (const char *)line + 3; - if ((error_code = mgcp_verify_ci(endp, conn_id))) { - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID)); - goto error3; - } - break; - default: - LOGPEPTR(endp, trunk, DLMGCP, LOGL_NOTICE, "DLCX: Unhandled MGCP option: '%c'/%d\n", - line[0], line[0]); + if (hpars->callid) { + /* If we have no endpoint, but a call id in the request, then this request cannot be handled */ + if (!endp) { + LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, + "cannot handle requests with call-id (C) without endpoint -- abort!"); rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans); - break; + } + if (mgcp_verify_call_id(endp, hpars->callid) != 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CALLID)); + return create_err_response(endp, endp, 516, "DLCX", pdata->trans); + } + } + + if (hpars->connid) { + /* If we have no endpoint, but a connection id in the request, then this request cannot be handled */ + if (!endp) { + LOGPTRUNK(trunk, DLMGCP, LOGL_NOTICE, + "cannot handle requests with conn-id (I) without endpoint -- abort!"); + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_UNHANDLED_PARAM)); + return create_err_response(rq->trunk, NULL, 539, "DLCX", pdata->trans); + } + if ((rc = mgcp_verify_ci(endp, hpars->connid)) != 0) { + rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID)); + return create_err_response(endp, endp, rc, "DLCX", pdata->trans); } }
@@ -1324,7 +1316,7 @@ * wildcarded DLCX that refers to the selected endpoint. This means * that we drop all connections on that specific endpoint at once. * (See also RFC3435 Section F.7) */ - if (!conn_id) { + if (!hpars->connid) { int num_conns = mgcp_endp_num_conns(endp); LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "DLCX: missing ci (connectionIdentifier), will remove all connections (%d total) at once\n", @@ -1342,10 +1334,10 @@ }
/* Find the connection */ - conn = mgcp_endp_get_conn(endp, conn_id); + conn = mgcp_endp_get_conn(endp, hpars->connid); if (!conn) { rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_INVALID_CONNID)); - goto error3; + return create_err_response(endp, endp, 400, "DLCX", pdata->trans); } /* save the statistics of the current connection */ mgcp_format_stats(stats, sizeof(stats), conn); @@ -1366,9 +1358,6 @@
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_SUCCESS)); return create_ok_resp_with_param(endp, endp, 250, "DLCX", pdata->trans, stats); - -error3: - return create_err_response(endp, endp, error_code, "DLCX", pdata->trans); }
/* RSIP command handler, processes the received command */