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

forward uas CANCEL request header to uac CANCEL request #149

Merged
merged 11 commits into from
Jul 17, 2023

Conversation

xquanluu
Copy link
Collaborator

@xquanluu xquanluu commented Jul 14, 2023

@davehorton
Copy link
Collaborator

I need more of a description of what problem we are trying to solve here. I would like to see an issue created with full details that this PR links to before I will accept the changes

lib/srf.js Outdated
debug('createB2BUA: received CANCEL from A party, sending CANCEL to B');
res.send(487) ;
uacReq.cancel() ;
uacReq.cancel({
headers: copyUASHeaderToUAC(cancelReq)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if there is no cancelReq? This doesnt seem backwards compatible

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if there is no cancelReq, function copyUASHeaderToUAC return empty object {}, nothing will be added for UAC cancel. so it's still backwards compatible

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My concern is this PR use opts. proxyRequestHeaders for both invite and cancel requests. Not sure this is a good approach.

@davehorton davehorton merged commit 5f9af17 into main Jul 17, 2023
2 checks passed
@davehorton davehorton deleted the fix/b2bua-include-header-in-sub-req branch July 17, 2023 12:33
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