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.

Sign up for GitHub

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

Jump to bottom

destaddr overrides remote_port #8

Closed
vodik opened this issue Mar 7, 2016 · 3 comments
Closed

destaddr overrides remote_port #8

vodik opened this issue Mar 7, 2016 · 3 comments

Comments

@vodik
Copy link
Member

vodik commented Mar 7, 2016

If _clientdefaults contains a destaddr, and an agent's _values contains a remote_port, remote_port will get ignored and the destaddr field will be incorrectly used.
@goodboy
Copy link
Member

goodboy commented Mar 8, 2016

Seems this was introduced by 5833d47 which, funny enough, was trying to solve the opposite problem.

This shows that composite attributes (like socket addresses) really can't be treated the same as the lower level arg attrs. It might make sense to add a FieldComposite descriptor to commands.py which contains logic to avoid overwriting contained arg attrs unless they are unset in the agent's instance namespace.

I think I do have a quick hack for now though.

@goodboy goodboy closed this as completed in d0eaff2 Mar 8, 2016
@goodboy goodboy mentioned this issue Mar 8, 2016
Open
@goodboy
Copy link
Member

goodboy commented Mar 9, 2016

The hack broke everything else yay....

@goodboy goodboy reopened this Mar 9, 2016
@goodboy
Copy link
Member

goodboy commented Mar 11, 2016

Finally fixed by e0688a9.

@goodboy goodboy closed this as completed Mar 11, 2016
vodik pushed a commit to vodik/pysipp that referenced this issue Jun 23, 2016
@vodik
Prevent composite attrs from overriding arg attrs
1d35c93 
Composite agent attributes should never override the variables they
contain unless the underlying value(s) have not yet been set.

Fixes SIPp#8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@vodik @goodboy