opentelecoms-org / jsmpp

SMPP implemented in Java
Apache License 2.0
232 stars 164 forks source link

NumberingPlanIndicator does not handle Reserved values #134

Open killdashnine opened 4 years ago

killdashnine commented 4 years ago

I got some strange OA sending me NPI=5. According to some source this is maritme, but in official GSM 3.40 spec it is not shown as that, nor in SMPP 3.4 specs.

image

As NumberingPlanIndicator is a mandatory value for submit_sm and deliver_sm in the JSMPP API, I was thinking that either NumberingPlanIndicator should not be an enum, or some funky hacking like following would take place:

` public enum NumberingPlanIndicator { UNKNOWN(SMPPConstant.NPI_UNKNOWN), ISDN(SMPPConstant.NPI_ISDN), DATA(SMPPConstant.NPI_DATA), TELEX(SMPPConstant.NPI_TELEX), LAND_MOBILE(SMPPConstant.NPI_LAND_MOBILE), NATIONAL(SMPPConstant.NPI_NATIONAL), PRIVATE(SMPPConstant.NPI_PRIVATE), ERMES(SMPPConstant.NPI_ERMES), INTERNET(SMPPConstant.NPI_INTERNET), WAP(SMPPConstant.NPI_WAP), RESERVED((byte)0xff);

private byte value;
private NumberingPlanIndicator(byte value) {
    this.value = value;
}
private NumberingPlanIndicator withReservedValue(byte value) {
    this.value = value;
    return this;
}

/**
 * Return the value of NPI.
 * @return the value of NPI.
 */
public byte value() {
    return value;
}

/**
 * Get the associated <tt>NumberingPlanIndicator</tt> by it's value.
 *
 * @param value is the value.
 * @return the associated enum const for specified value.
 * @throws IllegalArgumentException if there is no enum const associated
 *      with specified <tt>value</tt>.
 */
public static NumberingPlanIndicator valueOf(byte value)
        throws IllegalArgumentException {
    for (NumberingPlanIndicator val : values()) {
        if (val.value == value)
            return val;
    }

    return RESERVED.withReservedValue(value);
}

} `

So I added RESERVED enum option here and withReservedValue instead of throwing an exception returning a RESERVED with the actual value. Note that this is very hacky as I needed a quick solution to pass the NPI=5 traffic.

I myself think that making NumberingPlanIndicator a class would be the better option.

I'd be happy to send a pull request if we could agree on a way forward. For now I am keeping this hack in place.

killdashnine commented 4 years ago

Strangely wireshark decodes NPI 5 as service centre specific:

image

Found this is 3GPP 23.040

image

So maybe the constants en enum needs to be updated in JSMPP.

darkkiss1984 commented 4 years ago

In SS7 , SCCP layer, NPI=5 is indeed maritime mobile.

https://en.wikipedia.org/wiki/Telephone_numbering_plan#Numbering_plan_indicator

image

killdashnine commented 4 years ago

Yes saw that, but I couldn't find it anywhere in official specs. Anyhow the problem is that JSMPP is not able currently to pass other values as NPI.

darkkiss1984 commented 4 years ago

That's true. I guess I just wanted to point out, that problems could still appear, when dealing with reserved values and so many different SMSC implementations. After all, the provider SMSC is responsible to translate your SMPP submit_sm into an mT-ForwardSM and deliver it across SS7. And the other way around , the mO-ForwardSM into an SMPP deliver_sm.

But that's not JSMPP's problem in any way... totally agree with you.

pmoerenhout commented 4 years ago

Making it a class would make it more flexible. Together with SMPP5 additions, I will look into this!

killdashnine commented 4 years ago

Ok, if you need help with pull requests, let me know.