499666 - Support more methods to get AP list than libiw

Status: RESOLVED WORKSFORME

(Core :: DOM: Geolocation, defect)

Description

[Mikhail Khlopotov]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; ru; rv:1.9.1) Gecko/20090616 Firefox/3.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; ru; rv:1.9.1) Gecko/20090616 Firefox/3.5

When I try to play with geolocation on Linux on this page -
http://www.mozilla.com/en-US/firefox/geolocation/

Firefox does not submit access points available.
Instead, it submits a zero-filled descrition of one access point:

{"version":"1.1.0","access_token":"2:2lbU2s9YlIv_dacf:3M4mTGwfIDthjsPj","wifi_towers":[{"mac_address":"00-00-00-00-00-00","ssid":"","signal_strength":0}]}

I have enabled logging by editing NetworkGeolocationProvider.js, here's the result:
*** WIFI GEO: startup called
*** WIFI GEO: provider url = https://www.google.com/loc/json
*** WIFI GEO: watch called
*** WIFI GEO: onChange called
*** WIFI GEO: client sending: {"version":"1.1.0","access_token":"2:2lbU2s9YlIv_dacf:3M4mTGwfIDthjsPj","wifi_towers":[{"mac_address":"00-00-00-00-00-00","ssid":"","signal_strength":0}]}
*** WIFI GEO: service returned: {"location":{"latitude":xxxxxxx,"longitude":xxxxxx,"accuracy":xx000.0}}
*** WIFI GEO: shutdown  called

Reproducible: Always

Steps to Reproduce:
1. Go to any page, requesting location
2. Permit location request
3. See location API request body for a list of AP's nearby.
Actual Results:  
Firefox submits {"version":"1.1.0","access_token":"2:2lbU2s9YlIv_dacf:3M4mTGwfIDthjsPj","wifi_towers":[{"mac_address":"00-00-00-00-00-00","ssid":"","signal_strength":0}]}

Expected Results:  
Firefox should submit
{"version":"1.1.0","access_token":"2:Br1U1be3LjOpCSLN:5KPUVUQd7JGUVFV5","wifi_towers":[{"mac_address":"xx-xx-xx-xx-xx-xx","ssid":"jootter","signal_strength":-29},{"mac_address":"xx-xx-xx-xx-xx-xx","ssid":"My Home Wi-Fi","signal_strength":-63},{"mac_address":"xx-xx-xx-xx-xx-xx","ssid":"UnderNet","signal_strength":-65},{"mac_address":"xx-xx-xx-xx-xx-xx","ssid":"Asus-7-97","signal_strength":-58},{"mac_address":"xx-xx-xx-xx-xx-xx","ssid":"poswifi","signal_strength":-45},{"mac_address":"xx-xx-xx-xx-xx-xx","ssid":"dlink","signal_strength":-69},{"mac_address":"xx-xx-xx-xx-xx-xx","ssid":"VQQ_wi-fi","signal_strength":-78},{"mac_address":"xx-xx-xx-xx-xx-xx","ssid":"Crowne Plaze","signal_strength":-74}]}

Windows version submits a list of access points correctly.

Comment 1

[cmtalbert]

Doug, do you know what's going on here?  I can't reproduce this. Doing the same thing I get the following messages posted:

*** WIFI GEO: startup called
*** WIFI GEO: provider url = https://www.google.com/loc/json
*** WIFI GEO: watch called
*** WIFI GEO: onChange called
*** WIFI GEO: client sending: {"version":"1.1.0","access_token":"2:mUeuftFT28uBHbtz:--k5OpB0iAFv2Ubz","wifi_towers":[]}
*** WIFI GEO: service returned: {"location":{"latitude":37.415,"longitude":-122.059,"accuracy":24000.0}}
*** WIFI GEO: shutdown  called

I don't see any mac addresses being output in the logs.  Mikhail, did you do more than just uncomment the lines in the LOG function of NetworkGeolocationProvider.js?  However, given that the site works for me, Firefox should be submitting the proper data.

Tested on: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1pre) Gecko/20090609 Shiretoko/3.5pre

Comment 2

[Doug Turner (:dougt)]

No idea.  what type/version of linux?

Comment 3

[Mikhail Khlopotov]

Clint,
I noticed this behavior before making any changes to files.
I suppose my case is different from yours - I run FF on a laptop with working WiFi adapter, and I have quite a number of networks available, and I expect them to be submitted.
Looking through NetworkGeolocationProvider.js it seems like FF detects the fact that there are wireless networks, but fails to list them properly.

If I can do more debug on my laptop, please let me know.

Dough,
It's Ubuntu 9.01

Comment 4

[Doug Turner (:dougt)]

Mikhail,
do you have iwconfig installed?  can you report the output?

Comment 5

[Mikhail Khlopotov]

iwconfig when not associated:

lo        no wireless extensions.

eth0      no wireless extensions.

irda0     no wireless extensions.

wmaster0  no wireless extensions.

wlan0     IEEE 802.11abg  ESSID:""  
          Mode:Managed  Frequency:2.412 GHz  Access Point: Not-Associated   
          Tx-Power=15 dBm   
          Retry min limit:7   RTS thr:off   Fragment thr=2352 B   
          Encryption key:off
          Power Management:off
          Link Quality:0  Signal level:0  Noise level:0
          Rx invalid nwid:0  Rx invalid crypt:0  Rx invalid frag:0
          Tx excessive retries:0  Invalid misc:0   Missed beacon:0

pan0      no wireless extensions.

FF submits {"version":"1.1.0","access_token":"2:2lbU2s9YlIv_dacf:3M4mTGwfIDthjsPj","wifi_towers":[{"mac_address":"00-00-00-00-00-00","ssid":"","signal_strength":0}]}

iwconfig when associated:

lo        no wireless extensions.

eth0      no wireless extensions.

irda0     no wireless extensions.

wmaster0  no wireless extensions.

wlan0     IEEE 802.11abg  ESSID:"jootter"  
          Mode:Managed  Frequency:2.462 GHz  Access Point: 00:1D:7E:4C:32:97   
          Bit Rate=54 Mb/s   Tx-Power=15 dBm   
          Retry min limit:7   RTS thr:off   Fragment thr=2352 B   
          Encryption key:8E9A-0E3F-B61F-2474-8556-BC01-C1F1-23FD [3]   Security mode:open
          Power Management:off
          Link Quality=97/100  Signal level:-28 dBm  Noise level=-65 dBm
          Rx invalid nwid:0  Rx invalid crypt:0  Rx invalid frag:0
          Tx excessive retries:0  Invalid misc:0   Missed beacon:0

pan0      no wireless extensions.


When assiciated with the network, FF submits only this one network (there are many networks present) in wifi_towers with correct MAC and empty SSID:
{"version":"1.1.0","access_token":"2:2lbU2s9YlIv_dacf:3M4mTGwfIDthjsPj","wifi_towers":[{"mac_address":"00-1d-7e-4c-32-97","ssid":"","signal_strength":-22}]}

Comment 6

[cmtalbert]

(In reply to comment #3)
> Clint,
> I noticed this behavior before making any changes to files.
> I suppose my case is different from yours - I run FF on a laptop with working
> WiFi adapter, and I have quite a number of networks available, and I expect
> them to be submitted.
> Looking through NetworkGeolocationProvider.js it seems like FF detects the fact
> that there are wireless networks, but fails to list them properly.

That might be why I couldn't reproduce this -- I'm running it on a laptop that's on wireless, but my Ubuntu OS is in a VM.  And that VM thinks that it's bridged connection is a wired one.  :/ Thanks for helping us debug Mikhail.

Comment 7

[Mikhail Khlopotov]

After doing some research and debugging I have discovered:
- the real problem is that libiw has not access to real AP scanning with regular user, only with root
- so FF3.5 running with regular user privileges can at it's best get only one AP mac if the system is associated with that AP.
- there is an initiative at Ubuntu to provide some integral solutions to the case - https://wiki.ubuntu.com/MozillaTeam/Specs/Karmic/FirefoxWifiScanning

So this bug in it's current summary line has no relevance.
Should be renamed to "Firefox 3.5 should support more methods to get AP list than libiw".

Comment 8

[Francis Markham]

NetworkManager supports getting a list of all access-points - but of course you need networkmanager installed to do this.  The attached python script should demonstrate it (works on ubuntu 9.04, but no attempts have been made to make it portable).

Comment 9

[Francis Markham]

Attachment: Demonstration of access point metadata retreval using network manager

Comment 10

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: wip v0.1

The wip use dbus as the python demonstration for getting APs.
For some reasons sending a lot of APs to the geolocation service is not very accurated, but at least, it is possible to send _only_ the best AP in the worst case.

I would like to understand what's wrong with sending many APs?

Comment 11

[Doug Turner (:dougt)]

I think you want to send all access points for the best accuracy.  Are you saying that when you send all APs you get a lower accuracy than when you send just a few?  Do you know if it is a single AP that is causing the degradation?

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Grr, I've forgot to add myself to the CC list!

(In reply to comment #11)
> I think you want to send all access points for the best accuracy.  Are you
> saying that when you send all APs you get a lower accuracy than when you send
> just a few?

Yes, that's what I'm saying but it looks like it depends where I am in the town.

As an example when I try from my home it is less accurate and instead of finding my street it just telling me I'm in Paris (or sometimes in an other Town in France :s).
But when I'm trying from the office, instead of telling me I'm in Copenhagen (not so far from the reality) it give me the exact street in Paris!

> Do you know if it is a single AP that is causing the degradation?

It could be, I'll investigate.

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Also it looks like my wifi driver are bogus, I've some very weird results in my list if access points (some are duplicate but the mac address is one byte different based on the previous result).

I'll try to find another driver but if someone else could test the patch it can be helpful

Comment 14

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #13)
> Also it looks like my wifi driver are bogus, I've some very weird results in my
> list if access points (some are duplicate but the mac address is one byte
> different based on the previous result).
> 
> I'll try to find another driver but if someone else could test the patch it can
> be helpful

It finally works very well today on my computer at home but i'm not sure of what has changed on my system :s
I'll try from the office this afternoon.

Comment 15

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

As an explanation, I don't have any garbage on my dbus results today. So the bad geolocation results where coming from some weird access points results.

Comment 16

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #15)
> As an explanation, I don't have any garbage on my dbus results today. So the
> bad geolocation results where coming from some weird access points results.

No it didn't work again! Only a few hours after.

Comment 17

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #16)
> (In reply to comment #15)
> > As an explanation, I don't have any garbage on my dbus results today. So the
> > bad geolocation results where coming from some weird access points results.
> 
> No it didn't work again! Only a few hours after.

But it works very well from the new Mozilla Europe office while unpatched versions are failing to give the right location.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

One thought on the patch:  even if a build was built with dbus, that doesn't mean that Firefox is running in an environment where NetworkManager is running.  It might be good to put this code into the existing nsWifiScannerUnix, and have it try dbus, and if the dbus code fails, fall back to the existing libiw code.  (That probably means building nsWifiScannerUnix if either dbus or libiw is enabled, and ifdef-ing it appropriately.)

I'm not sure if that's worth it, though.  Are pretty much all Linux users using NetworkManager these days?


Also, have you tried running with the patch under valgrind to see if something is introducing uninitialized data somewhere?

Comment 19

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #18)
> One thought on the patch:  even if a build was built with dbus, that doesn't
> mean that Firefox is running in an environment where NetworkManager is running.
>  It might be good to put this code into the existing nsWifiScannerUnix, and
> have it try dbus, and if the dbus code fails, fall back to the existing libiw
> code.  (That probably means building nsWifiScannerUnix if either dbus or libiw
> is enabled, and ifdef-ing it appropriately.)
> 
> I'm not sure if that's worth it, though.  Are pretty much all Linux users using
> NetworkManager these days?

I'm not sure of what is the right solution here too. I think there is probably as much people having NetworkManager on their box than the one that have libiw. But I can try to find numbers just to be sure and in the worst case merging nsWifiScannerUnix && nsWifiScannerDbus is a solution.

> 
> Also, have you tried running with the patch under valgrind to see if something
> is introducing uninitialized data somewhere?

I have not tried and it was my first thinking too but it looks like everything is set up properly and the data are well receive on the JS side.

Comment 20

[Doug Turner (:dougt)]

Comment on attachment 447910 [details] [diff] [review]
wip v0.1

it isn't clear that DBUS is better to use than the libiw stuff.  In your testing, do you get better results (e.g. multiple APs) with the dbus stuff?

Comment 21

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #20)
> (From update of attachment 447910 [details] [diff] [review])
> it isn't clear that DBUS is better to use than the libiw stuff.  In your
> testing, do you get better results (e.g. multiple APs) with the dbus stuff?

As said on IRC I get better results from 2 places on 3.

And for the worse result from home, I've just launched a test with a Windows machine from home (the place where geolocation is acting randomly) and I have the same results on the Windows as on Linux with DBus: sometimes the geolocation is right, sometimes it positionned me in the wrong city.

I suspect what you suggest at #c11 that one of the AP around is causing that.

Comment 22

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Juts found that there is an AP called PUZZLE (MAC: XX:XX:XX:4F:21:AC) around my home which is registered to the location the geolocation service give back to me.

Comment 23

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

(In reply to comment #22)
> Juts found that there is an AP called PUZZLE (MAC: XX:XX:XX:4F:21:AC) around my
> home which is registered to the location the geolocation service give back to
> me.

What I mean is that this AP is the root cause of the weirdness I'm seeing from my home.

This patch is basically the same as the previous one but fix some crashers on Linux.

Comment 24

[Doug Turner (:dougt)]

Comment on attachment 449028 [details] [diff] [review]
Patch


>+CPPSRCS	+= nsWifiScannerDBus.cpp
>+LOCAL_INCLUDES   += $(MOZ_DBUS_CFLAGS)

Align the +=

>+#define DBUS_INTERFACE_PROPERTIES "org.freedesktop.DBus.Properties"
>+#define DBUS_INTERFACE_DEVICE     "org.freedesktop.NetworkManager.Device"
>+#define DBUS_INTERFACE_WIRELESS "org.freedesktop.NetworkManager.Device.Wireless"

Align the strings.



>+#define NM_802_11_MODE_INFRA      2

Is this defined somewhere?

>+
>+nsresult
>+GetPropertiesFromPath(DBusConnection *connection,
>+                      nsWifiAccessPoint *ap,
>+                      const char *path)
>+{
>+  LOG(("Looking properties for %s\n", path));
>+  DBusMessage *message;
>+  const char* dbus_interface = DBUS_INTERFACE_AP;
>+
>+  DBusError err;
>+  dbus_error_init(&err);
>+
>+  int mode = -1;
>+  const char *dbus_property_mode = "Mode";
>+  message = dbus_message_new_method_call(NM_DBUS_SERVICE, path,
>+                                         DBUS_INTERFACE_PROPERTIES, "Get");
>+
>+  dbus_message_append_args(message,

Please move the variables as close to where there used as possible.  Does this makes it easier to understand where they are needed.  So, please move dbus_interface and dbus_property_mode right abover the append_args call.

>+                           DBUS_TYPE_STRING, &dbus_interface,
>+                           DBUS_TYPE_STRING, &dbus_property_mode,
>+                           DBUS_TYPE_INVALID);
>+  DBusMessage *reply = dbus_connection_send_with_reply_and_block(connection,

Space after the function.

>+                                                                 message,
>+                                                                 -1, &err);
>+  if (dbus_error_is_set(&err)) {
>+    LOG(("Can't get %s from device: %s\n", dbus_property_mode, err.message));
>+    dbus_error_free(&err);
>+    return NS_ERROR_NOT_AVAILABLE;
>+  }
>+
>+  DBusMessageIter iter;
>+  dbus_message_iter_init(reply, &iter);
>+
>+  if (dbus_message_iter_get_arg_type(&iter) == DBUS_TYPE_VARIANT) {
>+    DBusMessageIter subiter;
>+    dbus_message_iter_recurse(&iter, &subiter);
>+    dbus_message_iter_get_basic(&subiter, &mode);
>+  }

I don't understand this bit of code.  why do we need to test for a variant here?  Maybe this call just needs alot more documentation.

>+  dbus_message_append_args(message,
>+                           DBUS_TYPE_STRING, &dbus_interface,
>+                           DBUS_TYPE_STRING, &dbus_property_ssid,
>+                           DBUS_TYPE_INVALID);

space here

>+  reply = dbus_connection_send_with_reply_and_block(connection,
>+                                                    message,
>+                                                    -1, &err);
>+  if (dbus_error_is_set(&err)) {
>+    LOG(("Can't get %s from device: %s\n", dbus_property_ssid, err.message));
>+    dbus_error_free(&err);
>+    return NS_ERROR_NOT_AVAILABLE;
>+  }
>+
>+  DBusMessageIter iter2;
>+  dbus_message_iter_init(reply, &iter2);
>+
>+  if (dbus_message_iter_get_arg_type(&iter2) == DBUS_TYPE_VARIANT) {
>+    DBusMessageIter subiter;
>+    dbus_message_iter_recurse(&iter2, &subiter);
>+
>+    DBusMessageIter subsubiter;
>+    dbus_message_iter_recurse(&subiter, &subsubiter);
>+    dbus_message_iter_get_fixed_array(&subsubiter, &ssid, &ssid_len);
>+  }
>+
>+
>+  const char *hw = NULL;
>+  const char *dbus_property_hw = "HwAddress";
>+  message = dbus_message_new_method_call(NM_DBUS_SERVICE, path,
>+                                         DBUS_INTERFACE_PROPERTIES, "Get");
>+
>+  dbus_message_append_args(message,
>+                           DBUS_TYPE_STRING, &dbus_interface,
>+                           DBUS_TYPE_STRING, &dbus_property_hw,
>+                           DBUS_TYPE_INVALID);

space

>+  /* Print the results */

print?


fix those nits up.  What systems have to verified this on?  Are there any systems that we should consider using iwlib instead of dbus.  I might be happier if we dropped libiw all together if we don't file any problems just using dbus.

Comment 25

[Francis Markham]

I think we may need to keep the libiw implementation as a fallback for users who do not have networkmanager installed.  These may be users on older systems, or who have chosen to install a different connection manager such as wicd (which has its own dbus api).   This probably needs testing on systems with an older networkmanager as well, as I'm not sure how stable to networkmanager dbus API has been.

Comment 26

[Doug Turner (:dougt)]

right now, there is no fall back path in the patch.

Comment 27

[Hanno Schlichting]

Only the dbus based wifi scanner is left. An iwlib based one would commonly (non-root-users) only return a single wifi network. Privacy enhancements on all location services have introduced a minimum requirement of two wifi networks in order to return a wifi-based position estimate. So in most cases an iwlib based scanner wouldn't actually help, as it could only get the single currently connected to wifi network.