Use source address of request as SDP session and media address

[juha-h]

This PR always keep SDP session and media addresses up to date with request source address.

[cspiel1]

I think that src/sip should make no assumptions about the structure of the SIP body. So I suppose that SDP has to be built, encoded outside and then passed to function sip_request(), or similar as buffer.

[juha-h]

I think that src/sip should make no assumptions about the structure of the SIP body. So I suppose that SDP has to be built, encoded outside and then passed to function sip_request(), or similar as buffer.

I don't think that src/sip makes any assumptions about structure of SIP body. It just checks if message body is SDP and if so, updates its laddr using API function. Body is SDP if req->sdp is set.

Edit: In my tests all requests that baresip/re knows about had valid message body when they were sent out.

[juha-h]

Just a summary. This PR is functionally backwards compatible with current behavior. The only API change is in sipsess_connect() function that now takes one more argument (SDP session). Current (buggy) behavior can be preserved by setting value of that argument to NULL.

[juha-h]

Sebastian Reimers writes:

The hint is the double pointer, we have the same description on many places

Yes, fine.

[juha-h]

In this PR, sip/request.c request() function has this piece of code:

	/* add request line and Via header */
	err  = mbuf_printf(mb, "%s %s SIP/2.0\r\n", req->met, req->uri);
	err |= mbuf_printf(mb, "Via: SIP/2.0/%s %J;branch=%s;rport\r\n",
			   sip_transp_name(tp), &laddr, branch);

	/* add Contact header */
	err |= req->sendh ? req->sendh(tp, &laddr, dst, mb, req->arg) : 0;
	err |= mbuf_write_mem(mb, mbuf_buf(req->mb), mbuf_get_left(req->mb));

The name of the called handler is sendh, but actually it is defined in reg/reg.c and only adds contact header, nothing else.

It would seem logical that sendh would handle also the body of the request or there could be more than one handler, for example, message header handler and message body handler. The latter would be set based on what kind of body type is in question, for example, sdp handler and another one just adds Content-Length header and copies the body text to mb.

I don't propose any changes now, since I would not like to delay the fix. The bug was opened against my Android app more than two years ago on Oct 11, 2019 and I would like to finally close it.

[cspiel1]

It would seem logical that sendh would handle also the body of the request or there could be more than one handler, for example, message header handler and message body handler. The latter would be set based on what kind of body type is in question, for example, sdp handler and another one just adds Content-Length header and copies the body text to mb.

This is a good idea. Will have a look on it soon.

[juha-h]

I was thinking that perhaps sipsess_connect() struct mbuf *desc argument could be replaced with

sip_body_h *bodyh, void *barg

arguments. Then in call.c send_invite, they could have values

body_handler, call->sdp

where body_handler is defined like this:

static int body_handler(const struct sa *laddr, struct mbuf *mb, void *arg)
{
	struct sdp_session *sess;
	struct mbuf *bb = NULL;
	int err = 0;

	sess = (struct sdp_session *)arg;
	if (sess) {
		sdp_session_set_laddr(sess, laddr);
		err = sdp_encode(&bb, sess, true);
		bb->pos = 0;
		err |= mbuf_printf(mb, "Content-Length: %zu\r\n",
				   mbuf_get_left(bb));
		if (mbuf_get_left(bb) > 0) {
			err |= mbuf_write_str(mb, "\r\n");
			err |= mbuf_write_mem(mb, mbuf_buf(bb),
					      mbuf_get_left(bb));
		}
		mem_deref(bb);
	}

	return err;
}

[cspiel1]

I am currently working on a similar approach.

[juha-h]

I am currently working on a similar approach.

Good, so I don't need to double the work.

[cspiel1]

re PR: #172
retest PR: baresip/retest#34
baresip PR: baresip/baresip#1645

It works already. But have to fix a shortly added unit test in baresip.

[juha-h]

Christian Spielberger writes:

re PR: #172 retest PR: baresip/retest#34 baresip PR: baresip/baresip#1645 It works already. But have to fix a shortly added unit test in baresip.

Thanks, I'll test tomorrow.

[cspiel1]

Christian Spielberger writes:
re PR: #172 retest PR: baresip/retest#34 baresip PR: baresip/baresip#1645 It works already. But have to fix a shortly added unit test in baresip.
Thanks, I'll test tomorrow.

Ok, thanks! I tested only with IP address so far and checked if the SIP Header + Content looks ok.

[juha-h]

Now the test described in baresip/baresip#1631 (comment) worked!

One comment: is the concept of "best effort laddr" still needed?

               [email protected]
[email protected]: selected for request
call uri: sip:[email protected]
ua: using best effort laddr 192.168.213.45

[juha-h]

I'll close this in favor of #172.

[cspiel1]

Now the test described in baresip/baresip#1631 (comment) worked!

One comment: is the concept of "best effort laddr" still needed?

               [email protected]
[email protected]: selected for request
call uri: sip:[email protected]
ua: using best effort laddr 192.168.213.45

This is a good question.