934901 - [WAP push][CP] All APN settings will be removed from data and message settings after storing a CP message with non-support APPID.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Enpei]

Attachment: time stamp: 03:25~29 11/5

All default and new stored APN settings will be gone from data and message settings under Cellular & Data once user config a new CP message with non-support APPID.

* Build Number                
Gaia:     1aee772a384f1ed1148f08c6c7df45d2fe35506e
Gecko:    http://hg.mozilla.org/mozilla-central/rev/b4143e04bea1
BuildID   20131104044747
Version   28.0a1

* Reproduce Steps
1. Based on bug 917312 that we will only support APPID w2 and w4. So send mms or data CP message with APPID wA.
2. Enter correct PIN and apply the APN.
3. Go to Cellular & Data > Data or Message settings.

* Expected Result
Not sure how FxOS should handle non-support settings. But original APN settings should remain unchanged.

* Actual Result
Only custome settings left. Others are gone, need to hard reset(cold boot) phone to recover the setting.

* Occurrence rate
100%

Comment 1

[Enpei]

Attachment: before1.png

Comment 2

[Enpei]

Attachment: before2.png

Comment 3

[Enpei]

Attachment: after1.png

Comment 4

[Enpei]

Attachment: after2.png

Comment 5

[Enpei]

Attachment: CP message sent for the test

Comment 6

[Beatriz Rodríguez [:brg]]

(In reply to Enpei from comment #0)
> * Expected Result
> Not sure how FxOS should handle non-support settings. But original APN
> settings should remain unchanged.

Ok + The document is received and processed correctly and ignoring the settings for applications not supported by the device.
> 
> * Actual Result
> Only custome settings left. Others are gone, need to hard reset(cold boot)
> phone to recover the setting.

Hi Enpei, thanks for the report, this is a blocker issue.

Comment 7

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Enpei from comment #5)
> Created attachment 827262 [details]
> CP message sent for the test

I'm not able to reproduce the bug with this provisioning document. The APPLICATION characteristic node whose APPI value is `wA` will be ignored (see [1] please).

The provisioning document attached has a valid APN which is:
{"NAPID":"TWN_INTERNET_NAPID","carrier":"TWN_INTERNET","apn":"mms","type":["mms"],"mmsc":"http://mms.catch.net.tw","mmsproxy":"10.1.1.2","TO-NAPID":["TWN_INTERNET_NAPID"],"mmsport":"80"}

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/wappush/js/parsed_doc.js#L247-L249

Comment 8

[Enpei]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #7)
> (In reply to Enpei from comment #5)
> > Created attachment 827262 [details]
> > CP message sent for the test
> 
> I'm not able to reproduce the bug with this provisioning document. The
> APPLICATION characteristic node whose APPI value is `wA` will be ignored
> (see [1] please).
Have you removed Settings from Application manager after step 2? sorry that I should add in reproduce step, there is a bug 934869 that you need to do extra step to see latest APN setting after storing from CP message.

> The provisioning document attached has a valid APN which is:
> {"NAPID":"TWN_INTERNET_NAPID","carrier":"TWN_INTERNET","apn":"mms","type":
> ["mms"],"mmsc":"http://mms.catch.net.tw","mmsproxy":"10.1.1.2","TO-NAPID":
> ["TWN_INTERNET_NAPID"],"mmsport":"80"}
> 
> [1]
> https://github.com/mozilla-b2g/gaia/blob/master/apps/wappush/js/parsed_doc.
> js#L247-L249

Comment 9

[José Antonio Olivera Ortega [:jaoo]]

Attachment: 934901-test.patch

I'm trying to reproduce the issue with this patch and I'm not able to reproduce it :(. This patch allows the user to receive the provisioning document in the terminal. The WAP push app is now in the grid and it has a button to receive the provisioning document. In order to give a try the user has to open the WAP push app and click on the button. After that a new client provisioning message is received.

Well, here are the steps I've taken and I'm not able to reproduce the bug.
1.- Open the setting app and see which APNs for MMS the devices has.
2.- (Without closing the setting app) Receive a couple of CP messages.
3.- (With the setting app still open) See the APNs for MMS the device has. The list of APNs for MMS has not been updated (bug 934869).
4.- Close the setting app.
5.- Open the setting app again.
6.- See the list of APNs for MMS. The list has been updated.
7.- Close the setting app.
8.- Receive a new CP message.
9.- Open the setting app and see which APNs for MMS the devices has. The list has been updated and it has one APN more.

Enpei, Is there any difference with the steps you took? Thanks.

Comment 10

[José Antonio Olivera Ortega [:jaoo]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #9)
> Created attachment 827433 [details] [diff] [review]
> 934901-test.patch
> 
> I'm trying to reproduce the issue with this patch and I'm not able to
> reproduce it :(. This patch allows the user to receive the provisioning
> document in the terminal. The WAP push app is now in the grid and it has a
> button to receive the provisioning document. In order to give a try the user
> has to open the WAP push app and click on the button. After that a new
> client provisioning message is received.
> 
> Well, here are the steps I've taken and I'm not able to reproduce the bug.
> 1.- Open the setting app and see which APNs for MMS the devices has.
> 2.- (Without closing the setting app) Receive a couple of CP messages.
> 3.- (With the setting app still open) See the APNs for MMS the device has.
> The list of APNs for MMS has not been updated (bug 934869).
> 4.- Close the setting app.
> 5.- Open the setting app again.
> 6.- See the list of APNs for MMS. The list has been updated.
> 7.- Close the setting app.
> 8.- Receive a new CP message.
> 9.- Open the setting app and see which APNs for MMS the devices has. The
> list has been updated and it has one APN more.
> 
> Enpei, Is there any difference with the steps you took? Thanks.

Well I've figured out what the different is. The provisioning doc attached have a valid APPLICATION characteristic node and another one invalid. That's the reason I'm not able to reproduced the issue.

I've tried with a provisioning doc with only one APPLICATION characteristic node which is invalid and I'm able to reproduce the issue.

Patch coming.

Comment 11

[José Antonio Olivera Ortega [:jaoo]]

Attachment: 934901-test.patch

This patch is useful to give a try and reproduce the issue without a server.

Comment 12

[José Antonio Olivera Ortega [:jaoo]]

Attachment: v1

Some background first for the reviewer is always welcome. Well, the provisioning doc in the WAP push message might not have valid APPLICATION characteristic nodes (other ones that those ones for the Browsing Enabler and AC for the Multimedia Messaging System Enabler which are APNs for data calls and for MMS). In this case we must not parse network access points pointed by the TO-NAPID parameter in these invalid APPLICATION characteristic nodes. We were parsing these APNs and storing them. The problem is that for these network access points we cannot give a value (default/mms) to the `type` property of the APN object and we were having an error (see [1]) in the settings app causing no APNs in the APN panels in the setting app at all.

Current patch fixes the issue. We are not parsing network access points pointed by the TO-NAPID parameter in these invalid APPLICATION characteristic nodes any more.

I know this is a bit difficult to understand so let me know if you have any question.

Gabriele, I'm sorry for bothering you with these reviews but you are the one with some background and knowledge about we have here. Would you review this patch please? Thanks.
 
[1] E/GeckoConsole( 3945): [JavaScript Error: "TypeError: apnList[i].type is undefined" {file: "app://settings.gaiamobile.org/js/carrier.js" line: 66}]

Comment 13

[Enpei]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #12)
> Created attachment 827518 [details] [diff] [review]
> v1
> 
> Some background first for the reviewer is always welcome. Well, the
> provisioning doc in the WAP push message might not have valid APPLICATION
> characteristic nodes (other ones that those ones for the Browsing Enabler
> and AC for the Multimedia Messaging System Enabler which are APNs for data
> calls and for MMS). In this case we must not parse network access points
> pointed by the TO-NAPID parameter in these invalid APPLICATION
> characteristic nodes. We were parsing these APNs and storing them. The
> problem is that for these network access points we cannot give a value
> (default/mms) to the `type` property of the APN object and we were having an
> error (see [1]) in the settings app causing no APNs in the APN panels in the
> setting app at all.
> 
> Current patch fixes the issue. We are not parsing network access points
> pointed by the TO-NAPID parameter in these invalid APPLICATION
> characteristic nodes any more.
Hi José, from user's point of view, what would we see when we receive a CP with non-support APPID after the bug is fixed? Still store it?

Comment 14

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Enpei from comment #13)

> Hi José, from user's point of view, what would we see when we receive a CP
> with non-support APPID after the bug is fixed? Still store it?

The application shows a confirm dialog alerting the user about there are no valid APNs in the CP message to store. The string message is 'The configuration message you received does not have valid APNs.'.

Comment 15

[Enpei]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #14)
> The application shows a confirm dialog alerting the user about there are no
> valid APNs in the CP message to store. The string message is 'The
> configuration message you received does not have valid APNs.'.

Thank you. Sorry that I would like to clarify again. So this means that as long as there is invalid characteristic type even there are valid ones, the message will be regarded as invalid APN, right?

Comment 16

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Enpei from comment #15)
> (In reply to José Antonio Olivera Ortega [:jaoo] from comment #14)
> > The application shows a confirm dialog alerting the user about there are no
> > valid APNs in the CP message to store. The string message is 'The
> > configuration message you received does not have valid APNs.'.
> 
> Thank you. Sorry that I would like to clarify again. So this means that as
> long as there is invalid characteristic type even there are valid ones, the
> message will be regarded as invalid APN, right?

Valid APNs from w2 and/or w4 APPLICATION characteristic nodes will be still accepted. If a CP message has both valid and invalid APNs only the valid ones will be stored.

Comment 17

[Preeti Raghunath(:Preeti)]

Broken feature. APN settings should not be entered by user.

Comment 18

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 827518 [details] [diff] [review]
v1

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

Sorry for the delay but I was busy landing stuff myself :) This is good for me, land at will.

Comment 19

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Pointer to Github PR: https://github.com/mozilla-b2g/gaia/pull/13467

Comment 20

[José Antonio Olivera Ortega [:jaoo]]

master: https://github.com/mozilla-b2g/gaia/commit/fd1ba23974088bb652044a2e8905dde5c296031a

Please, do not uplift this patch until bug 917312 gets landed in v1.2 branch.

Comment 21

[Enpei]

Verified on master 
Buri
Gaia:     cc9fec48957c9d217221d403aac4b5f35a826dd6
Gecko:    http://hg.mozilla.org/mozilla-central/rev/16949049f03d
BuildID   20131110040200
Version   28.0a1

Comment 22

[Jason Smith [:jsmith]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=917312#c88.