Closed Bug 947110 Opened 11 years ago Closed 11 years ago

B2G RIL: Add support for decoding FCP Template in BerTlvHelper.

Categories

(Firefox OS Graveyard :: RIL, defect)

Product:
Firefox OS Graveyard
Component:
RIL
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:1.3+, firefox27 wontfix, firefox28 fixed, firefox29 fixed, b2g-v1.3 fixed)

Status:
RESOLVED FIXED
Milestone:
1.3 C2/1.4 S2(17jan)
Project Flags:
blocking-b2g 1.3+
Tracking Flags:
Tracking Status
firefox27 --- wontfix
firefox28 --- fixed
firefox29 --- fixed
b2g-v1.3 --- fixed

People

(Reporter: allstars.chh, Assigned: sku)

References

Details

Attachments

(3 files, 11 obsolete files)

Bug 947110 - Part1: RIL patch. B2G RIL: Add support for decoding FCP Template in BerTlvHelper. v5. r=yoshi.
13.17 KB, patch
Details | Diff | Splinter Review
Bug 947110 - Part2: Add test cases. B2G RIL: Add support for decoding FCP Template in BerTlvHelper. v2. r=yoshi.
3.49 KB, patch
Details | Diff | Splinter Review
[Patch for 1.2f] Bug 947110 - B2G RIL: Add support for decoding FCP Template in BerTlvHelper. r=yoshi.
16.51 KB, patch
Details | Diff | Splinter Review 
This is found by Shawn.
FCP Template is also one of BER TLV format. (TS 101.220, clause 7.2)

We could reuse BerTlvHelper To decode FCP template from USIM (TS 102.221, 11.1.1.3.2).
fugu? because it blocks fugu+ bug 924740.
blocking-b2g: --- → fugu?
Sam, could you help to check this issue?
Flags: needinfo?(sam.hua)
Flags: needinfo?(sam.hua)
add ming.li.

Hi Steven, 
Any requirements for RILC or modem about this issue?
Attached patch Test Patch for Bug 947110. (obsolete) — — Splinter Review 
This is not a formal patch. And, still need test case part.
so 924740 will also be fixed after this modification?
(In reply to sam.hua from comment #5)
> so 924740 will also be fixed after this modification?

Yes, (924740 and 947110 are the same issue.)
Comment on attachment 8346437 [details] [diff] [review]
Test Patch for Bug 947110.

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

::: dom/system/gonk/ril_worker.js
@@ +4872,5 @@
>     * Process STK Proactive Command.
>     */
>    processStkProactiveCommand: function processStkProactiveCommand() {
>      let length = Buf.readInt32();
> +    let berTlv; 

remove space.
(In reply to shawn ku [:sku] from comment #6)
> (In reply to sam.hua from comment #5)
> > so 924740 will also be fixed after this modification?
> 
> Yes, (924740 and 947110 are the same issue.)

No, this bug is to add more functionalities in BerTlvHelper.

Bug 924740 is to handle the result the decoded by BerTLVHelper.

That's why we have seperate bugs.
Comment on attachment 8346437 [details] [diff] [review]
Test Patch for Bug 947110.

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

I guess you want to take this bug so have some comments here.

::: dom/system/gonk/ril_consts.js
@@ +705,5 @@
> +this.BER_FCP_SA_COMPACT_FORMAT_TAG = 0x8c; // Security Attribute - Compact Format.
> +this.BER_FCP_SAT_EXPANDED_FORMAT_TAG = 0xab; // Security Attribute Template - Expanded Format.
> +this.BER_FCP_PROPRIETARY_TEMPLATE_TAG = 0xa5;
> +this.BER_FCP_PIN_STATUS_DATA_OBJECTS_TAG = 0xc6;
> +

These consts should be put below with those existing values.

Also put those definitions in asscending order.

::: dom/system/gonk/ril_worker.js
@@ +4875,5 @@
>      let length = Buf.readInt32();
> +    let berTlv; 
> +    try {
> +      berTlv = BerTlvHelper.decode({tag: BER_PROACTIVE_COMMAND_TAG,
> +                                    dataLen: length / 2});

simply BerTlvHelper.decode(dataLen);

@@ +10852,5 @@
>    /**
>     * Decode Ber TLV.
>     *
> +   * @param tag
> +   *        The expected tag to be parsing.

no need.

@@ +10865,5 @@
>  
> +    if (options.tag !== tag) {
> +      throw new Error("Unexpected Ber tag 0x" + tag.toString(16));
> +      return null;
> +    }

no need.

@@ +10866,5 @@
> +    if (options.tag !== tag) {
> +      throw new Error("Unexpected Ber tag 0x" + tag.toString(16));
> +      return null;
> +    }
> +

should decode the length here.

this.decodeLength(dataLen);

Or call it processLength if you like.

@@ +10872,5 @@
> +    if (typeof method != "function") {
> +      throw new Error("Unknown Ber tag 0x" + tag.toString(16));
> +      return;
> +    }
> +

Since the length is handled here.

So you need to call those processXXX functions to get the value part.

let value = method.call(length);

return {
  tag: tag,
  length: length,
  value: value
};

@@ +11252,5 @@
>    /**
>     * Process a ICC_COMMAND_GET_RESPONSE type command for REQUEST_SIM_IO.
>     */
>    processICCIOGetResponse: function processICCIOGetResponse(options) {
>      let strLen = Buf.readInt32();

Move the code below to Bug 924740

@@ +13710,5 @@
>  
>  /**
> + * Helper for ICC FCP Template.
> + */
> +let ICCFcpTemplateHelper = {

It seems to me the code here is overlapped with BerTlvHelper.

And seems to me we won't need this helper and we can do it in BerTlvHelper.
Comment on attachment 8346437 [details] [diff] [review]
Test Patch for Bug 947110.

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

::: dom/system/gonk/ril_worker.js
@@ +10889,5 @@
> +    // The length is either 1 or 2 for FCP template.
> +    let length = GsmPDUHelper.readHexOctet();
> +    hlen++;
> +    if (dataLen !== (hlen + length)) {
> +      length = (length << 8) | GsmPDUHelper.readHexOctet();

This part is incorrect, need correction. see TS 101 220 7.1.2 Length encoding
Attachment #8348535 - Flags: review?(allstars.chh)
Attachment #8348537 - Flags: review?(allstars.chh)
Assignee: nobody → sku
Comment on attachment 8348535 [details] [diff] [review]
Bug 947110 - Part1: RIL patch. B2G RIL: Add support for decoding FCP Template in BerTlvHelper.

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

Mostly okay,
cancelling r? for I'd like to revise the handling of TLV object.

::: dom/system/gonk/ril_worker.js
@@ +10864,5 @@
>      let hlen = 0;
>      let tag = GsmPDUHelper.readHexOctet();
>      hlen++;
>  
> +    // The length is either 1 or 2 for Ber TLV format.

The length is coded onto 1 or 2 bytes.

@@ +10892,5 @@
> +    if (typeof method != "function") {
> +      throw new Error("Unknown Ber tag 0x" + tag.toString(16));
> +    }
> +
> +    return method.call(this, length);

So you return TLV in the method, which I think is not a good way to do this.

You decode tag octet and length octet(s) here, so it's better to construct the TLV in this function, which means the handler for BerTlvHelper is only to process the 'value' part.

Otherwise you can see each of your handle has to copy the 'tag' and 'length' from this function.

@@ +10939,5 @@
> +    return {
> +      tag: tag,
> +      length: length,
> +      value: this.retrieve(tag, length),
> +      hlen: hlen

we don't have to pass hlen.
Since the tag and the length will take 2 bytes.

@@ +10962,5 @@
> +    let method = this[tag];
> +    if (typeof method != "function") {
> +      if (DEBUG) {
> +        debug("Unknown Ber tag : 0x" + tag.toString(16));
> +      }

throw new Error(...);
Attachment #8348535 - Flags: review?(allstars.chh)
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #14)
> Comment on attachment 8348535 [details] [diff] [review]
> Bug 947110 - Part1: RIL patch. B2G RIL: Add support for decoding FCP
> Template in BerTlvHelper.
> 
> Review of attachment 8348535 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Mostly okay,
> cancelling r? for I'd like to revise the handling of TLV object.
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +10864,5 @@
> >      let hlen = 0;
> >      let tag = GsmPDUHelper.readHexOctet();
> >      hlen++;
> >  
> > +    // The length is either 1 or 2 for Ber TLV format.
> 
> The length is coded onto 1 or 2 bytes.

Will address this in next patch.

> 
> @@ +10892,5 @@
> > +    if (typeof method != "function") {
> > +      throw new Error("Unknown Ber tag 0x" + tag.toString(16));
> > +    }
> > +
> > +    return method.call(this, length);
> 
> So you return TLV in the method, which I think is not a good way to do this.
> 
> You decode tag octet and length octet(s) here, so it's better to construct
> the TLV in this function, which means the handler for BerTlvHelper is only
> to process the 'value' part.
> 
> Otherwise you can see each of your handle has to copy the 'tag' and 'length'
> from this function.
> 

Will address this in next patch.

> @@ +10939,5 @@
> > +    return {
> > +      tag: tag,
> > +      length: length,
> > +      value: this.retrieve(tag, length),
> > +      hlen: hlen
> 
> we don't have to pass hlen.
> Since the tag and the length will take 2 bytes.

Will do.

> 
> @@ +10962,5 @@
> > +    let method = this[tag];
> > +    if (typeof method != "function") {
> > +      if (DEBUG) {
> > +        debug("Unknown Ber tag : 0x" + tag.toString(16));
> > +      }
> 
> throw new Error(...);

"throw new Error(...);" might not be a proper action we should take here.
We got 8 tags from getResponse, and we only care about 0x82/0x83/0x80 currently.

If throw a new excpetion, we need to catch that within the same function block.
otherwise, there is no way for us to trigger "Buf.seekIncoming(length * Buf.PDU_HEX_OCTET_SIZE);" to ignore the chunk we don't need. That's why I use log for debugging's convenient.

What is you opinion?

Description                   Tag
File Descriptor               0x82
File Identifier               0x83
Proprietary information       0xA5
Life Cycle Status Integer     0x8A
Security attributes           0x8B || 0x8C || 0xAB
File size                     0x80
Total file size               0x81
Short File Identifier (SFI)   0x88
Hi Yoshi:
 Could you please check my bottom reply on comment 15?
Flags: needinfo?(allstars.chh)
blocking-b2g: fugu? → fugu+
(In reply to shawn ku [:sku] from comment #16)
> Hi Yoshi:
>  Could you please check my bottom reply on comment 15?

seekIncoming is better, please ignore my previous comment for 'throw new Error'.
Flags: needinfo?(allstars.chh)
Attachment #8349288 - Flags: review?(allstars.chh)
Comment on attachment 8349288 [details] [diff] [review]
Bug 947110 - Part1: RIL patch. B2G RIL: Add support for decoding FCP Template in BerTlvHelper. v2.

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

::: dom/system/gonk/ril_worker.js
@@ +10892,5 @@
> +    if (typeof method != "function") {
> +      throw new Error("Unknown Ber tag 0x" + tag.toString(16));
> +    }
> +
> +    return method.call(this, length);

? you only return 'value' object, instead of TLV ?

let value = method.call(...);

return {
  tag: tag,
  length: length,
  value: value
};

@@ +10903,5 @@
> +   *        The length of data in bytes.
> +   */
> +  processFcpTemplate: function processFcpTemplate(length) {
> +    let tlvs = this.decodeChunks(length);
> +    return {value: tlvs};

return tlvs;

@@ +10917,2 @@
>      let ctlvs = ComprehensionTlvHelper.decodeChunks(length);
> +    return {value: ctlvs};

ditto

@@ +10992,5 @@
> +    let dataCodingByte = GsmPDUHelper.readHexOctet();
> +    // See TS 102 221 Table 11.5, we only care the first 3 bytes for EF type.
> +    let fileType = fileDescriptorByte & 0x07;
> +
> +    let fileDescriptor;

let fileDescriptor = {
  fileType: fileType
};

@@ +11002,5 @@
> +        fileType: fileType,
> +        recordLength: ((GsmPDUHelper.readHexOctet() << 8) +
> +                       GsmPDUHelper.readHexOctet()),
> +        numOfRecords: GsmPDUHelper.readHexOctet()
> +      };

fileDescriptor.recordLength = ..;
fileDescriptor.numOfRecords = ...;

@@ +11003,5 @@
> +        recordLength: ((GsmPDUHelper.readHexOctet() << 8) +
> +                       GsmPDUHelper.readHexOctet()),
> +        numOfRecords: GsmPDUHelper.readHexOctet()
> +      };
> +    } else {

remove else.

@@ +11023,5 @@
> +    return {fileId : (GsmPDUHelper.readHexOctet() << 8) +
> +                      GsmPDUHelper.readHexOctet()};
> +  },
> +
> +  searchForTag: function searchForTag(tag, tlvs) {

who uses this function?

@@ +11028,5 @@
> +    let iter = Iterator(tlvs);
> +    return this.searchForNextTag(tag, iter);
> +  },
> +
> +  searchForNextTag: function searchForNextTag(tag, iter) {

here too.
Attachment #8349288 - Flags: review?(allstars.chh) → review-
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #19)
> Comment on attachment 8349288 [details] [diff] [review]
> Bug 947110 - Part1: RIL patch. B2G RIL: Add support for decoding FCP
> Template in BerTlvHelper. v2.
> 
> Review of attachment 8349288 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +10892,5 @@
> > +    if (typeof method != "function") {
> > +      throw new Error("Unknown Ber tag 0x" + tag.toString(16));
> > +    }
> > +
> > +    return method.call(this, length);
> 
> ? you only return 'value' object, instead of TLV ?

value contains berTLV (from FCP template) or COMPREHENSION-TLV (from Card application toolkit data objects)
the origin tag/lenght are not used outside, that's why I only return value.

I will add tag/length back in next patch. 

> 
> let value = method.call(...);
> 
> return {
>   tag: tag,
>   length: length,
>   value: value
> };
> 
> @@ +10903,5 @@
> > +   *        The length of data in bytes.
> > +   */
> > +  processFcpTemplate: function processFcpTemplate(length) {
> > +    let tlvs = this.decodeChunks(length);
> > +    return {value: tlvs};
> 
> return tlvs;
> 
> @@ +10917,2 @@
> >      let ctlvs = ComprehensionTlvHelper.decodeChunks(length);
> > +    return {value: ctlvs};
> 
> ditto
> 
> @@ +10992,5 @@
> > +    let dataCodingByte = GsmPDUHelper.readHexOctet();
> > +    // See TS 102 221 Table 11.5, we only care the first 3 bytes for EF type.
> > +    let fileType = fileDescriptorByte & 0x07;
> > +
> > +    let fileDescriptor;
> 
> let fileDescriptor = {
>   fileType: fileType
> };
> 
> @@ +11002,5 @@
> > +        fileType: fileType,
> > +        recordLength: ((GsmPDUHelper.readHexOctet() << 8) +
> > +                       GsmPDUHelper.readHexOctet()),
> > +        numOfRecords: GsmPDUHelper.readHexOctet()
> > +      };
> 
> fileDescriptor.recordLength = ..;
> fileDescriptor.numOfRecords = ...;
> 

Thanks! Will do.

> @@ +11003,5 @@
> > +        recordLength: ((GsmPDUHelper.readHexOctet() << 8) +
> > +                       GsmPDUHelper.readHexOctet()),
> > +        numOfRecords: GsmPDUHelper.readHexOctet()
> > +      };
> > +    } else {
> 
> remove else.
> 
> @@ +11023,5 @@
> > +    return {fileId : (GsmPDUHelper.readHexOctet() << 8) +
> > +                      GsmPDUHelper.readHexOctet()};
> > +  },
> > +
> > +  searchForTag: function searchForTag(tag, tlvs) {
> 
> who uses this function?

Shince this is a helper function, I prefer to let searchForTag/searchForNextTag APIs rady first.
It looks like to me you think there is no caller, these APIs does not need to be existed.

however, w/o these APIs, it's hard for us to write meaningful test case down.
> 
> @@ +11028,5 @@
> > +    let iter = Iterator(tlvs);
> > +    return this.searchForNextTag(tag, iter);
> > +  },
> > +
> > +  searchForNextTag: function searchForNextTag(tag, iter) {
> 
> here too.
(In reply to shawn ku [:sku] from comment #20)
> 
> Shince this is a helper function, I prefer to let
> searchForTag/searchForNextTag APIs rady first.
> It looks like to me you think there is no caller, these APIs does not need
> to be existed.
> 
> however, w/o these APIs, it's hard for us to write meaningful test case down.

You could write your util in the test case.
Also when reviewing this I think I should file another bug for creating some util function for ComprehensionTlvHelper, BerTlvHelper for handling TLV.
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #21)
> (In reply to shawn ku [:sku] from comment #20)
> > 
> > Shince this is a helper function, I prefer to let
> > searchForTag/searchForNextTag APIs rady first.
> > It looks like to me you think there is no caller, these APIs does not need
> > to be existed.
> > 
> > however, w/o these APIs, it's hard for us to write meaningful test case down.
> 
> You could write your util in the test case.
> Also when reviewing this I think I should file another bug for creating some
> util function for ComprehensionTlvHelper, BerTlvHelper for handling TLV.

I believe the ComprehensionTlvHelper was covered by test_stk_proactive_command_search_next_tag@test_ril_worker_stk.js unless you think it is necessary to add more tests on it.
Comment on attachment 8349891 [details] [diff] [review]
Bug 947110 - Part1: RIL patch. B2G RIL: Add support for decoding FCP Template in BerTlvHelper. v3.

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

Update address parts for part1 patch.
Attachment #8349891 - Flags: review?(allstars.chh)
Comment on attachment 8348537 [details] [diff] [review]
Bug 947110 - Part2: Add test cases. B2G RIL: Add support for decoding FCP Template in BerTlvHelper.

Obsolete part2 since searchForTag/searchForNextTag are removed.
Attachment #8348537 - Attachment is obsolete: true
Attachment #8348537 - Flags: review?(allstars.chh)
Attachment #8349904 - Flags: review?(allstars.chh)
Comment on attachment 8349891 [details] [diff] [review]
Bug 947110 - Part1: RIL patch. B2G RIL: Add support for decoding FCP Template in BerTlvHelper. v3.

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

::: dom/system/gonk/ril_consts.js
@@ +536,4 @@
>  this.RESPONSE_DATA_STRUCTURE = 13;
>  this.RESPONSE_DATA_RECORD_LENGTH = 14;
>  
>  // Types of files  TS 11.11 9.3

Actually it should be struture of file.

Structure could be LinearFixed, Cyclic, Transparent or BerTLV.
Type could be MF, DF or EF.

I'll file another bug to fix this.

@@ +541,5 @@
>  this.EF_TYPE_LINEAR_FIXED = 1;
>  this.EF_TYPE_CYCLIC = 3;
>  
> +// TS 102.221 11.1.1.4.3 Table 11.5: File descriptor byte.
> +this.UICC_EF_TYPE = {};

rename it to structure, not type.

::: dom/system/gonk/ril_worker.js
@@ +10895,5 @@
> +
> +    let value = method.call(this, length);
> +
> +    return {
> +     tag: tag,

indent. 
Use two spaces.

@@ +10902,5 @@
> +    };
> +  },
> +
> +  /**
> +   * Construct a param for FCP template tlv.

process the value part for FCP template TLV.

@@ +10913,5 @@
> +    return tlvs;
> +  },
> +
> +  /**
> +   * Construct a param for proactive command tlv.

ditto.

@@ +10926,5 @@
> +
> +  /**
> +   * Decode raw data to a Ber-TLV.
> +   */
> +  decodeBerTlv: function decodeBerTlv() {

decodeInnerTlv, since you're decoding the inner BER TLVs, and try to prevent confusion with original decode().

@@ +10945,5 @@
> +      if (tlv.value) {
> +        chunks.push(tlv);
> +      }
> +      index += tlv.length;
> +      // tag + length fields consume 2 bytes for FCP template.

remove 'for FCP template'.

Since there are many kinds of BER tags.

@@ +10975,5 @@
> +   */
> +  retrieveFileSizeData: function retrieveFileSizeData(length) {
> +    let fileSizeData = 0;
> +    for (let i = 0; i < length; i++) {
> +      fileSizeData += (GsmPDUHelper.readHexOctet() << (8*(length-i-1)));

fileSizeData = fileSizeData << 8;
fileSizeData += GsmPDUHelper.readHexOctet();

@@ +10995,5 @@
> +   */
> +  retrieveFileDescriptor: function retrieveFileDescriptor(length) {
> +    let fileDescriptorByte = GsmPDUHelper.readHexOctet();
> +    let dataCodingByte = GsmPDUHelper.readHexOctet();
> +    // See TS 102 221 Table 11.5, we only care the first 3 bytes for EF type.

the least 3 bits for the structure of file.

@@ +10996,5 @@
> +  retrieveFileDescriptor: function retrieveFileDescriptor(length) {
> +    let fileDescriptorByte = GsmPDUHelper.readHexOctet();
> +    let dataCodingByte = GsmPDUHelper.readHexOctet();
> +    // See TS 102 221 Table 11.5, we only care the first 3 bytes for EF type.
> +    let fileType = fileDescriptorByte & 0x07;

rename to 'structure'.
Attachment #8349891 - Flags: review?(allstars.chh)
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #27)
 > ::: dom/system/gonk/ril_consts.js
> @@ +536,4 @@
> >  this.RESPONSE_DATA_STRUCTURE = 13;
> >  this.RESPONSE_DATA_RECORD_LENGTH = 14;
> >  
> >  // Types of files  TS 11.11 9.3
> 
> Actually it should be struture of file.
> 
> Structure could be LinearFixed, Cyclic, Transparent or BerTLV.
> Type could be MF, DF or EF.
> 
> I'll file another bug to fix this.
> 

Bug 952025.

        Attachment #8349904 -
        Attachment is obsolete: true

        Attachment #8349904 -
        Flags: review?(allstars.chh)

    
  

        Attachment #8349954 -
        Flags: review?(allstars.chh)

    
  

        Attachment #8349957 -
        Flags: review?(allstars.chh)

    
    

    
  
Comment on attachment 8349954 [details] [diff] [review]
Bug 947110 - Part1: RIL patch. B2G RIL: Add support for decoding FCP Template in BerTlvHelper. v4.

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

Thanks.
Add r=me.

        Attachment #8349954 -
        Flags: review?(allstars.chh) → review+

        Attachment #8349957 -
        Flags: review?(allstars.chh) → review+

    
    

    
  
From the try I think Marionette-webapi tests are fail.
Keywords: checkin-needed

    
    

    
  
Thanks Yoshi, I will check what cause this failures on Monday morning.

    
    

    
  
Thanks for Youshi again....

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #36)
> From the try I think Marionette-webapi tests are fail.

The root cause is:
"if (dataLen - hlen < length) {" was changed to "if (dataLen - hlen !== length) {" in patch

and for two-bytes length BerTLV, hlen++ is missed (should be a old bug, but not found yet).
That cause an unexpected expcetion thrown.

I will provide a new patch to review on Monday.



// Original one
    } else if (temp === 0x81) {
      length = GsmPDUHelper.readHexOctet();

// Fixed one
    } else if (temp === 0x81) {
      length = GsmPDUHelper.readHexOctet();
+      hlen++;



  decode: function decode(dataLen) {
    let hlen = 0;
    let tag = GsmPDUHelper.readHexOctet();
    hlen++;

    // The length is coded onto 1 or 2 bytes.
    // Length    | Byte 1                | Byte 2
    // 0 - 127   | length ('00' to '7f') | N/A
    // 128 - 255 | '81'                  | length ('80' to 'ff')
    let length;
    let temp = GsmPDUHelper.readHexOctet();
    hlen++;
    if (temp < 0x80) {
      length = temp;
    } else if (temp === 0x81) {
      length = GsmPDUHelper.readHexOctet();
      if (length < 0x80) {
        throw new Error("Invalid length " + length);
      }
    ...

    
  

        Attachment #8350973 -
        Flags: review?(allstars.chh)

        Attachment #8350973 -
        Flags: review?(allstars.chh) → review+

    
    

    
  
fugu DSDS use v1.3 branch. nominate 1.3? here.
blocking-b2g: fugu+ → 1.3?

    
    

    
  
Yes, it is a 1.3+ bug. Without this, the DSDS SIM functions doesn't work.
blocking-b2g: 1.3? → 1.3+

    
    

    
  
Shawn, you may need to provide a patch for 1.3 branch.
Flags: needinfo?(sku)
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: