Closed Bug 1119136 Opened 6 years ago Closed 6 years ago

[Contacts][dolphin][FFOS7715 v2.1] Contacts can't be imported from some special SIM cards to device

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: shiwei.zhang, Assigned: jdai)

References

Details

(Whiteboard: [sprd391958], PTR1_Block)

Attachments

(2 files, 9 obsolete files)

3.08 KB, patch
Details | Diff | Splinter Review
1.62 KB, patch
edgar
: review+
Details | Diff | Splinter Review
*** Build Information
Gaia-Rev        b04a8cb7b2482e0a44e6702b48c42283a00b5b1e
Gecko-Rev       f6a4b637c4f59581ad64e4dfec3689442e98e268
Device-Name     dolphin

*** Description
Contacts can't be imported from some special SIM cards to device.
Stuck at Reading from SIM Card.


*** Steps to Reproduce
1.Insert a SIM card.
2.Contacts -> Import Contacts -> SIM card. 

*** Expected Results
Import contacts successfully

*** Actual Results
Stuck at Reading from SIM Card. 

*** SIM Card & Network Info 
Du (Dubai)

*** Reproduction Frequency: 
100%
Exceptions are found in Gecko LOG
01-07 17:22:24.583   128   447 I Gecko   : RIL Worker: Received 144 bytes.
01-07 17:22:24.583   128   447 I Gecko   : RIL Worker: Already read 0
01-07 17:22:24.583   128   447 I Gecko   : RIL Worker: New incoming parcel of size 140
01-07 17:22:24.583   128   447 I Gecko   : RIL Worker: Parcel (size 140): 0,0,0,0,82,1,0,0,0,0,0,0,144,0,0,0,0,0,0,0,56,0,0,0,54,0,55,0,54,0,68,0,54,0,55,0,55,0,52,0,54,0,55,0,54,0,65,0,54,0,68,0,50,0,67,0,50,0,53,0,54,0,68,0,50,0,67,0,54,0,65,0,50,0,69,0,54,0,68,0,49,0,53,0,56,0,49,0,52,0,54,0,52,0,56,0,54,0,49,0,54,0,52,0,56,0,52,0,54,0,52,0,52,0,52,0,54,0,52,0,52,0,52,0,54,0,49,0,70,0,70,0,48,0,49,0,0,0,0,0
01-07 17:22:24.583   128   447 I Gecko   : RIL Worker: We have at least one complete parcel.
01-07 17:22:24.583   128   447 I Gecko   : RIL Worker: [0] Solicited response for request type 28, token 338, error 0
01-07 17:22:24.583   128   447 I Gecko   : RIL Worker: [0] Handling parcel as REQUEST_SIM_IO
01-07 17:22:24.583   128   447 I Gecko   : RIL Worker: Parcel handling threw Error: invalid length of BCD number/SSC contents - 21
01-07 17:22:24.583   128   447 I Gecko   : ICCPDUHelperObject.prototype.readNumberWithLength@resource://gre/modules/ril_worker.js:10818:1
01-07 17:22:24.583   128   447 I Gecko   : ICCPDUHelperObject.prototype.readAlphaIdDiallingNumber@resource://gre/modules/ril_worker.js:10644:18
01-07 17:22:24.583   128   447 I Gecko   : callback@resource://gre/modules/ril_worker.js:13178:9
01-07 17:22:24.583   128   447 I Gecko   : ICCIOHelperObject.prototype.processICCIOReadRecord@resource://gre/modules/ril_worker.js:12972:7
01-07 17:22:24.583   128   447 I Gecko   : ICC_COMMAND_READ_RECORD@resource://gre/modules/ril_worker.js:12999:3
01-07 17:22:24.583   128   447 I Gecko   : ICCIOHelperObject.prototype.processICCIO@resource://gre/modules/ril_worker.js:12854:5
01-07 17:22:24.583   128   447 I Gecko   : REQUEST_SIM_IO@resource://gre/modules/ril_worker.js:5992:3
01-07 17:22:24.583   128   447 I Gecko   : RilObject.prototype.handleParcel@resource://gre/modules/ril_worker.js:5619:7
01-07 17:22:24.583   128   447 I Gecko   : BufObject.prototype.processParcel@resource://gre/modules/ril_worker.js:156:5
01-07 17:22:24.583   128   447 I Gecko   : require._tmpModules[":resource://gre/modules/workers/worker_buf.js"]/Buf.processIncoming@blob:null/196d96ee-3fb6-4aeb-82ef-65d46268aad7:549:9
01-07 17:22:24.583   128   447 I Gecko   : ContextPool.handleRilMessage@resource://gre/modules/ril_worker.js:16189:5
01-07 17:22:24.583   128   447 I Gecko   : onRIL
Corresponding radio LOG
01-07 17:22:24.553   148   598 D use-Rlog/RLOG-AT: [w] Channel1: AT> AT+CRSM=178,28474,95,4,28,0,"3f007f10"
01-07 17:22:24.573   148   199 D use-Rlog/RLOG-AT: [w] Channel1: AT< +CRSM: 144,0,676D6774676A6D2C256D2C6A2E6D158146486164846444644461FF01
01-07 17:22:24.573   148   199 D use-Rlog/RLOG-AT: [w] Channel1: AT< OK
01-07 17:22:24.573   148   598 D use-Rlog/RLOG-RILC: [w] [0338]< SIM_IO {sw1=0x90,sw2=0x0,676D6774676A6D2C256D2C6A2E6D158146486164846444644461FF01}
Hi Shiwei, could you kindly help to attach full log in bugzilla? Thank you.
Flags: needinfo?(shiwei.zhang)
Attached file Gecko_ril_LOG.zip (obsolete) —
Entire log files attached.

Is there anything unexpected in readNumberWithLength()?
(In reply to Edgar Chen [:edgar][:echen] from comment #3)
> Hi Shiwei, could you kindly help to attach full log in bugzilla? Thank you.

Hi Edgar,
 Thanks for you attention, full logs are attached.
Flags: needinfo?(shiwei.zhang)
Assignee: nobody → jdai
(In reply to Shiwei Zhang from comment #4)
> Created attachment 8545762 [details]
> Gecko_ril_LOG.zip
> 
> Entire log files attached.
> 
> Is there anything unexpected in readNumberWithLength()?

Hi Shiwei,

Yes, there has an unexpected length in readNumberWithLength(). Accoring to TS 131 102 4.4.2.3, EF ADN Length of BCD number/SSC contents maximum value is 11, but in this response parcel length is 21(0x15). This is an illegal contact record in ICC.
Flags: needinfo?(shiwei.zhang)
(In reply to johndai from comment #6)
> (In reply to Shiwei Zhang from comment #4)
> > Created attachment 8545762 [details]
> > Gecko_ril_LOG.zip
> > 
> > Entire log files attached.
> > 
> > Is there anything unexpected in readNumberWithLength()?
> 
> Hi Shiwei,
> 
> Yes, there has an unexpected length in readNumberWithLength(). Accoring to
> TS 131 102 4.4.2.3, EF ADN Length of BCD number/SSC contents maximum value
> is 11, but in this response parcel length is 21(0x15). This is an illegal
> contact record in ICC.

yanyan, please help.
Flags: needinfo?(shiwei.zhang)
(In reply to johndai from comment #6)
> (In reply to Shiwei Zhang from comment #4)
> > Created attachment 8545762 [details]
> > Gecko_ril_LOG.zip
> > 
> > Entire log files attached.
> > 
> > Is there anything unexpected in readNumberWithLength()?
> 
> Hi Shiwei,
> 
> Yes, there has an unexpected length in readNumberWithLength(). Accoring to
> TS 131 102 4.4.2.3, EF ADN Length of BCD number/SSC contents maximum value
> is 11, but in this response parcel length is 21(0x15). This is an illegal
> contact record in ICC.

Hi johndai,

The following methods can reproduce the problem:

first, SIM card has a record, asked to the length of phone number exceeds 20. Here, it may operate through other mobile phone. 

second,the SIM card is inserted into the Firefox 7715 mobile phone, enter the contacts to execute the import contact from SIM card,

After will always stay in the Reading from SIM card... Page
According to TS 151.011 clause 10.5.1 EF_ADN,

a) Length of BCD number/SSC contents maximum value is 11. When an ADN/SSC has extension, it is indicated by the extension1 identifier being unequal to 'FF'. The remainder is stored in the EFEXT1 with the remaining length of the additional data being coded in the appropriate additional record itself (see clause 10.5.10).

EFadn Identifier is '6F3A', EFext1 Identifier is '6F4A'

b) see radio log, gecko only send '6F3A' command, but no send '6F4A' command.

cmd=0xB2,efid=0x6F3A,path=3f007f10

c) compare android 7715

read a icc record, will send '6F3A' and '6F4A' command.

//Android corresponding radio LOG

(cmd=0xB2,efid=0x6F4A,path=3F007F10,3,4,13,(null),pin2=(null),aid=(null))
01-03 23:55:50.580   137  1093 D use-Rlog/RLOG-RIL: [w] onRequest: SIM_IO sState=0
01-03 23:55:50.580   137  1093 D use-Rlog/RLOG-RIL: [w] channel5 state: '0' 
01-03 23:55:50.580   137  1093 D use-Rlog/RLOG-RIL: [w] get Channel ID '5'
01-03 23:55:50.580   137  1093 D use-Rlog/RLOG-AT: [w] Channel5: AT> AT+CRSM=178,28490,3,4,13,0,"3F007F10"
01-03 23:55:50.600   133   207 D use-Rlog/RLOG-AT: [w] Channel2: AT< +CRSM: 144,0,8098CE683CFFFFFFFFFFFFFFFFFF068121436587F9FFFFFFFFFFFFFF
01-03 23:55:50.600   133   207 D use-Rlog/RLOG-AT: [w] Channel2: AT< OK
01-03 23:55:50.600   133  1092 D use-Rlog/RLOG-RILC: [w] [0154]< SIM_IO {sw1=0x90,sw2=0x0,8098CE683CFFFFFFFFFFFFFFFFFF068121436587F9FFFFFFFFFFFFFF}

(cmd=0xB2,efid=0x6F3A,path=3F007F10,2,4,28,(null),pin2=(null),aid=(null))
01-03 23:55:50.600   133  1235 D use-Rlog/RLOG-RIL: [w] onRequest: SIM_IO sState=0
01-03 23:55:50.600   133  1235 D use-Rlog/RLOG-RIL: [w] channel2 state: '0' 
01-03 23:55:50.600   133  1235 D use-Rlog/RLOG-RIL: [w] get Channel ID '2'
01-03 23:55:50.600   133  1235 D use-Rlog/RLOG-AT: [w] Channel2: AT> AT+CRSM=178,28474,2,4,28,0,"3F007F10"
01-03 23:55:50.610   133   207 D use-Rlog/RLOG-AT: [w] Channel2: AT< +CRSM: 144,0,726968FFFFFFFFFFFFFFFFFFFFFF0581638869F6FFFFFFFFFFFFFFFF
01-03 23:55:50.610   133   207 D use-Rlog/RLOG-AT: [w] Channel2: AT< OK
01-03 23:55:50.610   133  1235 D use-Rlog/RLOG-RILC: [w] [0156]< SIM_IO {sw1=0x90,sw2=0x0,726968FFFFFFFFFFFFFFFFFFFFFF0581638869F6FFFFFFFFFFFFFFFF}
(In reply to yanyan from comment #8)
> (In reply to johndai from comment #6)
> > (In reply to Shiwei Zhang from comment #4)
> > > Created attachment 8545762 [details]
> > > Gecko_ril_LOG.zip
> > > 
> > > Entire log files attached.
> > > 
> > > Is there anything unexpected in readNumberWithLength()?
> > 
> > Hi Shiwei,
> > 
> > Yes, there has an unexpected length in readNumberWithLength(). Accoring to
> > TS 131 102 4.4.2.3, EF ADN Length of BCD number/SSC contents maximum value
> > is 11, but in this response parcel length is 21(0x15). This is an illegal
> > contact record in ICC.
> 
> Hi johndai,
> 
> The following methods can reproduce the problem:
> 
> first, SIM card has a record, asked to the length of phone number exceeds
> 20. Here, it may operate through other mobile phone. 
> 
> second,the SIM card is inserted into the Firefox 7715 mobile phone, enter
> the contacts to execute the import contact from SIM card,
> 
> After will always stay in the Reading from SIM card... Page

Hi yanyan, 

Could you kindly provide full log in bugzilla? 
Thank you.
Flags: needinfo?(yanyan.an)
Attached file firefox_7715_Ril_Log.rar (obsolete) —
Flags: needinfo?(yanyan.an)
Attached file android(7715CopyContact)_Ril_log.rar (obsolete) —
(In reply to johndai from comment #10)
> Hi yanyan, 
> 
> Could you kindly provide full log in bugzilla? 
> Thank you.

Hi johndai,

 Thanks for you attention, full logs are attached.
 
 Contain firefox_7715_Ril_Log.rar and android(7715CopyContact)_Ril_log.rar:

 firefox_7715_Ril_Log.rar is firefox 7715 ril log, 

 android(7715CopyContact)_Ril_log.rar is android 7715 ril log, about copy sim contact to phone.
(In reply to yanyan from comment #9)
> According to TS 151.011 clause 10.5.1 EF_ADN,
> 
> a) Length of BCD number/SSC contents maximum value is 11. When an ADN/SSC
> has extension, it is indicated by the extension1 identifier being unequal to
> 'FF'. The remainder is stored in the EFEXT1 with the remaining length of the
> additional data being coded in the appropriate additional record itself (see
> clause 10.5.10).
> 
> EFadn Identifier is '6F3A', EFext1 Identifier is '6F4A'
> 
> b) see radio log, gecko only send '6F3A' command, but no send '6F4A' command.
> 
> cmd=0xB2,efid=0x6F3A,path=3f007f10
> 
> c) compare android 7715
> 
> read a icc record, will send '6F3A' and '6F4A' command.
> 
> //Android corresponding radio LOG
> 
> (cmd=0xB2,efid=0x6F4A,path=3F007F10,3,4,13,(null),pin2=(null),aid=(null))
> 01-03 23:55:50.580   137  1093 D use-Rlog/RLOG-RIL: [w] onRequest: SIM_IO
> sState=0
> 01-03 23:55:50.580   137  1093 D use-Rlog/RLOG-RIL: [w] channel5 state: '0' 
> 01-03 23:55:50.580   137  1093 D use-Rlog/RLOG-RIL: [w] get Channel ID '5'
> 01-03 23:55:50.580   137  1093 D use-Rlog/RLOG-AT: [w] Channel5: AT>
> AT+CRSM=178,28490,3,4,13,0,"3F007F10"
> 01-03 23:55:50.600   133   207 D use-Rlog/RLOG-AT: [w] Channel2: AT< +CRSM:
> 144,0,8098CE683CFFFFFFFFFFFFFFFFFF068121436587F9FFFFFFFFFFFFFF
> 01-03 23:55:50.600   133   207 D use-Rlog/RLOG-AT: [w] Channel2: AT< OK
> 01-03 23:55:50.600   133  1092 D use-Rlog/RLOG-RILC: [w] [0154]< SIM_IO
> {sw1=0x90,sw2=0x0,8098CE683CFFFFFFFFFFFFFFFFFF068121436587F9FFFFFFFFFFFFFF}
> 
> (cmd=0xB2,efid=0x6F3A,path=3F007F10,2,4,28,(null),pin2=(null),aid=(null))
> 01-03 23:55:50.600   133  1235 D use-Rlog/RLOG-RIL: [w] onRequest: SIM_IO
> sState=0
> 01-03 23:55:50.600   133  1235 D use-Rlog/RLOG-RIL: [w] channel2 state: '0' 
> 01-03 23:55:50.600   133  1235 D use-Rlog/RLOG-RIL: [w] get Channel ID '2'
> 01-03 23:55:50.600   133  1235 D use-Rlog/RLOG-AT: [w] Channel2: AT>
> AT+CRSM=178,28474,2,4,28,0,"3F007F10"
> 01-03 23:55:50.610   133   207 D use-Rlog/RLOG-AT: [w] Channel2: AT< +CRSM:
> 144,0,726968FFFFFFFFFFFFFFFFFFFFFF0581638869F6FFFFFFFFFFFFFFFF
> 01-03 23:55:50.610   133   207 D use-Rlog/RLOG-AT: [w] Channel2: AT< OK
> 01-03 23:55:50.610   133  1235 D use-Rlog/RLOG-RILC: [w] [0156]< SIM_IO
> {sw1=0x90,sw2=0x0,726968FFFFFFFFFFFFFFFFFFFFFF0581638869F6FFFFFFFFFFFFFFFF}

Hi yanyan,


First, as you mention a) |BCD number/SSC| maximux value is 11. However, in the file you gave me, the value is 21, which exceeds the maximum. As a result, we cannot proceed to read following value, since the check is failed.

Second, Reading records from EFext1 is not supported yet, so if there are more than 20 phone numbers, we can only get the first 20.
Flags: needinfo?(yanyan.an)
(In reply to johndai from comment #14)

> 
> Hi yanyan,
> 
> 
> First, as you mention a) |BCD number/SSC| maximux value is 11. However, in
> the file you gave me, the value is 21, which exceeds the maximum. As a
> result, we cannot proceed to read following value, since the check is failed.
> 
> Second, Reading records from EFext1 is not supported yet, so if there are
> more than 20 phone numbers, we can only get the first 20.

Hi johndai,

 Thanks for you attention!

 I venture to ask you a question, what is the value 21? Whether this 21 includes ext1 number length?

follow 3GPP TS 51.011 EFADN to see, the value does not contain the length of ext1, but only ADN/SSC length.

 a) Yes, see following radio log, if there are more than 20 phone numbers, in this response parcel length is 21(0x15).(see last line 158146486164846444644461FF01 from following radio log).

//corresponding radio log:
(cmd=0xB2,efid=0x6F3A,path=3f007f10,95,4,28,(null),pin2=(null),aid=(null))
01-07 17:22:24.553   148   598 D use-Rlog/RLOG-RIL: [w] onRequest: SIM_IO sState=4
01-07 17:22:24.553   148   598 D use-Rlog/RLOG-RIL: [w] channel1 state: '0' 
01-07 17:22:24.553   148   598 D use-Rlog/RLOG-RIL: [w] get Channel ID '1'
01-07 17:22:24.553   148   598 D use-Rlog/RLOG-AT: [w] Channel1: AT> AT+CRSM=178,28474,95,4,28,0,"3f007f10"
01-07 17:22:24.573   148   199 D use-Rlog/RLOG-AT: [w] Channel1: AT< +CRSM: 144,0,676D6774676A6D2C256D2C6A2E6D158146486164846444644461FF01
01-07 17:22:24.573   148   199 D use-Rlog/RLOG-AT: [w] Channel1: AT< OK
01-07 17:22:24.573   148   598 D use-Rlog/RLOG-RILC: [w] [0338]< SIM_IO {sw1=0x90,sw2=0x0,676D6774676A6D2C256D2C6A2E6D158146486164846444644461FF01}

 b) According to TS 151.011 clause 10.5.1 EF_ADN

Length of BCD number/SSC contents maximum value is 11, even when the actual ADN/SSC information length is greater than 11. so, if there are more than 20 phone numbers, in this response parcel length should be 11, not is 21(0x15).

see 158146486164846444644461FF01, '15'is the length of BCD number/SSC contents; '81' is TON and NPI;
'46486164846444644461' is ADN number; '01'is Extension1 Record Identifier.
When an ADN/SSC has extension, it is indicated by the extension1 identifier being unequal to 'FF'. 


 so, when there are more than 20 phone numbers, in this response parcel length should not be 21(0x15).

But for the value is 21, which exceeds the maximum, We can do a compatibility deal.
 
However, the value 21 is false, it may be a bug.

What do you think?
Flags: needinfo?(yanyan.an)
(In reply to yanyan from comment #15)
> (In reply to johndai from comment #14)
> 
> > 
> > Hi yanyan,
> > 
> > 
> > First, as you mention a) |BCD number/SSC| maximux value is 11. However, in
> > the file you gave me, the value is 21, which exceeds the maximum. As a
> > result, we cannot proceed to read following value, since the check is failed.
> > 
> > Second, Reading records from EFext1 is not supported yet, so if there are
> > more than 20 phone numbers, we can only get the first 20.
> 
> Hi johndai,
> 
>  Thanks for you attention!
> 
>  I venture to ask you a question, what is the value 21? Whether this 21
> includes ext1 number length?
> 
> follow 3GPP TS 51.011 EFADN to see, the value does not contain the length of
> ext1, but only ADN/SSC length.
> 
>  a) Yes, see following radio log, if there are more than 20 phone numbers,
> in this response parcel length is 21(0x15).(see last line
> 158146486164846444644461FF01 from following radio log).
> 
> //corresponding radio log:
> (cmd=0xB2,efid=0x6F3A,path=3f007f10,95,4,28,(null),pin2=(null),aid=(null))
> 01-07 17:22:24.553   148   598 D use-Rlog/RLOG-RIL: [w] onRequest: SIM_IO
> sState=4
> 01-07 17:22:24.553   148   598 D use-Rlog/RLOG-RIL: [w] channel1 state: '0' 
> 01-07 17:22:24.553   148   598 D use-Rlog/RLOG-RIL: [w] get Channel ID '1'
> 01-07 17:22:24.553   148   598 D use-Rlog/RLOG-AT: [w] Channel1: AT>
> AT+CRSM=178,28474,95,4,28,0,"3f007f10"
> 01-07 17:22:24.573   148   199 D use-Rlog/RLOG-AT: [w] Channel1: AT< +CRSM:
> 144,0,676D6774676A6D2C256D2C6A2E6D158146486164846444644461FF01
> 01-07 17:22:24.573   148   199 D use-Rlog/RLOG-AT: [w] Channel1: AT< OK
> 01-07 17:22:24.573   148   598 D use-Rlog/RLOG-RILC: [w] [0338]< SIM_IO
> {sw1=0x90,sw2=0x0,676D6774676A6D2C256D2C6A2E6D158146486164846444644461FF01}
> 
>  b) According to TS 151.011 clause 10.5.1 EF_ADN
> 
> Length of BCD number/SSC contents maximum value is 11, even when the actual
> ADN/SSC information length is greater than 11. so, if there are more than 20
> phone numbers, in this response parcel length should be 11, not is 21(0x15).
> 
> see 158146486164846444644461FF01, '15'is the length of BCD number/SSC
> contents; '81' is TON and NPI;
> '46486164846444644461' is ADN number; '01'is Extension1 Record Identifier.
> When an ADN/SSC has extension, it is indicated by the extension1 identifier
> being unequal to 'FF'. 
> 
> 
>  so, when there are more than 20 phone numbers, in this response parcel
> length should not be 21(0x15).
> 
> But for the value is 21, which exceeds the maximum, We can do a
> compatibility deal.
>  
> However, the value 21 is false, it may be a bug.
> 
> What do you think?
Hi yanyan,


a) I do agree with you, we should have a compatibility deal and not stuck in the |Import Contacts| page. If response parcel has incorrect |BCD number/SSC contents|, we shall neglect the content and continue the next record.

What do you think?

b) If there are more than 20 phone numbers, we can only get the first 20. I filed a Bug 1122376 to trace this issue.
(In reply to johndai from comment #16)
> (In reply to yanyan from comment #15)
> > (In reply to johndai from comment #14)
> > 
> > > 
> > > Hi yanyan,
> > > 
> > > 
> > > First, as you mention a) |BCD number/SSC| maximux value is 11. However, in
> > > the file you gave me, the value is 21, which exceeds the maximum. As a
> > > result, we cannot proceed to read following value, since the check is failed.
> > > 
> > > Second, Reading records from EFext1 is not supported yet, so if there are
> > > more than 20 phone numbers, we can only get the first 20.
> > 
> > Hi johndai,
> > 
> >  Thanks for you attention!
> > 
> >  I venture to ask you a question, what is the value 21? Whether this 21
> > includes ext1 number length?
> > 
> > follow 3GPP TS 51.011 EFADN to see, the value does not contain the length of
> > ext1, but only ADN/SSC length.
> > 
> >  a) Yes, see following radio log, if there are more than 20 phone numbers,
> > in this response parcel length is 21(0x15).(see last line
> > 158146486164846444644461FF01 from following radio log).
> > 
> > //corresponding radio log:
> > (cmd=0xB2,efid=0x6F3A,path=3f007f10,95,4,28,(null),pin2=(null),aid=(null))
> > 01-07 17:22:24.553   148   598 D use-Rlog/RLOG-RIL: [w] onRequest: SIM_IO
> > sState=4
> > 01-07 17:22:24.553   148   598 D use-Rlog/RLOG-RIL: [w] channel1 state: '0' 
> > 01-07 17:22:24.553   148   598 D use-Rlog/RLOG-RIL: [w] get Channel ID '1'
> > 01-07 17:22:24.553   148   598 D use-Rlog/RLOG-AT: [w] Channel1: AT>
> > AT+CRSM=178,28474,95,4,28,0,"3f007f10"
> > 01-07 17:22:24.573   148   199 D use-Rlog/RLOG-AT: [w] Channel1: AT< +CRSM:
> > 144,0,676D6774676A6D2C256D2C6A2E6D158146486164846444644461FF01
> > 01-07 17:22:24.573   148   199 D use-Rlog/RLOG-AT: [w] Channel1: AT< OK
> > 01-07 17:22:24.573   148   598 D use-Rlog/RLOG-RILC: [w] [0338]< SIM_IO
> > {sw1=0x90,sw2=0x0,676D6774676A6D2C256D2C6A2E6D158146486164846444644461FF01}
> > 
> >  b) According to TS 151.011 clause 10.5.1 EF_ADN
> > 
> > Length of BCD number/SSC contents maximum value is 11, even when the actual
> > ADN/SSC information length is greater than 11. so, if there are more than 20
> > phone numbers, in this response parcel length should be 11, not is 21(0x15).
> > 
> > see 158146486164846444644461FF01, '15'is the length of BCD number/SSC
> > contents; '81' is TON and NPI;
> > '46486164846444644461' is ADN number; '01'is Extension1 Record Identifier.
> > When an ADN/SSC has extension, it is indicated by the extension1 identifier
> > being unequal to 'FF'. 
> > 
> > 
> >  so, when there are more than 20 phone numbers, in this response parcel
> > length should not be 21(0x15).
> > 
> > But for the value is 21, which exceeds the maximum, We can do a
> > compatibility deal.
> >  
> > However, the value 21 is false, it may be a bug.
> > 
> > What do you think?
> Hi yanyan,
> 
> 
> a) I do agree with you, we should have a compatibility deal and not stuck in
> the |Import Contacts| page. If response parcel has incorrect |BCD number/SSC
> contents|, we shall neglect the content and continue the next record.
> 
> What do you think?
> 
> b) If there are more than 20 phone numbers, we can only get the first 20. I
> filed a Bug 1122376 to trace this issue.

Hi johndai,

  Thanks for you attention!   

  Is this case, we will have a compatibility deal.

  Thank you for your concern again!
If response parcel has incorrect |BCD number/SSC contents|, we shall neglect the content and continue next record.
Attachment #8545762 - Attachment is obsolete: true
Attachment #8549446 - Attachment is obsolete: true
Attachment #8549447 - Attachment is obsolete: true
Attachment #8551136 - Flags: review?(echen)
Blocks: 1123554
Comment on attachment 8551136 [details] [diff] [review]
Contacts can't be imported from some special SIM cards to device. (V1)

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

Please see my comments below. Thank you.

::: dom/system/gonk/ril_worker.js
@@ +10673,5 @@
>        if (numLen > ADN_MAX_BCD_NUMBER_BYTES) {
> +        if (DEBUG) {
> +          this.context.debug(
> +            "Warning: invalid length of BCD number/SSC contents - " + numLen);
> +        }

You have to seek the incoming buffer to the right position to skip remaining data.
And could we have some tests for the invalid length?
Attachment #8551136 - Flags: review?(echen)
(In reply to johndai from comment #18)
> Created attachment 8551136 [details] [diff] [review]
> Contacts can't be imported from some special SIM cards to device. (V1)
> 
> If response parcel has incorrect |BCD number/SSC contents|, we shall neglect
> the content and continue next record.

hi johndai,

    If the length of |BCD number/SSC contents| in response parcel, actually we also take a truncation process. What do you think?

      if (numLen > ADN_MAX_BCD_NUMBER_BYTES) {
        //throw new Error("invalid length of BCD number/SSC contents - " + numLen);
        numLen = ADN_MAX_BCD_NUMBER_BYTES;
        if (DEBUG) 
          this.context.debug(
            "length of BCD number/SSC contents is greater than 11, max length is set as : " + numLen);
      }
(In reply to yanyan from comment #20)
> (In reply to johndai from comment #18)
> > Created attachment 8551136 [details] [diff] [review]
> > Contacts can't be imported from some special SIM cards to device. (V1)
> > 
> > If response parcel has incorrect |BCD number/SSC contents|, we shall neglect
> > the content and continue next record.
> 
> hi johndai,
> 
>     If the length of |BCD number/SSC contents| in response parcel, actually
> we also take a truncation process. What do you think?
> 
>       if (numLen > ADN_MAX_BCD_NUMBER_BYTES) {
>         //throw new Error("invalid length of BCD number/SSC contents - " +
> numLen);
>         numLen = ADN_MAX_BCD_NUMBER_BYTES;
>         if (DEBUG) 
>           this.context.debug(
>             "length of BCD number/SSC contents is greater than 11, max
> length is set as : " + numLen);
>       }

Hi Yanyan, thanks for your proposal. But forcing the length to |ADN_MAX_BCD_NUMBER_BYTES| seems a bit tricky to me. IMHO, we should follow the spec whenever possible, and also if data contains an valid value (length in this case), we are unable to know the following data are still normal or actually they are corrupted as well, skipping whole data seems more reasonable to me. Thank you.
Update patch to address comment 19.
Attachment #8551136 - Attachment is obsolete: true
Attachment #8552342 - Flags: review?(echen)
Whiteboard: [sprd391958] → [sprd391958], PTR1_BLOCK
Whiteboard: [sprd391958], PTR1_BLOCK → [sprd391958], PTR1_Block
Comment on attachment 8552342 [details] [diff] [review]
Contacts can't be imported from some special SIM cards to device. (V2)

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

::: dom/system/gonk/tests/test_ril_worker_icc_ICCPDUHelper.js
@@ +464,5 @@
> +      number: "123456789",
> +      epectedNumber: "123456789"
> +    },
> +    {
> +      number: 0xff,

I guess this is for the case that length is `0xff`, but assign number to `0xff` isn't a correct setup, because you write data with |number.length| actually.

@@ +478,5 @@
> +  function do_test(aNumber, aExpectedNumber) {
> +    iccHelper.readDiallingNumber = function(numLen) {
> +      return aNumber.substring(0, numLen);
> +    };
> +    helper.writeHexOctet(number.length);

Is any reason changing from |number.length + 1| to |number.length|?
Attachment #8552342 - Flags: review?(echen)
(In reply to Edgar Chen [:edgar][:echen] from comment #23)
> Comment on attachment 8552342 [details] [diff] [review]
> Contacts can't be imported from some special SIM cards to device. (V2)
> 
> Review of attachment 8552342 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/tests/test_ril_worker_icc_ICCPDUHelper.js
> @@ +464,5 @@
> > +      number: "123456789",
> > +      epectedNumber: "123456789"
> > +    },
> > +    {
> > +      number: 0xff,
> 
> I guess this is for the case that length is `0xff`, but assign number to
> `0xff` isn't a correct setup, because you write data with |number.length|
> actually.
> 
Will do.

> @@ +478,5 @@
> > +  function do_test(aNumber, aExpectedNumber) {
> > +    iccHelper.readDiallingNumber = function(numLen) {
> > +      return aNumber.substring(0, numLen);
> > +    };
> > +    helper.writeHexOctet(number.length);
> 
> Is any reason changing from |number.length + 1| to |number.length|?

I think it is a typo of my code.
Thanks for catching these. I'll address these in next update.
This patch addresses two issues in comment 23.
1. Modifying incorrect number value.
2. Modifying |number.length| typo.
Attachment #8552342 - Attachment is obsolete: true
Attachment #8552899 - Flags: review?(echen)
Comment on attachment 8552899 [details] [diff] [review]
Contacts can't be imported from some special SIM cards to device. (V3)

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

::: dom/system/gonk/tests/test_ril_worker_icc_ICCPDUHelper.js
@@ +489,5 @@
> +          helper.writeHexOctet(0xff);
> +        }
> +      } else {
> +        for (let i = 0; i < ADN_MAX_BCD_NUMBER_BYTES - numLen; i++) {
> +          helper.writeHexOctet(0xff);

Per off-line sync, writing these data is for |seekIncoming|, but it is quite confused. How about just overriding |seekIncoming|?
Attachment #8552899 - Flags: review?(echen)
(In reply to Edgar Chen [:edgar][:echen] from comment #26)
> Comment on attachment 8552899 [details] [diff] [review]
> Contacts can't be imported from some special SIM cards to device. (V3)
> 
> Review of attachment 8552899 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/tests/test_ril_worker_icc_ICCPDUHelper.js
> @@ +489,5 @@
> > +          helper.writeHexOctet(0xff);
> > +        }
> > +      } else {
> > +        for (let i = 0; i < ADN_MAX_BCD_NUMBER_BYTES - numLen; i++) {
> > +          helper.writeHexOctet(0xff);
> 
> Per off-line sync, writing these data is for |seekIncoming|, but it is quite
> confused. How about just overriding |seekIncoming|?

Thanks for this suggestion. I'll address this in next update.
Modifying testcase to address comment 26.
Attachment #8552899 - Attachment is obsolete: true
Attachment #8552997 - Flags: review?(echen)
Comment on attachment 8552997 [details] [diff] [review]
Contacts can't be imported from some special SIM cards to device. (V4)

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

r=me with below nit addressed. Thank you, John.

::: dom/system/gonk/tests/test_ril_worker_icc_ICCPDUHelper.js
@@ +486,2 @@
>  
> +    if(aNumber != null) {

Nit: Space between `if` and `(`.

@@ +486,4 @@
>  
> +    if(aNumber != null) {
> +      let numLen = aNumber.length + 1;
> +      helper.writeHexOctet(numLen);

Nit: helper.writeHexOctet(aNumber.length + 1);
Attachment #8552997 - Flags: review?(echen) → review+
Modifying testcase to address comment 29.
Attachment #8552997 - Attachment is obsolete: true
Comment on attachment 8553453 [details] [diff] [review]
Contacts can't be imported from some special SIM cards to device. (Final), r=echen

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

::: dom/system/gonk/tests/test_ril_worker_icc_ICCPDUHelper.js
@@ +461,5 @@
>  
> +  let numbers = [
> +    {
> +      number: "123456789",
> +      epectedNumber: "123456789"

Is this a typo for "expectedNumber"?
Also, in commit messages, please describe the code change, not the bug symptom.
(In reply to David Major [:dmajor] (UTC+13) from comment #33)
> Comment on attachment 8553453 [details] [diff] [review]
> Contacts can't be imported from some special SIM cards to device. (Final),
> r=echen
> 
> Review of attachment 8553453 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/tests/test_ril_worker_icc_ICCPDUHelper.js
> @@ +461,5 @@
> >  
> > +  let numbers = [
> > +    {
> > +      number: "123456789",
> > +      epectedNumber: "123456789"
> 
> Is this a typo for "expectedNumber"?

Thanks for catching this, David.
Hi John, please help to provide a follow-up patch here to fix the typo.
Flags: needinfo?(jdai)
(In reply to David Major [:dmajor] (UTC+13) from comment #34)
> Also, in commit messages, please describe the code change, not the bug
> symptom.

Yes, we should do this, will be more carefully next time. Thank you.
This patch addresses two issues:
1. Modifying typo to address comment 33.
2. Modifying commit messages to address comment 34.
Attachment #8553453 - Attachment is obsolete: true
Flags: needinfo?(jdai)
(follow-up) Correcting typo s/epectedNumber/expectedNumber/.
Providing a follow-up patch to address comment 33.
Attachment #8556298 - Attachment is obsolete: true
Attachment #8556315 - Flags: review?(echen)
Attachment #8556269 - Attachment is obsolete: true
Attachment #8553453 - Attachment is obsolete: false
Attachment #8556315 - Attachment description: Correcting typo s/epectedNumber/expectedNumber/. → (follow-up) Correcting typo s/epectedNumber/expectedNumber/.
Comment on attachment 8556315 [details] [diff] [review]
(follow-up) Correcting typo s/epectedNumber/expectedNumber/.

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

LGTM, but please still provide the try result for this patch (with running only xpcshell test is enough) before landing. Thank you.
Attachment #8556315 - Flags: review?(echen) → review+
https://hg.mozilla.org/mozilla-central/rev/c45ef02bbe6e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
(In reply to johndai from comment #39)
> Created attachment 8556315 [details] [diff] [review]
> (follow-up) Correcting typo s/epectedNumber/expectedNumber/.

https://hg.mozilla.org/integration/b2g-inbound/rev/e525308016dd
Blocks: 1157082
You need to log in before you can comment on or make changes to this bug.