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

Update protocol to https for DEFAULT_HOST in WalledGardenInternetObservingStrategy #323

Closed
pwittchen opened this issue Feb 13, 2019 · 9 comments

Comments

@pwittchen
Copy link
Owner

commented Feb 13, 2019

See issue mentioned here: #299 (comment)

Line in the code:

It needs to be verified and tested before pushing.

@pwittchen pwittchen changed the title Update protocol to https in WalledGardenInternetObservingStrategy Update protocol to https for DEFAULT_HOST in WalledGardenInternetObservingStrategy Feb 13, 2019

@MatushkaRossiya

This comment has been minimized.

Copy link

commented Apr 9, 2019

Hi, is there any update on this?

@pwittchen

This comment has been minimized.

Copy link
Owner Author

commented Apr 9, 2019

Hi,

I didn't have time to work on this. I'll try to fix it as soon as I can.

Regards.

@thevoiceless

This comment has been minimized.

Copy link

commented Jun 10, 2019

FYI, this seems to work as expected (note that we must also set the port):

val settings = InternetObservingSettings.builder()
        .host("https://clients3.google.com/generate_204")
        .port(443)
        .build()

ReactiveNetwork.observeNetworkConnectivity(applicationContext)
        .flatMapSingle { ReactiveNetwork.checkInternetConnectivity(settings) }
        .subscribe(...)
@pwittchen

This comment has been minimized.

Copy link
Owner Author

commented Aug 6, 2019

For some reason, I get strange mockito errors in unit tests when I change default protocol from http, to https. I have to look at it.

@pwittchen

This comment has been minimized.

Copy link
Owner Author

commented Aug 6, 2019

Ok, I probably figured this out. I had to replace ErrorHandler mocks with stubs in a few unit tests.

pwittchen added a commit that referenced this issue Aug 6, 2019

@pwittchen

This comment has been minimized.

Copy link
Owner Author

commented Aug 6, 2019

Fixed in PR #361.

@georgejas

This comment has been minimized.

Copy link

commented Aug 15, 2019

@pwittchen
Thanks for this library!

I just upgraded to 3.0.4 to try and get rid of the error: ReactiveNetwork: Could not establish connection with WalledGardenStrategy
java.io.IOException: Cleartext HTTP traffic to clients3.google.com not permitted

I see your 3.0.4 change, but it is still not working with compileSdkVersion = 28

I think the issue is with createHttpUrlConnection - I changed HttpURLConnection types to HttpsURLConnection type, like this, and it worked:

    protected HttpsURLConnection createHttpUrlConnection(final String host, final int port,
                                                                       final int timeoutInMs) throws IOException {
        URL initialUrl = new URL(host);
        URL url = new URL(initialUrl.getProtocol(), initialUrl.getHost(), port, initialUrl.getFile());
        HttpsURLConnection urlConnection = (HttpsURLConnection) url.openConnection();
        urlConnection.setConnectTimeout(timeoutInMs);
        urlConnection.setReadTimeout(timeoutInMs);
        urlConnection.setInstanceFollowRedirects(false);
        urlConnection.setUseCaches(false);
        return urlConnection;
    }

Could you please make this update? Thanks!!!

@pwittchen

This comment has been minimized.

Copy link
Owner Author

commented Aug 15, 2019

@pwittchen pwittchen reopened this Aug 15, 2019

@pwittchen pwittchen pinned this issue Aug 15, 2019

pwittchen added a commit that referenced this issue Aug 19, 2019

adding new method for creating HttpsUrlConnection in
WalledGardenInternetObservingStrategy - solves #323
@pwittchen

This comment has been minimized.

Copy link
Owner Author

commented Aug 19, 2019

@georgejas this update will be available in the next release. HttpsURLConnection instance will be created in the case of using https protocol.

@pwittchen pwittchen closed this Aug 19, 2019

@pwittchen pwittchen unpinned this issue Aug 19, 2019

@pwittchen pwittchen referenced this issue Aug 20, 2019
3 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.