1028032 - [B2G][CBS] Remove Padding Charactor <CR> Correctly from Decoded CBS Message

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

According to |9.3.19 CBS-Message-Information-Page| in TS 23.041 [1]:
"
 This parameter is of a fixed length of 82 octets and carries up to and
 including 82 octets of user information. Where the user information is
 less than 82 octets, the remaining octets must be filled with padding.
 (see 3GPP TS 23.038)
"

According to |6.1.2.2.1 Packing of 7-bit characters| in TS 23.038 [2]:
Octet 1~ Octet 82 are fully encoding in 7bit GSM alphabet except the last 5 bits.

According to |6.2.1.1 GSM 7 bit Default Alphabet| and |6.2.3 UCS2| in TS 23.038 [2], the the padding character for CBS is <CR>.

What happens now is that we decode these 82 octets in either GSM 7bit alphabet or UCS accordingly without trying to remove the padding <CR> if available.

Hence, file this bug to address this problem.

[1] http://www.etsi.org/deliver/etsi_ts/123000_123099/123041/11.06.00_60/ts_123041v110600p.pdf
[2] http://www.etsi.org/deliver/etsi_ts/123000_123099/123038/11.00.00_60/ts_123038v110000p.pdf

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part1 v1: Remove Padding Charactor <CR> Correctly from Decoded CBS Message.

Verified with both GSM 7bit/UCS 2 CBS PDUs:
7Bit: cbs pdu C0020001011154741914AFA7C76B9058FEBEBB41E6371EA4AEB7E173D0DB5E9683E8E832881DD6E741E4F7B9D168341A8D46A3D168341A8D46A3D168341A8D46A3D168341A8D46A3D168341A8D46A3D168341A8D46A3D100
UCS2: cbs pdu C0020001481100540068006500200071007500690063006b002000620072006f0077006e00200066006f00780020006a0075006d007000730020006f00760065007200200074006800650020006c0061000D000D000D000D

Corresponding test case is needed before landing this patch.

Comment 2

[sam.hua]

Hi,
the wrong msg.body is:
{"fullBody":"NODSAMSUNGINDIA\r\r\r£@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"}

I am afraid that it couldn't be shortened to ""NODSAMSUNGINDIA".

Thanks!

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Sam,

The only padding character to be recognized shall be <CR> no matter it's GSM 7bit or UCS2 encoding according to what have mentioned in comment 0.

If you use bit 0 as padding, it is normal to have '@' decoded in GSM 7bit alphabet which is wrong according to the 3GPP standard mentioned in comment 0.

Comment 4

[sam.hua]

Okay.

How handle the case if the length of CBS is shorter than 88 from network? 
AT< +CBM: 22
AT< 321000320111CE27711A6C4EABCE63D2494C061B8D46
we will get "@" if we fill 0.

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

It's abnormal to have CBS PDU with length to 22 in GSM network according to 9.4.1.2 Message Parameter
(GSM CBS) and 9.4.1.3.2 Message Parameter (ETWS) in TS 23.041, where length of GSM CBS is 88 and length
of ETWS is 56.
Hence, there seems something wrong from the modem or network instead, 
because length 22 is neither a PDU for ETWS nor a PDU for GSM CBS.

If you still have to workaround it from device side as GSM CBS PDU (88 octets):
1. The content of the message always started at octet#7. (where octet#1 is the 1st octet).
   Hence, the content of message in the PDU in comment 4 shall be "CE27711A6C4EABCE63D2494C061B8D46".
2. Try to fill <CR> until octet#88.
   - For GSM 7bit alphabet, please refer to 6.1.2.2.1 Packing of 7-bit characters in TS 23.038, 
     to see how a GSM 7bit character is packed into the PDU.
     In GSM 7bit alphabet, <CR> is equal to "000 1101" in binary.
     a. Calculate the last unused bits (Most Significant Bit) of the content message:
        In this case "CE27711A6C4EABCE63D2494C061B8D46".length = 16, 
        the unused bit shall be (16 * 8 ) % 7 = 2.
     b. Which means we have to start to fill "000 1101" from the 2 MSBs of the last octet "46".
        (That is "01xx xxxx" in last octet and "xxx0 0011" in next new padding octet and so on.)
     c. Always remember to set 5 unused MSBs in octet#88 to '0' according to 6.1.2.2.1.
   - For UCS 2, it more easier:
     a. Ensure the length of content is even. Otherwise, there is something wrong.
     b. Just fill <CR> == "000D" as padding bytes.

Comment 6

[Joe Cheng [:jcheng] (please needinfo)]

unless this impacts tarako target market (does the tarako target market requires CB?)
let's move to 1.4? see if this is needed for 1.4
Thanks

Comment 7

[Preeti Raghunath(:Preeti)]

Moving this to backlog. Please re-nom if issue blocks shipment of Dolphin devices.

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part1 v2: Remove Padding Charactor <CR> Correctly from Decoded CBS Message.

Hi Hsinyi,

This is to remove the padding characters from CB message correctly.

May I have your review for this change? Thanks!

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part1 v2: Remove Padding Charactor <CR> Correctly from Decoded CBS Message.

Hi Hsinyi,

This is to remove the padding characters from CB message correctly.

May I have your review for this change?

Thanks!

Comment 10

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Part 2 v1: Add Test cases to verify padding <CR> in both GSM 7bit and UCS2 encoded PDU. r=htsai

Add corresponding test to verify if we clear the padding <CR> correctly for both GSM 7bit and UCS2 encoded PDU.

Comment 11

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8450136 [details] [diff] [review]
Patch Part1 v2: Remove Padding Charactor <CR> Correctly from Decoded CBS Message.

Review of attachment 8450136 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thank you.

::: dom/system/gonk/ril_worker.js
@@ +8534,5 @@
> +    //  less than 82 octets, the remaining octets must be filled with padding.
> +    // "
> +    // According to 6.2.1.1 GSM 7 bit Default Alphabet and 6.2.3 UCS2 in
> +    // TS 23.038, the padding character is <CR>.
> +    //

nit: remove the line

@@ +8535,5 @@
> +    // "
> +    // According to 6.2.1.1 GSM 7 bit Default Alphabet and 6.2.3 UCS2 in
> +    // TS 23.038, the padding character is <CR>.
> +    //
> +    if (msg.body) {

nit: bail-out earlier

if (!msg.body) {
  return;
}

Comment 12

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #11)
> Comment on attachment 8450136 [details] [diff] [review]
> Patch Part1 v2: Remove Padding Charactor <CR> Correctly from Decoded CBS
> Message.
> 
> Review of attachment 8450136 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, thank you.
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +8534,5 @@
> > +    //  less than 82 octets, the remaining octets must be filled with padding.
> > +    // "
> > +    // According to 6.2.1.1 GSM 7 bit Default Alphabet and 6.2.3 UCS2 in
> > +    // TS 23.038, the padding character is <CR>.
> > +    //
> 
> nit: remove the line
> 
> @@ +8535,5 @@
> > +    // "
> > +    // According to 6.2.1.1 GSM 7 bit Default Alphabet and 6.2.3 UCS2 in
> > +    // TS 23.038, the padding character is <CR>.
> > +    //
> > +    if (msg.body) {
> 
> nit: bail-out earlier
> 
> if (!msg.body) {
>   return;
> }

Thanks for the review. Nits will be addressed in next update. :)

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: atch Part1 v3: Remove Padding Charactor <CR> Correctly from Decoded CBS Message. r=htsai

Address nits in comment 11.

Comment 14

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8450138 [details] [diff] [review]
Part 2 v1: Add Test cases to verify padding <CR> in both GSM 7bit and UCS2 encoded PDU. r=htsai

r=htsai

Comment 15

[Bevis Tseng[:bevis][:btseng](Exited)]

try server is green:
https://tbpl.mozilla.org/?tree=Try&rev=a44e74be94c4

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/8f99bd9fc165
https://hg.mozilla.org/integration/b2g-inbound/rev/467e43ffefef

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8f99bd9fc165
https://hg.mozilla.org/mozilla-central/rev/467e43ffefef