Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for newer mobile OS changes. #5529

Merged
merged 6 commits into from
Jul 8, 2019
Merged

Conversation

nouser2013
Copy link
Contributor

Wanted to update the examples with the ones actually working on modern mobile OS. Tested on iOS 12.1, Android 5 and Android 7.0.

Took me quite a while to figure this out, but according to this issue (espressif/arduino-esp32#1037), in order to get a captive notification to show or the popup to open, the DNS server must resolve to a public IP. It will not work with a pivate one (e.g. 192.168.4.1).

On Android, a notification ("Register with Network") is displayed in top left notification bar.
On IOS, the login popup is displayed.
Took me quite a while to figure this out, but according to this issue (espressif/arduino-esp32#1037), in order to get a captive notification to show or the popup to open, the DNS server must resolve to a public IP. It will not work with a pivate one (e.g. 192.168.4.1).

On Android, a notification ("Register with Network") is displayed in top left notification bar.
On IOS, the login popup is displayed.
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the subnet should have no effect on any proper client. Please explain what you're trying to do here.

@d-a-v
Copy link
Collaborator

d-a-v commented Dec 20, 2018

172.217.x.x is not part of a private range (172.16.0.0 .. 172.31.255.255 is, ref).
@nouser2013 can you elaborate on your proposed changes ?

@nouser2013
Copy link
Contributor Author

nouser2013 commented Dec 20, 2018

Hi, gladly. Resolving the captive checks to an IP in a public range apparently triggers the notification to the user correctly, as stated in the referenced comment here.

Edit: specific post is this one.

Therefore, I'd propose to adapt the changes also to these examples in order to save future developers the headache of debugging the captive portal examples not functioning on Android.

@d-a-v
Copy link
Collaborator

d-a-v commented Dec 21, 2018

I tried with android 7.1.2(LineageOS), and these sketches are working with 192.168.4.1 and data plan disabled on mobile. Notification to log in appears. (later: this test was working maybe by chance, see below).

We are opened for improvements, but setting a random public IP address instead of an official private one (192.168.x.x, 10.x.x.x, 172.16.x.x..172.16.31.x.x) is not a good idea.

At first I was thinking that it might be related with complexity of URL handling in modern browsers because of unification into a single user input field of 1) URL, 2) searchbar 3) cache, and 4) general simplification of the so-called "user experience" leading to hidden-maybe-unwise assumptions.

Then I tried with a laptop (doing lots and lots of dns requests):

  • captiveportal example sadly dies with WDT from within a too long loop
  • with WiFiManager - which using DNSServer from this core - I observed that sometimes captive-dns works, sometimes not.
  • more importantly, and sadly, the dig tool reports errors: Warning: Message parser reports malformed message packet

Aside from this, there are some weirdnesses in our DNSServer.

We must solve this DNSServer issue first, and check afterwards whether the issue from which this PR originates is still valid.

@devyte @LaborEtArs I read somewhere that mDNS and DNS have the same packet format. Our DNSServer is not meant to be a DNS forwarder so both are similar regarding pure-local-only-DNS functionality. Is there any chance that we could include local-only legacy DNS server from a subset of mDNS code ?

@tablatronix what are your thinkings ?

@tablatronix
Copy link
Contributor

I agree with @d-a-v , noting this as a workaround might be useful, maybe as a commented out note about the address space restrictions,

I have not personally observed the captive portal issues as I do not use android, but using a rando non class C ip is not a good idea officially, considered a kludge.

@nouser2013
Copy link
Contributor Author

nouser2013 commented Dec 21, 2018

[...] but using a random non class C ip is not a good idea officially, considered a kludge.

Out of curiosity, could you elaborate? E.g., is there a standardized captive-portal behaviour I've overlooked?

As to why it seems to work on (my) 7.0, 5.0 and 4.3 devices with the public IP only, I cannot even speculate. It shouldn't make a difference.

@tablatronix
Copy link
Contributor

I think this is still being discussed in that other post, it seems to be some kind of security restriction in some versions of android, or maybe a setting, I don't think anyone has figured it out yet, as we could not find specific definitive docs on it. It should work.

@LaborEtArs
Copy link
Contributor

@d-a-v Your right, mDNS uses the DNS protocol. However the implemented logic is quite different... so, to get a DNS server would be a lot of additional work...

MisterD66 added a commit to MisterD66/Arduino that referenced this pull request Jun 1, 2019
Android phones have a operatin system feature that will display a captive portal shortly after connection to a wifi that requires a log in. This is only triggered if you dont use an adress in the private range!

This is a new request based on
 esp8266#5529
and espressif/arduino-esp32#1037

it seems that Android phones will not display the autoredict page if it is in the private IP Range.

I found this fix in the two issues above and tried it with several android phones, non of them displayed it bevore:
Xiaomi Redmi Note 7 (miui 10.3 Android 7) 
Samsung Galaxy A7  (Android 8.0.0) 
Amazon Kindle Fire 7 (Fire OS 5.6.4.0 Android 5.1)

I think for easy access to newcomers this should be changed so it works instantly!
@Misiu
Copy link

Misiu commented Jul 1, 2019

any updates on this? I've seen multiple places mentioning that Captive Portal doesn't work on some devices.
tripflex/wifi-captive-portal#7
Hieromon/AutoConnect#85
https://github.com/tripflex/captive-portal/blob/dev/src/mgos_captive_portal.c#L368-L380

https://github.com/tripflex/captive-portal#known-endpoints

DNSServer got a rework so maybe this can be finally fixed?

@d-a-v
Copy link
Collaborator

d-a-v commented Jul 1, 2019

Given the links above, and also this one, and if the examples work better, I tend to think we can merge it if we can add documentation (at least a ref to this issue) in these changes.

@nouser2013, or @MisterD66 from #6173, can you do that ?

@d-a-v d-a-v mentioned this pull request Jul 1, 2019
@d-a-v d-a-v requested a review from earlephilhower July 1, 2019 10:57
@d-a-v
Copy link
Collaborator

d-a-v commented Jul 8, 2019

@nouser2013 and @MisterD66 thank you for bringing these fixes.

That helped understanding and solving other issues (I finally experimented it too).
I'm going to merge this one (two fixes) and I'll add comments in a further one.

ref: https://issuetracker.google.com/issues/36932514
ref: https://bugs.chromium.org/p/chromium/issues/detail?id=405925

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants