989135 - [B2G][Component][Location] Having a quotation mark in the Wi-Fi hotspot password or SSID is not handled gracefully.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[Lionel Mauritson]

Attachment: logcat_20140327_1615.txt

Description:
If a quotation mark ( " ) is entered in to the SSID or Password text field the Wifi hotspot will not remain enabled if turned on. After turning Wifi hotspot on after a few seconds it will turn itself back off with no warning or explanation to the user.

Repro Steps:
1) Update a Buri to BuildID: 20140326000201
2) Go to Settings -> Internet sharing
3) Make sure Wifi hotspot is off and go to Hotspot settings
4) Change the SSID or Password to include a quotation mark

Actual:
After turning Wifi hotspot on after a few seconds it will turn itself back off with no warning or explanation to the user.

Expected:
The user is notified of any issues regarding the text entered in the SSID or Password fields.

1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140326000201
Gaia: 7e705dd4718d528974d99ac31866318d7e201152
Gecko: 4889124accfa
Version: 30.0a2
Firmware Version: v1.2-device.cfg


Keywords: Tethering, Wi-fi, Internet sharing, Settings,


Notes: Along the same lines there is no warning when the password is too short, though the user will be unable to press "OK." This caught me off guard once.

Related to: bug 828925, 

Repro frequency: 100%

Link to failed test case: https://moztrap.mozilla.org/manage/case/8752/

See attached: Logcat, video: http://youtu.be/2p0agIbIXbA

Comment 1

[Lionel Mauritson]

This issue does NOT occur on 1.3

1.3 Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140327004001
Gaia: 8d159066289fdb4651e13e838fb91fb9226a2cd5
Gecko: 4c7778cc0c98
Version: 28.0
Firmware Version: v1.2-device.cfg

Entering a quotation mark in the SSID or Password field does not cause any obvious issues. Wifi remains enabled and other devices are able to connect to and use the hotspot.

Comment 2

[Harvey]

Last Working
1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140129121913
Gaia: ea7971a9f5bbf63a3977161f09ce7138202c7c62
Gecko: 90bfbf075f3f
Version: 29.0a1
Firmware Version: v1.2-device.cfg

First Broken
1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140129123816
Gaia: 0bc0e703df197d46dfffb9ac65cb85d2e3e10c4a
Gecko: 73eefb421e2a
Version: 29.0a1
Firmware Version: v1.2-device.cfg

First Broken Gecko last working Gaia: Issue Reproduces
Gaia: ea7971a9f5bbf63a3977161f09ce7138202c7c62
Gecko: 73eefb421e2a


First broken Gaia Last Working Gaia: Issue does NOT reproduce
Gaia: 0bc0e703df197d46dfffb9ac65cb85d2e3e10c4a
Gecko: 90bfbf075f3f

Gecko Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=90bfbf075f3f&tochange=73eefb421e2a

Comment 3

[Jason Smith [:jsmith]]

Looks like bug 864931 caused this regression.

Dimi - Can you confirm? If that's true, then this should be nominated to 1.3T, since this landed there as well.

Comment 4

[Dimi Lee [:dimi]]

Attachment: 989135 fix v1

Comment 5

[Dimi Lee [:dimi]]

Attachment: bug 989135 Fix v2

Comment 6

[Jason Smith [:jsmith]]

Bumping to 1.3T? as this regression hits that branch as well.

Comment 7

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

1.3T+ regresion

Comment 8

[Vincent Chang[:vchang][changyihsin]]

Comment on attachment 8399889 [details] [diff] [review]
bug 989135 Fix v2

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

r=me with nits addressed.

::: dom/system/gonk/NetworkUtils.cpp
@@ +74,5 @@
>  static const char* USB_CONFIG_DELIMIT = ",";
>  static const char* NETD_MESSAGE_DELIMIT = " ";
>  
>  static const uint32_t BUF_SIZE = 1024;
> +static const uint32_t MAX_SSID_SIZE = 32;

Nit: The maximum SSID length should be 32 + the character "\0".

@@ +269,5 @@
>  
>  /**
> + * Helper function to do string search and replace.
> + */
> +static void replace(const char* src, const char* strold, const char* strnew, char* dst) {

Nit: maximum 80 characters per line.

Comment 9

[Dimi Lee [:dimi]]

Attachment: Bug_989135_v3.patch

Comment 10

[Dimi Lee [:dimi]]

Attachment: Bug_989135_v4.patch

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/a6d44e79977c

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a6d44e79977c

Comment 13

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/e757fdd55426

Comment 14

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Comment on attachment 8402470 [details] [diff] [review]
Bug_989135_v4.patch

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

I just notice this patch, and I think we need a follow up to fix the issue in this patch.

::: dom/system/gonk/NetworkUtils.cpp
@@ +74,5 @@
>  static const char* USB_CONFIG_DELIMIT = ",";
>  static const char* NETD_MESSAGE_DELIMIT = " ";
>  
>  static const uint32_t BUF_SIZE = 1024;
> +static const uint32_t MAX_SSID_SIZE = 33;

Although max length for SSID is 32 bytes, the max possible length for SSID becomes 64 bytes after |escapeQuote()|.
For example, SSID string consists of 32 '\'s will become 64 '\'s after escape.

@@ +279,5 @@
> +  char *r;
> +  uint32_t oldlen = strlen(strold);
> +  uint32_t newlen = strlen(strnew);
> +
> +  for (p = src, r = dst; (q = strstr(p, strold)) != nullptr; p = q + oldlen) {

We didn't check dst size, might cause buffer overflow here.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Is this something that's still wanted on 1.4? Seems that way per the whiteboard, but I don't see an approval request here.

Comment 17

[Dimi Lee [:dimi]]

Comment on attachment 8402470 [details] [diff] [review]
Bug_989135_v4.patch

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

[Approval Request Comment]
Bug caused by             : This is regressing bug 864931
User impact if declined   : If user enter quotation mark in the Wi-Fi hotspot password or SSID, hotspot mode won't work
Testing completed         : Yes.
Risk to taking this patch : Medium, there is also a followup bug 994524 need to be landed
String or IDL/UUID changes made by this patch: None.

Comment 18

[Michael Vines [:m1] [:evilmachines]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #16)
> Is this something that's still wanted on 1.4? Seems that way per the
> whiteboard, but I don't see an approval request here.

Yes we need this in v1.4 please.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8402470 [details] [diff] [review]
Bug_989135_v4.patch

v1.4 is off Aurora and on to b2g30_v1_4 now, so switching the approval request.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/704341d3a26a

Comment 21

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

This is verified fixed on today's Tarako build


1.3t Environmental Variables:
Device: Tarako 1.3t
BuildID: 20140501014002
Gaia: d26a776beae0070b0032248a2ce482bbe6321a6d
Gecko: e90f4b655511
Version: 28.1
Firmware Version: sp6821