2:09 a.m.
There are problems with some real-life scenarios where SMS-DELIVER originator
address is an alphanumeric string with 11 chars, some extended ASCII (like äöü).
Ofono assumes Short Message's TP-OA max length is 11 octets (ref [1] and [2])
and rejects messages whose _decoded_ alphanumeric address takes more
than 11 bytes.
However, [3] defines Alphanumeric address is 7-bit coded referring to [4].
7-bit alphabet coding allows packing up to twelve 8-bit characters into 11
octets. The limit applies to octets, not to decoded characters or bytes.
Here is a fix proposals for your consideration,
with these a phone utilizing ofono will accept such short messages.
FYI a commercial Android phone reference handles the same messages fine.
Note: I've been able to test only using max 11-character TP-OA, but referred
specifications suggest a 12th char would fit into the 11 octets as well.
REFERENCES:
[1] 24.011 8.2.5.1 Originator address element
[2] 23040 9.2.2.1 SMS‑DELIVER type
[3] 23040 9.1.2.5 Address fields
[4] 3GPP TS 23.038 GSM 7‑bit default alphabet
[5] Problem 1 callstack:
ofono_sms_deliver_notify
src.sms:handle_deliver
src.sms:sms_dispatch
src/sms.c:compute_incoming_msgid
src/smsutil.c:sms_encode
src/smsutil.c:encode_deliver
src/smsutil.c:sms_encode_address_field
if (len > 11)
[6] Problem 2 callstack:
ofono_sms_deliver_notify
src/smsutil.c:sms_decode
src/smsutil.c:decode_deliver
src/smsutil.c:sms_decode_address_field
if (strlen(utf8) > 20
Tommi Kenakkala (2):
sms: SMS-DELIVER TP-OA Alphanum decoding and encoding fix
unit: SMS-DELIVER decoding/encoding test for long alphanum address
src/smsutil.c | 12 +++++++++---
src/smsutil.h | 6 +++++-
unit/test-sms.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 62 insertions(+), 8 deletions(-)
--
1.7.9.5
2:09 a.m.
New subject: [PATCH 1/2] sms: SMS-DELIVER TP-OA Alphanum decoding and encoding fix
Ofono ignores receved SMS if they have a long alphanum address with
extended ascii characters:
1) sms_encode_address_field: TP-OA max length comparison before encoding
is incorrect because TP-OA's 11 octets are 7-bit coded and thus transport
twelve 8-bit chars. Change ofono to compare character count to character
limit instead of octet limit.
2) sms_decode_address_field: Decoding from 7-bit alphabet will output max.
twelve 8-bit chars which converted to UTF-8 take 24 bytes.
Increase oFono sms_address struct's array accordingly.
---
src/smsutil.c | 12 +++++++++---
src/smsutil.h | 6 +++++-
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/smsutil.c b/src/smsutil.c
index be60ee9..b2ff349 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -524,7 +524,8 @@ static gboolean encode_validity_period(const struct
sms_validity_period *vp,
gboolean sms_encode_address_field(const struct sms_address *in, gboolean sc,
unsigned char *pdu, int *offset)
{
- size_t len = strlen(in->address);
+ const char *addr = (const char *)&in->address;
+ size_t len = strlen(addr);
unsigned char addr_len = 0;
unsigned char p[10];
@@ -546,7 +547,8 @@ gboolean sms_encode_address_field(const struct sms_address *in,
gboolean sc,
unsigned char *gsm;
unsigned char *r;
- if (len > 11)
+ /* TP-OA's 11 octets may fit 12 7-bit packed characters */
+ if (g_utf8_strlen(addr, strlen(addr)) > 12)
return FALSE;
gsm = convert_utf8_to_gsm(in->address, len, NULL, &written, 0);
@@ -675,7 +677,11 @@ gboolean sms_decode_address_field(const unsigned char *pdu, int len,
if (utf8 == NULL)
return FALSE;
- if (strlen(utf8) > 20) {
+ /*
+ * TP-OA's 11 7-bit octets may contain 12 8-bit chars,
+ * which converted to unicode would take 24+1 bytes.
+ */
+ if (strlen(utf8) > 24) {
g_free(utf8);
return FALSE;
}
diff --git a/src/smsutil.h b/src/smsutil.h
index b1001f8..e8266c8 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -220,7 +220,11 @@ enum cbs_geo_scope {
struct sms_address {
enum sms_number_type number_type;
enum sms_numbering_plan numbering_plan;
- char address[21]; /* Max 20 in semi-octet, 11 in alnum */
+ /*
+ * An alphanum TP-OA is 11 7-bit octets. Those can contain max 12 8-bit
+ * characters taking 24 bytes when converted to UTF-8.
+ */
+ char address[25];
};
struct sms_scts {
--
1.7.9.5
5:59 a.m.
New subject: [PATCH 1/2] sms: SMS-DELIVER TP-OA Alphanum decoding and encoding fix
Hi Tommi,
On 02/12/2015 09:09 AM, Tommi Kenakkala wrote:
Ofono ignores receved SMS if they have a long alphanum address with
extended ascii characters:
1) sms_encode_address_field: TP-OA max length comparison before encoding
is incorrect because TP-OA's 11 octets are 7-bit coded and thus transport
twelve 8-bit chars. Change ofono to compare character count to character
limit instead of octet limit.
How did you arrive at 11 octet figure? TP-OA is limited to 12 bytes. So
12 - TON - LEN = 10 bytes payload.
10 bytes with 7-bit packed characters -> 11 unpacked characters
All characters in the GSM default alphabet can be described by a maximum
of 2 bytes in utf8. Exception is the euro sign, which is an extended
character, but requires 3 utf8 bytes to encode. So a buffer of 11 * 2 +
1 bytes should be sufficient to hold an alphanumeric address.
2) sms_decode_address_field: Decoding from 7-bit alphabet will output
max.
twelve 8-bit chars which converted to UTF-8 take 24 bytes.
Increase oFono sms_address struct's array accordingly.
---
src/smsutil.c | 12 +++++++++---
src/smsutil.h | 6 +++++-
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/smsutil.c b/src/smsutil.c
index be60ee9..b2ff349 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -524,7 +524,8 @@ static gboolean encode_validity_period(const struct
sms_validity_period *vp,
gboolean sms_encode_address_field(const struct sms_address *in, gboolean sc,
unsigned char *pdu, int *offset)
{
- size_t len = strlen(in->address);
+ const char *addr = (const char *)&in->address;
+ size_t len = strlen(addr);
unsigned char addr_len = 0;
unsigned char p[10];
@@ -546,7 +547,8 @@ gboolean sms_encode_address_field(const struct sms_address *in,
gboolean sc,
unsigned char *gsm;
unsigned char *r;
- if (len > 11)
+ /* TP-OA's 11 octets may fit 12 7-bit packed characters */
+ if (g_utf8_strlen(addr, strlen(addr)) > 12)
g_utf8_strlen() > 11 here
return FALSE;
gsm = convert_utf8_to_gsm(in->address, len, NULL, &written, 0);
@@ -675,7 +677,11 @@ gboolean sms_decode_address_field(const unsigned char *pdu, int
len,
if (utf8 == NULL)
return FALSE;
- if (strlen(utf8) > 20) {
+ /*
+ * TP-OA's 11 7-bit octets may contain 12 8-bit chars,
+ * which converted to unicode would take 24+1 bytes.
+ */
+ if (strlen(utf8) > 24) {
and strlen(utf8) > 22 here...
g_free(utf8);
return FALSE;
}
diff --git a/src/smsutil.h b/src/smsutil.h
index b1001f8..e8266c8 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -220,7 +220,11 @@ enum cbs_geo_scope {
struct sms_address {
enum sms_number_type number_type;
enum sms_numbering_plan numbering_plan;
- char address[21]; /* Max 20 in semi-octet, 11 in alnum */
+ /*
+ * An alphanum TP-OA is 11 7-bit octets. Those can contain max 12 8-bit
+ * characters taking 24 bytes when converted to UTF-8.
+ */
+ char address[25];
And address should be 23, the comment is wrong
};
struct sms_scts {
Regards,
-Denis
2:10 a.m.
New subject: [PATCH 2/2] unit: SMS-DELIVER decoding/encoding test for long alphanum address
---
unit/test-sms.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 48 insertions(+), 4 deletions(-)
diff --git a/unit/test-sms.c b/unit/test-sms.c
index 7b644df..5b4a37f 100644
--- a/unit/test-sms.c
+++ b/unit/test-sms.c
@@ -38,6 +38,12 @@ static const char *simple_deliver = "07911326040000F0"
"040B911346610089F60000208062917314480CC8F71D14969741F977FD07";
static const char *alnum_sender = "0791447758100650"
"040DD0F334FC1CA6970100008080312170224008D4F29CDE0EA7D9";
+static const char *unicode_deliver = "04819999990414D0FBFD7EBFDFEFF77BFE1E001"
+ "9512090801361807E00DC00FC00C400E400D600F600C500E500D800F800C"
+ "600E600C700E700C900E900CA00EA00DF003100320033003400350036003"
+ "7003800390030002000540068006900730020006D0065007300730061006"
+ "7006500200069007300200036003300200075006E00690063006F0064006"
+ "5002000630068006100720073002E";
static const char *simple_submit = "0011000B916407281553F80000AA"
"0AE8329BFD4697D9EC37";
@@ -298,13 +304,16 @@ static void test_deliver_encode(void)
int encoded_pdu_len;
int encoded_tpdu_len;
char *encoded_pdu;
+ unsigned int smsc_len;
+ /* test simple_deliver */
decoded_pdu = decode_hex(simple_deliver, -1, &pdu_len, 0);
g_assert(decoded_pdu);
g_assert(pdu_len == (long)strlen(simple_deliver) / 2);
- ret = sms_decode(decoded_pdu, pdu_len, FALSE, 30, &sms);
+ smsc_len = decoded_pdu[0] + 1;
+ ret = sms_decode(decoded_pdu, pdu_len, FALSE, pdu_len - smsc_len, &sms);
g_free(decoded_pdu);
@@ -322,7 +331,7 @@ static void test_deliver_encode(void)
}
g_assert(ret);
- g_assert(encoded_tpdu_len == 30);
+ g_assert(encoded_tpdu_len == pdu_len - smsc_len);
g_assert(encoded_pdu_len == pdu_len);
encoded_pdu = encode_hex(pdu, encoded_pdu_len, 0);
@@ -331,12 +340,14 @@ static void test_deliver_encode(void)
g_free(encoded_pdu);
+ /* test alnum_sender */
decoded_pdu = decode_hex(alnum_sender, -1, &pdu_len, 0);
g_assert(decoded_pdu);
g_assert(pdu_len == (long)strlen(alnum_sender) / 2);
- ret = sms_decode(decoded_pdu, pdu_len, FALSE, 27, &sms);
+ smsc_len = decoded_pdu[0] + 1;
+ ret = sms_decode(decoded_pdu, pdu_len, FALSE, pdu_len - smsc_len, &sms);
g_free(decoded_pdu);
@@ -354,7 +365,7 @@ static void test_deliver_encode(void)
}
g_assert(ret);
- g_assert(encoded_tpdu_len == 27);
+ g_assert(encoded_tpdu_len == pdu_len - smsc_len);
g_assert(encoded_pdu_len == pdu_len);
encoded_pdu = encode_hex(pdu, encoded_pdu_len, 0);
@@ -362,6 +373,39 @@ static void test_deliver_encode(void)
g_assert(strcmp(alnum_sender, encoded_pdu) == 0);
g_free(encoded_pdu);
+
+ /* test unicode_deliver*/
+ decoded_pdu = decode_hex(unicode_deliver, -1, &pdu_len, 0);
+ g_assert(decoded_pdu);
+ g_assert(pdu_len == (long)strlen(unicode_deliver) / 2);
+
+ smsc_len = decoded_pdu[0] + 1;
+ ret = sms_decode(decoded_pdu, pdu_len, FALSE, pdu_len - smsc_len, &sms);
+
+ g_free(decoded_pdu);
+
+ g_assert(ret);
+ g_assert(sms.type == SMS_TYPE_DELIVER);
+
+ ret = sms_encode(&sms, &encoded_pdu_len, &encoded_tpdu_len, pdu);
+
+ if (g_test_verbose()) {
+ int i;
+
+ for (i = 0; i < encoded_pdu_len; i++)
+ g_print("%02X", pdu[i]);
+ g_print("\n");
+ }
+
+ g_assert(ret);
+ g_assert(encoded_tpdu_len == pdu_len - smsc_len);
+ g_assert(encoded_pdu_len == pdu_len);
+
+ encoded_pdu = encode_hex(pdu, encoded_pdu_len, 0);
+
+ g_assert(strcmp(unicode_deliver, encoded_pdu) == 0);
+
+ g_free(encoded_pdu);
}
static void test_simple_submit(void)
--
1.7.9.5
6:04 a.m.
New subject: [PATCH 2/2] unit: SMS-DELIVER decoding/encoding test for long alphanum address
Hi Tommi,
On 02/12/2015 09:10 AM, Tommi Kenakkala wrote:
---
unit/test-sms.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 48 insertions(+), 4 deletions(-)
diff --git a/unit/test-sms.c b/unit/test-sms.c
index 7b644df..5b4a37f 100644
--- a/unit/test-sms.c
+++ b/unit/test-sms.c
@@ -38,6 +38,12 @@ static const char *simple_deliver = "07911326040000F0"
"040B911346610089F60000208062917314480CC8F71D14969741F977FD07";
static const char *alnum_sender = "0791447758100650"
"040DD0F334FC1CA6970100008080312170224008D4F29CDE0EA7D9";
+static const char *unicode_deliver =
"04819999990414D0FBFD7EBFDFEFF77BFE1E001"
+ "9512090801361807E00DC00FC00C400E400D600F600C500E500D800F800C"
+ "600E600C700E700C900E900CA00EA00DF003100320033003400350036003"
+ "7003800390030002000540068006900730020006D0065007300730061006"
+ "7006500200069007300200036003300200075006E00690063006F0064006"
+ "5002000630068006100720073002E";
static const char *simple_submit = "0011000B916407281553F80000AA"
"0AE8329BFD4697D9EC37";
@@ -298,13 +304,16 @@ static void test_deliver_encode(void)
int encoded_pdu_len;
int encoded_tpdu_len;
char *encoded_pdu;
+ unsigned int smsc_len;
+ /* test simple_deliver */
decoded_pdu = decode_hex(simple_deliver, -1, &pdu_len, 0);
g_assert(decoded_pdu);
g_assert(pdu_len == (long)strlen(simple_deliver) / 2);
- ret = sms_decode(decoded_pdu, pdu_len, FALSE, 30, &sms);
+ smsc_len = decoded_pdu[0] + 1;
+ ret = sms_decode(decoded_pdu, pdu_len, FALSE, pdu_len - smsc_len, &sms);
This seems extraneous to this patch. Probably belongs in a separate patch.
g_free(decoded_pdu);
@@ -322,7 +331,7 @@ static void test_deliver_encode(void)
}
g_assert(ret);
- g_assert(encoded_tpdu_len == 30);
+ g_assert(encoded_tpdu_len == pdu_len - smsc_len);
as above.
g_assert(encoded_pdu_len == pdu_len);
encoded_pdu = encode_hex(pdu, encoded_pdu_len, 0);
@@ -331,12 +340,14 @@ static void test_deliver_encode(void)
g_free(encoded_pdu);
+ /* test alnum_sender */
decoded_pdu = decode_hex(alnum_sender, -1, &pdu_len, 0);
g_assert(decoded_pdu);
g_assert(pdu_len == (long)strlen(alnum_sender) / 2);
- ret = sms_decode(decoded_pdu, pdu_len, FALSE, 27, &sms);
+ smsc_len = decoded_pdu[0] + 1;
+ ret = sms_decode(decoded_pdu, pdu_len, FALSE, pdu_len - smsc_len, &sms);
as above
g_free(decoded_pdu);
@@ -354,7 +365,7 @@ static void test_deliver_encode(void)
}
g_assert(ret);
- g_assert(encoded_tpdu_len == 27);
+ g_assert(encoded_tpdu_len == pdu_len - smsc_len);
as above
g_assert(encoded_pdu_len == pdu_len);
encoded_pdu = encode_hex(pdu, encoded_pdu_len, 0);
@@ -362,6 +373,39 @@ static void test_deliver_encode(void)
g_assert(strcmp(alnum_sender, encoded_pdu) == 0);
g_free(encoded_pdu);
+
+ /* test unicode_deliver*/
+ decoded_pdu = decode_hex(unicode_deliver, -1, &pdu_len, 0);
+ g_assert(decoded_pdu);
+ g_assert(pdu_len == (long)strlen(unicode_deliver) / 2);
+
+ smsc_len = decoded_pdu[0] + 1;
+ ret = sms_decode(decoded_pdu, pdu_len, FALSE, pdu_len - smsc_len, &sms);
+
+ g_free(decoded_pdu);
+
+ g_assert(ret);
+ g_assert(sms.type == SMS_TYPE_DELIVER);
+
+ ret = sms_encode(&sms, &encoded_pdu_len, &encoded_tpdu_len, pdu);
+
+ if (g_test_verbose()) {
+ int i;
+
+ for (i = 0; i < encoded_pdu_len; i++)
+ g_print("%02X", pdu[i]);
+ g_print("\n");
+ }
+
+ g_assert(ret);
+ g_assert(encoded_tpdu_len == pdu_len - smsc_len);
+ g_assert(encoded_pdu_len == pdu_len);
+
+ encoded_pdu = encode_hex(pdu, encoded_pdu_len, 0);
+
+ g_assert(strcmp(unicode_deliver, encoded_pdu) == 0);
+
+ g_free(encoded_pdu);
}
static void test_simple_submit(void)
Regards,
-Denis
2775
days inactive
2775
days old
4 comments
2 participants
participants (2)
-
Denis Kenzior -
Tommi Kenakkala