[Replicant] [PATCH v2] Fix "RepWifi connection not correctly reported to userspace apps" Issue #1792.

[Joonas Kylmälä]

Fil:
> Joonas Kylmälä wrote:
>> It is common practice to define them at top of the file. Also the AOSP
>> guidelines have it:
>> <https://source.android.com/source/code-style#define-fields-in-standard-places>.
>> Well, now you might say that that is no reason but the thing is that
>> people are expecting them to be there and it makes it easier to read the
>> code for them if the things are in the familiar places.
> I didn't say there's no reason to put those declaration at the top of
> the file. I just said I have *my* reason to keep them where they are,
> but I could move them if some other *strong* reason is given.
> 
> Anyway, I read those guidelines, and I tried to be as compliant as
> possible. What they actually say is: "Define fields either at the top of
> the file *or immediately before the methods that use them*." And that's
> what I did.

I'm not talking about the method below the declarations, but instead
about the code that is above the declarations:

+
+        // TODO RepWifi extension. Remove when a low-level, long-lived
solution is found.
+        if (ni == null) {
+            ni = getNetworkInfoByInterfaceName(IFACE_NAME_WIFI);
+        }
+        if (ni == null) {
+            ni = getNetworkInfoByInterfaceName(IFACE_NAME_TETHER);
+        }
+


>> No, I'm pretty sure my university network gives public ip addresses for
>> everyone connecting to their wifi.
> I'm pretty sure you should double check on that one. Please, see [1].

I tested it now. This is the output I get at the moment from ifconfig
while connected with wifi:

wlp4s0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet6 2001:14ba:8023:4c00:85a2:186a:4075:4e49  prefixlen 64
scopeid 0x0<global>
        inet6 fe80::e5fb:91cc:9fe2:7c44  prefixlen 64  scopeid 0x20<link>
        ether 44:85:00:d3:c4:58  txqueuelen 1000  (Ethernet)
        RX packets 3628258  bytes 4455552923 (4.1 GiB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 1647787  bytes 244108200 (232.7 MiB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0

So it gives global i.e. public ipv6 address.

>>> The reason why I put the additional condition that interface's address
>>> must be in the "site local" range, is that it tries to enforce the fact
>>> that WiFi negotiation hasn't failed. 
>>> It's an effort to be "less uncertain" about the interface being
>>> *actually* connected to WiFi and try to avoid "false positives".
>>> For example, if something in the negotiation goes wrong, wlan0 could
>>> still have an IP address. Maybe it could be in the "link local" range.
>>> That condition cuts away at least that case, reducing the likelihood of
>>> reporting "false" connections.
>>
>> I don't know what to do about this.
> 
> I honestly don't think anything else needs to be done on that side.. 
> Or.. well.. a DNS query or a ping could be issued to be sure the
> connection is real. But there are a few downsides with that approach:
> 1) it would cause way too much overhead and may make calling
> applications lag for too long or even crash if they think their request
> to getActiveNetworkInfo() has timed out (I've experienced this behaviour
> while debugging my patch).
> 2) hard-coded IP addresses should be included as DNS or ping targets. No
> good.
> 3) I'm quite sure that the original method itself doesn't perform any
> practical check on the reachability of the internet. It just reports
> what kind of connection the device is currently using (namely, data or
> wifi). And I don't want to change the original scope of the function so
> much.
> 

We could check how the AOSP handles this problem when the wifi is
properly integrated with the OS. This is not so much on topic but could
get one started: <https://community.nxp.com/docs/DOC-93603>.

Joonas