From d4774290d5aad1e44dd883aec99e069f5eb06a97 Mon Sep 17 00:00:00 2001 From: Petter Hesselberg Date: Fri, 6 Sep 2019 17:30:27 +0200 Subject: [PATCH 1/4] Initial implementation of safe handling of Parcelable arrays --- .../android/app/src/main/AndroidManifest.xml | 2 + .../react/uiapp/RNTesterActivity.java | 53 +++++++++++++++---- .../com/facebook/react/bridge/Arguments.java | 10 ++++ 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/RNTester/android/app/src/main/AndroidManifest.xml b/RNTester/android/app/src/main/AndroidManifest.xml index d520a547e7a882..57e13004d2c384 100644 --- a/RNTester/android/app/src/main/AndroidManifest.xml +++ b/RNTester/android/app/src/main/AndroidManifest.xml @@ -48,6 +48,8 @@ + + diff --git a/RNTester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java b/RNTester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java index 4facf807dbc55b..523b88a2629845 100644 --- a/RNTester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java +++ b/RNTester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java @@ -6,8 +6,13 @@ */ package com.facebook.react.uiapp; +import android.content.Intent; import android.content.res.Configuration; +import android.net.Uri; import android.os.Bundle; +import android.os.Parcel; +import android.os.Parcelable; + import androidx.annotation.Nullable; import com.facebook.react.ReactActivity; import com.facebook.react.ReactActivityDelegate; @@ -16,6 +21,7 @@ public class RNTesterActivity extends ReactActivity { public static class RNTesterActivityDelegate extends ReactActivityDelegate { private static final String PARAM_ROUTE = "route"; + private static final String PARAM_BUNDLE_TEST = "bundle-test"; private Bundle mInitialProps = null; private final @Nullable ReactActivity mActivity; @@ -27,16 +33,45 @@ public RNTesterActivityDelegate(ReactActivity activity, String mainComponentName @Override protected void onCreate(Bundle savedInstanceState) { // Get remote param before calling super which uses it - Bundle bundle = mActivity.getIntent().getExtras(); - if (bundle != null && bundle.containsKey(PARAM_ROUTE)) { - String routeUri = - new StringBuilder("rntester://example/") - .append(bundle.getString(PARAM_ROUTE)) - .append("Example") - .toString(); - mInitialProps = new Bundle(); - mInitialProps.putString("exampleFromAppetizeParams", routeUri); + Intent intent = mActivity.getIntent(); + if (Intent.ACTION_VIEW.equals(intent.getAction())) { + Uri data = intent.getData(); + if (data != null) { + intent.setData(null); // Reset so that we only take action once. + mInitialProps = new Bundle(); + String host = data.getHost(); + if ("example".equals(host)) { + String routeUri = + "rntester://example/" + + data.getQueryParameter(PARAM_ROUTE) + + "Example"; + mInitialProps.putString("exampleFromAppetizeParams", routeUri); + } else if ("bundle-test".equals(host)) { + Bundle[] children = new Bundle[2]; + children[0] = new Bundle(); + children[1] = new Bundle(); + children[0].putString("exampleChildKey", "exampleChild1"); + children[1].putString("exampleChildKey", "exampleChild2"); + mInitialProps.putSerializable("bundle", children); + Parcel parcel = null; + byte[] bytes = null; + + try { + parcel = Parcel.obtain(); + mInitialProps.writeToParcel(parcel, 0); + bytes = parcel.marshall(); + parcel.unmarshall(bytes, 0, bytes.length); + parcel.setDataPosition(0); + mInitialProps = Bundle.CREATOR.createFromParcel(parcel); + } finally { + if (parcel != null) { + parcel.recycle(); + } + } + } + } } + super.onCreate(savedInstanceState); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/Arguments.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/Arguments.java index e17e3c1aca1166..9d4dc86014a88b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/Arguments.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/Arguments.java @@ -7,6 +7,8 @@ package com.facebook.react.bridge; import android.os.Bundle; +import android.os.Parcelable; + import androidx.annotation.Nullable; import java.lang.reflect.Array; import java.util.AbstractList; @@ -218,6 +220,14 @@ public static WritableArray fromArray(Object array) { for (boolean v : (boolean[]) array) { catalystArray.pushBoolean(v); } + } else if (array instanceof Parcelable[]) { + for (Parcelable v : (Parcelable[]) array) { + if (v instanceof Bundle) { + catalystArray.pushMap(fromBundle((Bundle) v)); + } else { + throw new IllegalArgumentException("Unexpected array member type " + v.getClass()); + } + } } else { throw new IllegalArgumentException("Unknown array type " + array.getClass()); } From ddb78d10b2301071f1952a6dbc508cd738a10e41 Mon Sep 17 00:00:00 2001 From: Petter Hesselberg Date: Mon, 9 Sep 2019 08:09:04 +0200 Subject: [PATCH 2/4] Add ArgumentsTest --- .../facebook/react/bridge/ArgumentsTest.java | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 ReactAndroid/src/androidTest/java/com/facebook/react/bridge/ArgumentsTest.java diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/bridge/ArgumentsTest.java b/ReactAndroid/src/androidTest/java/com/facebook/react/bridge/ArgumentsTest.java new file mode 100644 index 00000000000000..27fde09a84108b --- /dev/null +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/bridge/ArgumentsTest.java @@ -0,0 +1,82 @@ +package com.facebook.react.bridge; + +import android.content.Context; +import android.os.Bundle; +import android.os.Parcel; + +import androidx.annotation.NonNull; +import androidx.test.platform.app.InstrumentationRegistry; +import androidx.test.runner.AndroidJUnit4; + +import com.facebook.soloader.SoLoader; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static com.facebook.react.bridge.Arguments.fromBundle; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +@RunWith(AndroidJUnit4.class) +public class ArgumentsTest { + + @Before + public void setUp() { + Context context = InstrumentationRegistry.getInstrumentation().getTargetContext(); + SoLoader.init(context, false); + } + + @Test + public void testFromBundle() { + verifyBundle(createBundle()); + } + + @Test + public void testFromMarshaledBundle() { + verifyBundle(marshalAndUnmarshalBundle(createBundle())); + } + + private void verifyBundle(@NonNull Bundle bundle) { + WritableMap map = fromBundle(bundle); + assertNotNull(map); + + assertEquals(ReadableType.Array, map.getType("children")); + ReadableArray children = map.getArray("children"); + assertNotNull(children); + assertEquals(1, children.size()); + + assertEquals(ReadableType.Map, children.getType(0)); + ReadableMap child = children.getMap(0); + assertNotNull(child); + assertEquals("exampleChild", child.getString("exampleChildKey")); + } + + @NonNull + private Bundle marshalAndUnmarshalBundle(@NonNull Bundle bundle) { + Parcel parcel = null; + try { + parcel = Parcel.obtain(); + bundle.writeToParcel(parcel, 0); + + byte[] bytes = parcel.marshall(); + parcel.unmarshall(bytes, 0, bytes.length); + parcel.setDataPosition(0); + return Bundle.CREATOR.createFromParcel(parcel); + } finally { + if (parcel != null) { + parcel.recycle(); + } + } + } + + @NonNull + private Bundle createBundle() { + Bundle bundle = new Bundle(); + Bundle[] children = new Bundle[1]; + children[0] = new Bundle(); + children[0].putString("exampleChildKey", "exampleChild"); + bundle.putSerializable("children", children); + return bundle; + } +} From 681c71b8a508bed49339bdda541d2670f9fce23f Mon Sep 17 00:00:00 2001 From: Petter Hesselberg Date: Mon, 9 Sep 2019 08:14:04 +0200 Subject: [PATCH 3/4] Revert RNTesterActivity changes --- .../android/app/src/main/AndroidManifest.xml | 2 - .../react/uiapp/RNTesterActivity.java | 53 ++++--------------- 2 files changed, 9 insertions(+), 46 deletions(-) diff --git a/RNTester/android/app/src/main/AndroidManifest.xml b/RNTester/android/app/src/main/AndroidManifest.xml index 57e13004d2c384..d520a547e7a882 100644 --- a/RNTester/android/app/src/main/AndroidManifest.xml +++ b/RNTester/android/app/src/main/AndroidManifest.xml @@ -48,8 +48,6 @@ - - diff --git a/RNTester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java b/RNTester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java index 523b88a2629845..4facf807dbc55b 100644 --- a/RNTester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java +++ b/RNTester/android/app/src/main/java/com/facebook/react/uiapp/RNTesterActivity.java @@ -6,13 +6,8 @@ */ package com.facebook.react.uiapp; -import android.content.Intent; import android.content.res.Configuration; -import android.net.Uri; import android.os.Bundle; -import android.os.Parcel; -import android.os.Parcelable; - import androidx.annotation.Nullable; import com.facebook.react.ReactActivity; import com.facebook.react.ReactActivityDelegate; @@ -21,7 +16,6 @@ public class RNTesterActivity extends ReactActivity { public static class RNTesterActivityDelegate extends ReactActivityDelegate { private static final String PARAM_ROUTE = "route"; - private static final String PARAM_BUNDLE_TEST = "bundle-test"; private Bundle mInitialProps = null; private final @Nullable ReactActivity mActivity; @@ -33,45 +27,16 @@ public RNTesterActivityDelegate(ReactActivity activity, String mainComponentName @Override protected void onCreate(Bundle savedInstanceState) { // Get remote param before calling super which uses it - Intent intent = mActivity.getIntent(); - if (Intent.ACTION_VIEW.equals(intent.getAction())) { - Uri data = intent.getData(); - if (data != null) { - intent.setData(null); // Reset so that we only take action once. - mInitialProps = new Bundle(); - String host = data.getHost(); - if ("example".equals(host)) { - String routeUri = - "rntester://example/" + - data.getQueryParameter(PARAM_ROUTE) + - "Example"; - mInitialProps.putString("exampleFromAppetizeParams", routeUri); - } else if ("bundle-test".equals(host)) { - Bundle[] children = new Bundle[2]; - children[0] = new Bundle(); - children[1] = new Bundle(); - children[0].putString("exampleChildKey", "exampleChild1"); - children[1].putString("exampleChildKey", "exampleChild2"); - mInitialProps.putSerializable("bundle", children); - Parcel parcel = null; - byte[] bytes = null; - - try { - parcel = Parcel.obtain(); - mInitialProps.writeToParcel(parcel, 0); - bytes = parcel.marshall(); - parcel.unmarshall(bytes, 0, bytes.length); - parcel.setDataPosition(0); - mInitialProps = Bundle.CREATOR.createFromParcel(parcel); - } finally { - if (parcel != null) { - parcel.recycle(); - } - } - } - } + Bundle bundle = mActivity.getIntent().getExtras(); + if (bundle != null && bundle.containsKey(PARAM_ROUTE)) { + String routeUri = + new StringBuilder("rntester://example/") + .append(bundle.getString(PARAM_ROUTE)) + .append("Example") + .toString(); + mInitialProps = new Bundle(); + mInitialProps.putString("exampleFromAppetizeParams", routeUri); } - super.onCreate(savedInstanceState); } From 17a1be52551c96578bc2c07a315a0fbcee4dd61d Mon Sep 17 00:00:00 2001 From: Petter Hesselberg Date: Mon, 9 Sep 2019 08:21:26 +0200 Subject: [PATCH 4/4] Add test comment --- .../java/com/facebook/react/bridge/ArgumentsTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/bridge/ArgumentsTest.java b/ReactAndroid/src/androidTest/java/com/facebook/react/bridge/ArgumentsTest.java index 27fde09a84108b..bd0932b6304a9f 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/bridge/ArgumentsTest.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/bridge/ArgumentsTest.java @@ -1,8 +1,10 @@ package com.facebook.react.bridge; import android.content.Context; +import android.content.Intent; import android.os.Bundle; import android.os.Parcel; +import android.os.Parcelable; import androidx.annotation.NonNull; import androidx.test.platform.app.InstrumentationRegistry; @@ -32,6 +34,11 @@ public void testFromBundle() { verifyBundle(createBundle()); } + /** + * When passing a bundle via {@link Intent} extras, it gets parceled and unparceled. + * Any array of bundles will return as an array of {@link Parcelable} instead. This test + * verifies that {@link Arguments#fromArray} handles this situation correctly. + */ @Test public void testFromMarshaledBundle() { verifyBundle(marshalAndUnmarshalBundle(createBundle()));