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

dialog: fix rtags of forking INVITE with 100rel #947

Merged
merged 1 commit into from
Sep 17, 2023

Conversation

maximilianfridrich
Copy link
Contributor

src/sip/dialog.c Outdated
renc.end = 0;

err |= sip_msg_hdr_apply(msg, msg->req, SIP_HDR_RECORD_ROUTE,
record_route_handler, &renc) ? ENOMEM : 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats looks a bit strange to me. If record_route_handler returns true, ENOMEM should returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, thanks. I will still clean this up a bit, hence the draft status. I wanted to make use of the GitHub workflows to make sure the test I wrote also works on other platforms.

test/sipsess.c Outdated Show resolved Hide resolved
test/sipsess.c Outdated Show resolved Hide resolved
test/sipsess.c Outdated Show resolved Hide resolved
@maximilianfridrich
Copy link
Contributor Author

I had to remove the new test for retest as I was not able to write a test that would properly simulate a forked INVITE. Instead, I tested it with a SIPP scenario and the PRACKs now have the correct tags.

Other than that the behavior of re stays the same regarding forked INVITES. I.e. only the first 2XX response is ACKed, subsequent ones are ignored.

@maximilianfridrich maximilianfridrich marked this pull request as ready for review September 14, 2023 07:12
@alfredh alfredh merged commit fc588b9 into baresip:main Sep 17, 2023
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants