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

Various tweaks #14

Merged
merged 6 commits into from
Sep 4, 2013
Merged

Various tweaks #14

merged 6 commits into from
Sep 4, 2013

Conversation

sfgeorge
Copy link
Contributor

@sfgeorge sfgeorge commented Sep 3, 2013

  • Little fixes to From, Contact, and Route headers
  • Ensure BYEs work through proxies

@@ -167,15 +167,16 @@ def send_digits(digits, delay = 0.250)
def send_bye(opts = {})
msg = <<-MSG

BYE sip:[service]@[remote_ip]:[remote_port] SIP/2.0
BYE [next_url] SIP/2.0
Copy link
Member

Choose a reason for hiding this comment

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

Is this change compatible with what was there before? The SIPp docs say:

If the "rrs" attribute in a recv command is set to "true", then the [next_url] contains the contents of the Contact header (i.e within the '<' and '>' of Contact)

If rrs is not set to true, what will [next_url] expand to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm relying on the rrs in the initial #receive_answer to be true. Without that, I believe that [next_url] would yield an empty string and break the entire exchange.

Copy link
Member

Choose a reason for hiding this comment

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

When you say the rrs in #receive_answer, are you referring to this: https://github.com/bklang/sippy_cup/pull/15/files#L1R111 Right now it appears to be hard coded, but your other PR might make that no longer the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, exactly. I did happen to make changes to it in #15. But there rrs still defaults to true if not explicitly overridden. I believe that [next_url] in BYE would only break if someone explicitly set rrs to false for some reason.

@bklang
Copy link
Member

bklang commented Sep 4, 2013

Thanks @sfgeorge! Looks good, merging.

bklang added a commit that referenced this pull request Sep 4, 2013
@bklang bklang merged commit 17fbc65 into mojolingo:master Sep 4, 2013
@sfgeorge sfgeorge deleted the feature/various-tweaks branch September 5, 2013 06:21
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