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

Fix hang when decodeAvpSet receives vendorId=0 #87

Merged
merged 1 commit into from Mar 16, 2017

Conversation

Projects
None yet
3 participants
@jochettino
Contributor

jochettino commented Mar 10, 2017

I added a boolean variable to control the presence of vendor-id AVP, even if it is vendorId = 0.

Closes #86

Fix hang when decodeAvpSet receives vendorId=0
I added a boolean variable to control the presence of vendor-id AVP, even if it is vendorId = 0.
@deruelle

This comment has been minimized.

Member

deruelle commented Mar 10, 2017

Thanks @jochettino for the contribution !
Can you please sign the Contributor License Agreement at https://telestax.com/open-source/#Contribute so we can review and accept the contribution ?

@jochettino

This comment has been minimized.

Contributor

jochettino commented Mar 10, 2017

Done!

@deruelle deruelle requested a review from brainslog Mar 12, 2017

@deruelle deruelle added this to the 1.7.0.FINAL milestone Mar 12, 2017

@deruelle

This comment has been minimized.

Member

deruelle commented Mar 12, 2017

Thanks @jochettino. @brainslog can you please review ?

@brainslog

This comment has been minimized.

Collaborator

brainslog commented Mar 16, 2017

Thanks for the nice catch, @jochettino! You are right, a vendor id of zero MUST NOT be used, as per RFC 6733:

A Vendor-ID value of zero (0) corresponds to the IETF-adopted AVP
values, as managed by IANA.  Since the absence of the Vendor-ID
field implies that the AVP in question is not vendor specific,
implementations MUST NOT use the value of zero (0) for the
Vendor-ID field.

Anyway, we should protect against that, so thanks for the fix! Merging.

I will also add a regression test for this fix, feel free to include one next time ;)

@brainslog brainslog merged commit 7e0a90d into RestComm:master Mar 16, 2017

@jochettino jochettino deleted the jochettino:ElementParser.decodeAvpSet-hangs-when-vendor=0 branch May 18, 2017

@jochettino jochettino restored the jochettino:ElementParser.decodeAvpSet-hangs-when-vendor=0 branch May 18, 2017

@jochettino jochettino deleted the jochettino:ElementParser.decodeAvpSet-hangs-when-vendor=0 branch May 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment