Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow dialog establishment when remote does not provide To tag #3394

Merged
merged 2 commits into from
Feb 23, 2023

Conversation

nanangizz
Copy link
Member

To fix #1096.

The assertion occurs in the following line because the dialog is not established:

/* Dialog must have been established */
PJ_ASSERT_RETURN(inv->dlg->state == PJSIP_DIALOG_STATE_ESTABLISHED,
PJ_EINVALIDOP);

The standard (rfc3261 12.1.2 UAC Behavior) specifies that it should be tolerated:

   A UAC MUST be prepared to receive a response without a tag in the To
   field, in which case the tag is considered to have a value of null.

      This is to maintain backwards compatibility with [RFC 2543](https://www.rfc-editor.org/rfc/rfc2543), which
      did not mandate To tags.

@sauwming
Copy link
Member

@nanangizz
Copy link
Member Author

How about here? https://github.com/pjsip/pjproject/blob/master/pjsip/src/pjsip/sip_dialog.c#L716

Honestly I don't know whether it needs to be changed too or not. SIP dialog seems to be introduced in rfc3261 (searched for 'dialog' in rfc2543 found none), so perhaps we should just leave it as is?

@sauwming
Copy link
Member

What I found in the older RFC https://www.rfc-editor.org/rfc/rfc2543#section-6.37 regarding forking is this:

The "tag" parameter serves as a general mechanism to distinguish
   multiple instances of a user identified by a single SIP URL. As
   proxies can fork requests, the same request can reach multiple
   instances of a user (mobile and home phones, for example). As each
   can respond, there needs to be a means to distinguish the responses
   from each at the caller. The situation also arises with multicast
   requests. The tag in the To header field serves to distinguish
   responses at the UAC. It MUST be placed in the To field of the
   response by each instance when there is a possibility that the
   request was forked at an intermediate proxy. The "tag" MUST be added
   by UAS, registrars and redirect servers, but MUST NOT be inserted
   into responses forwarded upstream by proxies. The "tag" is added for
   all definitive responses for all methods, and MAY be added for
   informational responses from a UAS or redirect server. All subsequent
   transactions between two entities MUST include the "tag" parameter,
   as described in [Section 11](https://www.rfc-editor.org/rfc/rfc2543#section-11).

So, yes, the To tag must be present in the case of forking. But I'm just not sure why we should assert instead of just returning error.

@nanangizz
Copy link
Member Author

So, yes, the To tag must be present in the case of forking.

   A UAC MUST be prepared to receive a response without a tag in the To
   field, in which case the tag is considered to have a value of null.

It looks like no To tag is basically acceptable, i.e: equal to having tag with null value (whatever it means), I believe it is applicable for forking scenario too.

But then, the API is part of SIP dialog API (using pjsip_dlg prefix), while the concept of dialog does not seem to present in the older RFC, and in the newer RFC, To tag is mandatory. So we can say that the check for To tag in the code is proper.

But I'm just not sure why we should assert instead of just returning error.

Yes, the library does not seem to verify the To tag existence before calling it (start here).

A similar check in create_uas_dialog() here, the comment mentions that the application should check/verify it before.
But pjsua_call_on_incoming() does not seem to do so. So it is either the app or the pjsua_dlg_fork() that needs to be updated.

@nanangizz
Copy link
Member Author

For now, I guess let's just leave the other issues (normal check instead of assertion).

@nanangizz nanangizz merged commit b0a90b1 into master Feb 23, 2023
@nanangizz nanangizz deleted the issue-1096 branch February 23, 2023 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion in sending UPDATE after receiving 200/INVITE response without to-tag
3 participants