SMS-Deliver decoding fails with Alphanumeric TP-OA

[kharry5]

The unmarshal code appears to miscalculate the length of the address. Will submit a PR

[warthog618]

Your report is too light on detail for me to accept the change.

The address encoding/decoding follows the standard 3GPP TS 23.040 version 9.3.0 Release 9 Section 9.1.2.5.

Specifically as per this abridged extract:

The Type-of-Address field format is as follows:
Type-of-number:
Bits 6 5 4
1 0 1 Alphanumeric, (coded according to 3GPP TS 23.038 [9] GSM 7-bit default alphabet)

I take that to mean that the address is 7bit encoded. That applies to the whole field - including the length field, and so the length in the address is the number of septets. Your change uses the same length as semi-octet encoding, which the other ToNs use, but that is not Alphanumeric encoding as defined in the spec.

Can you provide a reference that shows that the length should be interpreted as per your change?
Otherwise I have to conclude you are dealing with a non-compliant encoder.
(I also note you haven't changed MarshalBinary which still encodes as per the spec. If there is a bug then you need to change both.)

[kharry5]

You are in the correct place in the standard, but you need to read a bit further. The Address-Length field does not change meaning based on the Type-of-number, it is always represented the same way. "The Address-Length field is an integer representation of the number of useful semi-octets within the Address-Value
field, i.e. excludes any semi octet containing only fill bits. "

Here is a TP-OA from a mt-forwardsm message captured off an actual mobile network.
0ed0d637396c7ebbcb
and a screenshot of wireshark correctly decoding it.

The MarshalBinary is probably also incorrect. I didn't look into that because I'm only using the decoding portions of the library. Do you want a different PR for that?

[warthog618]

Nuts - the section you reference is actually a bit earlier, not a bit further.

The encode and decode should both be fixed in the one PR - they are the two sides of the one problem.

Your decode fix is incomplete due to the "excludes any semi octet containing only fill bits" clause - it doesn't allow for that and will drop a semi-octet when l is odd.

And the tests need to be extended to cover the semi-octet of fill bits case - in both directions.

I'll take a look at it shortly.

[warthog618]

I've committed a fix in 690ae30.
See if that works for you.

[kharry5]

Seems fine on the decoding side. Could you tag a new patch release please.

[warthog618]

Fix in v0.1.2.