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

[TIMOB-26377] Android: Allow non-https requests to work when targeting Android P #10319

Merged
merged 2 commits into from Sep 12, 2018

Conversation

jquick-axway
Copy link
Contributor

JIRA:
https://jira.appcelerator.org/browse/TIMOB-26377

Summary:

  • Modified "AndroidManifest.xml" template to set <application/> attribute "android:usesCleartextTraffic" to true for backward compatibility.
  • Note that you can modify "tiapp.xml" and set this attribute to false.
  • Our existing unit tests already covers "http" requests, but are not failing because we're not targeting Android P by default yet.

WebView Test:

  1. Set up the "tiapp.xml" as shown below. It must target API Level 28. (Leave the <application/> tag commented out.)
  2. Acquire an Android P device with Internet access.
  3. Build and run the below code on the device.
  4. Verify that the app displays Google website: https://www.google.com
  5. Uncomment the <application/> tag from the "tiapp.xml". (Note that setting usesCleartextTraffic to false blocks "http" requests.)
  6. Build and run on the same Android device.
  7. Verify that WebView fails to load page and shows a "Webpage not available" error message. (This is good because it's honoring the manifest setting.)

tiapp.xml

<ti:app>
	<android xmlns:android="http://schemas.android.com/apk/res/android">
		<manifest>
			<uses-sdk android:minSdkVersion="16" android:targetSdkVersion="28"/>
<!--
			<application android:usesCleartextTraffic="false"/>
-->
		</manifest>
	</android>
</ti:app>

app.js

var window = Ti.UI.createWindow();
window.add(Ti.UI.createWebView({ url: "http://www.google.com" }));
window.open();

HTTPClient Test:

  1. Set up "tiapp.xml" to target API Level 28 as shown below.
  2. Use the test code "HttpGetTest.js" attached to TIMOB26377.
  3. Build and run on an Android P device having Internet access.
  4. Verify that you do not get a "Cleartext HTTP traffic to <URL> not permitted" response onscreen. Instead, you see the source of https://www.google.com.
<ti:app>
	<android xmlns:android="http://schemas.android.com/apk/res/android">
		<manifest>
			<uses-sdk android:minSdkVersion="16" android:targetSdkVersion="28"/>
		</manifest>
	</android>
</ti:app>

…g Android P

- Modified "AndroidManifest.xml" template to set <application/> attribute "android:usesCleartextTraffic" to true for backward compatibility.
Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

CR: PASS

This looks good, but what if someone updates an existing tiapp.xml to target Android P? Maybe we should also update the build scripts to insert this attribute into the tiapp.xml?

@jquick-axway
Copy link
Contributor Author

This looks good, but what if someone updates an existing tiapp.xml to target Android P? Maybe we should also update the build scripts to insert this attribute into the tiapp.xml?

This change will always insert the usesCleartextTraffic attribute when you build with Titanium 7.5.0, whether you want it or not. And it'll inject it regardless of which API Level you are targeting. It'll work for existing Titanium projects and new projects.

Also, the usesCleartextTraffic attribute has existed since API Level 23. It used to default to true if not defined in the "AndroidManifest.xml". The breaking change on Google's end is that it will default to false when targeting Android P. So, my change maintains backward compatibility, but also allows devs to opt-in to setting it to false as well.

@hansemannn
Copy link
Collaborator

@jquick-axway Is it overridable? For example, if an (enterprise) customer wants to explicitly disallow http connections.

@jquick-axway
Copy link
Contributor Author

Is it overridable?

Yes. In fact, that's one of the tests above. :)

@build
Copy link
Contributor

build commented Sep 12, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@lokeshchdhry
Copy link
Contributor

FR Passed.

  1. Having android:usesCleartextTraffic="false" blocks http on android P & older devices.
  2. Having android:usesCleartextTraffic="true" does not block http on android P & older devices.

Studio Ver: 5.1.1.201809051655
SDK Ver: 7.5.0.v20180906093938
OS Ver: 10.13.5
Xcode Ver: Xcode 9.4.1
Appc NPM: 4.2.13
Appc CLI: 7.0.6
Daemon Ver: 1.1.3
Ti CLI Ver: 5.1.1
Alloy Ver: 1.13.2
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 10.0.2
Devices: ⇨ google Nexus 5 (Android 6.0.1)
⇨ google Pixel (Android 9)
Emulator: Android 4.1

@lokeshchdhry lokeshchdhry merged commit c71e5a4 into tidev:master Sep 12, 2018
@jquick-axway
Copy link
Contributor Author

Just to be clear, the android:usesCleartextTraffic="false" setting is only respected by:

  • HTTPClient on Android 6.0 and higher
  • WebView on Android 8.0 and higher

The above are Android OS limitations on Google's end. See...
https://developer.android.com/guide/topics/manifest/application-element#usesCleartextTraffic

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

Successfully merging this pull request may close these issues.

None yet

5 participants