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

Do not append colon if uri password is NULL #598

Merged
merged 1 commit into from Aug 12, 2015

Conversation

jarrodb
Copy link

@jarrodb jarrodb commented Aug 12, 2015

I agree with @brettnem's assessment of the problem with #593. As @liviuchircu pointed out, this pull requests addresses the problem in do_action.c:1032 where a colon is blindly added if the user exists without consideration for the presence of an actual password... it attempts a memcpy on a NULL pointer no less (albeit with a length of 0).

I think if parse_uri(tmp, len, &uri)<0) finds an actual semi colon after the user and the password IS indeed blank, uri.password.s should be initialized to an empty string "" instead of NULL so that do_action.c:1032 check passes and the current "problematic" behavior will occur. But otherwise, if the password is NULL, the colon should not be appended.

This should satisfy the userinfo requirements as well as fix the issue presented.

Thoughts?

@liviuchircu
Copy link
Member

Although current behavior barely passes RFC requirements (i.e. it's correct), a SIP server should try to be as permissive as possible when receiving, yet strictly adhere to recommended RFC behavior when sending. And appending unnecessary colons to URI usernames doesn't seem to be too recommended.

The fix looks great, since it still propagates existing empty-string passwords, if script writer really needs this. Many thanks, @jarrodb !

liviuchircu added a commit that referenced this pull request Aug 12, 2015
Do not append colon if uri password is NULL
@liviuchircu liviuchircu merged commit a85c14c into OpenSIPS:master Aug 12, 2015
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.

None yet

2 participants