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

[Bug] [Android] not following redirects for http/https URLs since version 3 #383

Open
ath0mas opened this issue Nov 20, 2020 · 3 comments · May be fixed by #396
Open

[Bug] [Android] not following redirects for http/https URLs since version 3 #383

ath0mas opened this issue Nov 20, 2020 · 3 comments · May be fixed by #396

Comments

@ath0mas
Copy link
Contributor

ath0mas commented Nov 20, 2020

Describe the bug
Some URLs with redirects between http/https were properly resolved in success with plugin version 2.5.1, but are now resolved in error with status 301/302 since 3.0 up to current 3.1.0.

System info

  • affected HTTP plugin version: 3.0.0 to 3.1.0
  • affected platform(s) and version(s): Android
  • affected device(s): devices and emulators
  • cordova version: cordova 10
  • cordova platform version(s): cordova-android 9

Are you using ionic-native-wrapper?
No

Minimum viable code to reproduce
Seems linked to a requested url on http redirecting to other location and to https - while result is ok if request on https with redirect.

Example: http://foobar.test.com/ gives a 302 to https://www.test.com

cordova.plugin.http.get('http://foobar.test.com/', null, null, console.info, console.error);

with config.xml

<edit-config file="AndroidManifest.xml" target="/manifest/application" mode="merge">
    <application android:usesCleartextTraffic="true" />
</edit-config>

Maybe skipped redirect specs #344 can detect this?

@ath0mas ath0mas changed the title [Bug] [Android] not following redirects for some URLs since version 3 [Bug] [Android] not following redirects for http/https URLs since version 3 Nov 20, 2020
@ath0mas
Copy link
Contributor Author

ath0mas commented Nov 20, 2020

Just confirmed the http/https fact.

For these tests I used httpbingo.org/redirect-to and mixing http and https between base and redirect URLs,

from https://httpbingo.org/redirect-to?url=https%3A%2F%2Fhttpbingo.org&status_code=302

\ 2.5.1 3.0.0 3.1.0 ( 3.1.0 iOS )
http → http ✔️ ✔️ ✔️ ✔️
https → https ✔️ ✔️ ✔️ ✔️
http → https ✔️ ✔️
https → http ✔️ ✔️

✔️ success, status 200
❌ error, status 302

(same results with &status_code=301 instead of 302)

@ath0mas
Copy link
Contributor Author

ath0mas commented Jan 12, 2021

Android code mostly in HttpRequest seems to rely on java.net.HttpURLConnection toggling redirect through setInstanceFollowRedirects.

Quick search about http/https redirection links to posts like here in SO and most of all to Android HttpURLConnection Response Handling :

This implementation doesn't follow redirects from HTTPS to HTTP or vice versa.

And how okhttp does it for example: RetryAndFollowUpInterceptor#buildRedirectRequest(), with config followSslRedirects
Or something like this? or like that?

ath0mas added a commit to ath0mas/cordova-plugin-advanced-http that referenced this issue Jan 17, 2021
spec is commented because failing, as detected in issue silkimen#383
ath0mas added a commit to ath0mas/cordova-plugin-advanced-http that referenced this issue Jan 17, 2021
spec is commented because failing, as detected in issue silkimen#383
@ath0mas ath0mas linked a pull request Jan 17, 2021 that will close this issue
ath0mas added a commit to ath0mas/cordova-plugin-advanced-http that referenced this issue Jan 18, 2021
@ath0mas
Copy link
Contributor Author

ath0mas commented Jan 18, 2021

New spec for this issue is ready in PR #396.

Help welcome on anything to try, possible fix, or contribution please

ath0mas added a commit to ath0mas/cordova-plugin-advanced-http that referenced this issue Jan 20, 2021


when followRedirects=true and still receiving a redirect response, do manual recursion using "Location" header as the new url
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 a pull request may close this issue.

1 participant