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

Wrong length check in UserDataHeaderImpl.java #275

Closed
smarek opened this issue Dec 28, 2017 · 6 comments
Closed

Wrong length check in UserDataHeaderImpl.java #275

smarek opened this issue Dec 28, 2017 · 6 comments
Labels
Milestone

Comments

@smarek
Copy link

smarek commented Dec 28, 2017

Check in getApplicationPortAddressing16BitAddress() method of UserDataHeaderImpl is wrong, 16bit application port by docs ETSI TS 100 901 V7.5.0 (2001-12) section 9.2.3.24.4 is 4 octets (2 for destination and 2 for originator port)

In parser implementation this check is actually implemented correctly (see ApplicationPortAddressing16BitAddressImpl.java#L53 ), and these kind of checks might be eliminated from UserDataHeaderImpl.java if correct input to parser is checked on field-level.

Below proposed trivial fix diff against current master at 91be33c

diff --git a/map/map-impl/src/main/java/org/mobicents/protocols/ss7/map/smstpdu/UserDataHeaderImpl.java b/map/map-impl/src/main/java/org/mobicents/protocols/ss7/map/smstpdu/UserDataHeaderImpl.java
index aa750c8f3..f49677366 100644
--- a/map/map-impl/src/main/java/org/mobicents/protocols/ss7/map/smstpdu/UserDataHeaderImpl.java
+++ b/map/map-impl/src/main/java/org/mobicents/protocols/ss7/map/smstpdu/UserDataHeaderImpl.java
@@ -143,7 +143,7 @@ public class UserDataHeaderImpl implements UserDataHeader {
     @Override
     public ApplicationPortAddressing16BitAddress getApplicationPortAddressing16BitAddress() {
         byte[] buf = this.data.get(_InformationElementIdentifier_ApplicationPortAddressingScheme16BitAddress);
-        if (buf != null && buf.length == 1)
+        if (buf != null && buf.length == 4)
             return new ApplicationPortAddressing16BitAddressImpl(buf);
         else
             return null;
@vetss
Copy link
Contributor

vetss commented Dec 29, 2017

Hello @smarek

thanks for your feedback. Your note is correct.

May I ask you to read THE TELESTAX OPEN SOURCE PLAYBOOK:
https://telestax.com/open-source/#Contribute
and sign Contributor License Agreement

@vetss vetss added this to the 8.0.0 milestone Dec 29, 2017
@vetss vetss added the bug label Dec 29, 2017
@smarek
Copy link
Author

smarek commented Dec 29, 2017

I signed CLA, not with github, but with corporate e-mail account, but https://github.com/smarek is listed as my profile, so you should be able to match me against list of signatures.

@vetss
Copy link
Contributor

vetss commented Jan 2, 2018

Thanks @smarek

I have added your fix by this commit 4c14755

Thanks for update.

@vetss vetss closed this as completed Jan 2, 2018
@gsaslis
Copy link
Contributor

gsaslis commented Jan 9, 2018

@smarek thanks for contributing this! 👍

I've already added your name in our Contributors Hall of Fame 👏 . If you would like to open a PR so that we can merge this in properly, we'd also be happy to do that too. Just let me know ;)

@smarek
Copy link
Author

smarek commented Jan 9, 2018

@gsaslis thanks, this one is already fixed by mentioned commit, so i'll directly provide PR next time we find out something to fix. Cheers!

@gsaslis
Copy link
Contributor

gsaslis commented Jan 9, 2018

awesome!

(fwiw - i was just talking about you submitting a brand new PR with the same fix, so we can ensure the commit bears your name in GitHub. Probably just a minor detail now, but thought I'd check anyway ;) )

Looking forward to your future PRs !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants