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

Implement Via headers for requests for outbound calls #218

Merged
merged 10 commits into from
Nov 18, 2024

Conversation

biglittlebigben
Copy link
Contributor

For outbound calls, when making requests (BYE/REFER), the Via header should have the IP address/port of the instance sending the request.

We use the private IP because the Transaction logic in sipgo uses this information to bind the socket when making establishing the connection. This should work for UDP, but this will still fail for TCP.

@biglittlebigben biglittlebigben requested a review from a team as a code owner October 31, 2024 23:57
Comment on lines +1148 to +1150
// Remove all Via headers
for req.RemoveHeader("Via") {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, this might be incorrect. Each router is supposed to append it's own Via and then remove that line when requests go back. So I believe we are supposed to add own own line and that's it.

Copy link
Contributor Author

@biglittlebigben biglittlebigben Nov 18, 2024

Choose a reason for hiding this comment

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

The behavior you state above is correct for responses within a given transaction: the responses must have the same VIA headers as the request, and proxies on the way back are supposed to "peel" their own Via entries from the response,. The REFER request is however a new request, in a new transaction, sent directly to the server from the INVITE Contact header. As such, only the IP address of the request initiator should be in the Via header. I believe the examples in the REFER RFC confirm the above: https://www.rfc-editor.org/rfc/rfc3515#section-4

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, you are right, I forgot that's a new request. Makes sense 👍

@biglittlebigben biglittlebigben merged commit f7fecf7 into main Nov 18, 2024
3 checks passed
@biglittlebigben biglittlebigben deleted the benjamin/via_header branch November 18, 2024 18:15
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.

2 participants