Merge "Parcel only the canonical Uri.Part representation, not both." into qt-qpr1-dev

core/java/android/net/Uri.java

@@ -1987,17 +1987,26 @@ public abstract class Uri implements Parcelable, Comparable<Uri> {
          * Enum which indicates which representation of a given part we have.
          */
         static class Representation {
-            static final int BOTH = 0;
             static final int ENCODED = 1;
             static final int DECODED = 2;
         }
 
         volatile String encoded;
         volatile String decoded;
+        private final int mCanonicalRepresentation;
 
         AbstractPart(String encoded, String decoded) {
+            if (encoded != NOT_CACHED) {
+                this.mCanonicalRepresentation = Representation.ENCODED;
                 this.encoded = encoded;
+                this.decoded = NOT_CACHED;
+            } else if (decoded != NOT_CACHED) {
+                this.mCanonicalRepresentation = Representation.DECODED;
+                this.encoded = NOT_CACHED;
                 this.decoded = decoded;
+            } else {
+                throw new IllegalArgumentException("Neither encoded nor decoded");
+            }
         }
 
         abstract String getEncoded();
@@ -2009,25 +2018,21 @@ public abstract class Uri implements Parcelable, Comparable<Uri> {
         }
 
         final void writeTo(Parcel parcel) {
-            @SuppressWarnings("StringEquality")
+            final String canonicalValue;
-            boolean hasEncoded = encoded != NOT_CACHED;
+            if (mCanonicalRepresentation == Representation.ENCODED) {
-
+                canonicalValue = encoded;
-            @SuppressWarnings("StringEquality")
+            } else if (mCanonicalRepresentation == Representation.DECODED) {
-            boolean hasDecoded = decoded != NOT_CACHED;
+                canonicalValue = decoded;
-
-            if (hasEncoded && hasDecoded) {
-                parcel.writeInt(Representation.BOTH);
-                parcel.writeString(encoded);
-                parcel.writeString(decoded);
-            } else if (hasEncoded) {
-                parcel.writeInt(Representation.ENCODED);
-                parcel.writeString(encoded);
-            } else if (hasDecoded) {
-                parcel.writeInt(Representation.DECODED);
-                parcel.writeString(decoded);
             } else {
-                throw new IllegalArgumentException("Neither encoded nor decoded");
+                throw new IllegalArgumentException("Unknown representation: "
+                    + mCanonicalRepresentation);
             }
+            if (canonicalValue == NOT_CACHED) {
+                throw new AssertionError("Canonical value not cached ("
+                    + mCanonicalRepresentation + ")");
+            }
+            parcel.writeInt(mCanonicalRepresentation);
+            parcel.writeString(canonicalValue);
         }
     }
 
@@ -2059,13 +2064,12 @@ public abstract class Uri implements Parcelable, Comparable<Uri> {
 
         static Part readFrom(Parcel parcel) {
             int representation = parcel.readInt();
+            String value = parcel.readString();
             switch (representation) {
-                case Representation.BOTH:
-                    return from(parcel.readString(), parcel.readString());
                 case Representation.ENCODED:
-                    return fromEncoded(parcel.readString());
+                    return fromEncoded(value);
                 case Representation.DECODED:
-                    return fromDecoded(parcel.readString());
+                    return fromDecoded(value);
                 default:
                     throw new IllegalArgumentException("Unknown representation: "
                             + representation);
@@ -2127,6 +2131,11 @@ public abstract class Uri implements Parcelable, Comparable<Uri> {
         private static class EmptyPart extends Part {
             public EmptyPart(String value) {
                 super(value, value);
+                if (value != null && !value.isEmpty()) {
+                    throw new IllegalArgumentException("Expected empty value, got: " + value);
+                }
+                // Avoid having to re-calculate the non-canonical value.
+                encoded = decoded = value;
             }
 
             @Override
@@ -2245,14 +2254,12 @@ public abstract class Uri implements Parcelable, Comparable<Uri> {
         static PathPart readFrom(Parcel parcel) {
             int representation = parcel.readInt();
             switch (representation) {
-                case Representation.BOTH:
-                    return from(parcel.readString(), parcel.readString());
                 case Representation.ENCODED:
                     return fromEncoded(parcel.readString());
                 case Representation.DECODED:
                     return fromDecoded(parcel.readString());
                 default:
-                    throw new IllegalArgumentException("Bad representation: " + representation);
+                    throw new IllegalArgumentException("Unknown representation: " + representation);
             }
         }
 

core/tests/coretests/src/android/net/UriTest.java

@@ -24,6 +24,9 @@ import androidx.test.filters.SmallTest;
 import junit.framework.TestCase;
 
 import java.io.File;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
@@ -816,6 +819,63 @@ public class UriTest extends TestCase {
                 Uri.parse("content://com.example/path%2Fpath")));
     }
 
+
+    /**
+     * Check that calling Part(String, String) with inconsistent Strings does not lead
+     * to the Part's encoded vs. decoded values being inconsistent.
+     */
+    public void testPart_consistentEncodedVsDecoded() throws Exception {
+        Object authority = createPart(Class.forName("android.net.Uri$Part"), "a.com", "b.com");
+        Object path = createPart(Class.forName("android.net.Uri$PathPart"), "/foo/a", "/foo/b");
+        Uri uri = makeHierarchicalUri(authority, path);
+        // In these cases, decoding/encoding the encoded/decoded representation yields the same
+        // String, so we can just assert equality.
+        // assertEquals(uri.getPath(), uri.getEncodedPath());
+        assertEquals(uri.getAuthority(), uri.getEncodedAuthority());
+
+        // When both encoded and decoded strings are given, the encoded one is preferred.
+        assertEquals("a.com", uri.getAuthority());
+        assertEquals("/foo/a", uri.getPath());
+    }
+
+    private Object createPart(Class partClass, String encoded, String decoded) throws Exception {
+        Constructor partConstructor = partClass.getDeclaredConstructor(String.class, String.class);
+        partConstructor.setAccessible(true);
+        return partConstructor.newInstance(encoded, decoded);
+    }
+
+    private static Uri makeHierarchicalUri(Object authority, Object path) throws Exception {
+        Class hierarchicalUriClass = Class.forName("android.net.Uri$HierarchicalUri");
+        Constructor hierarchicalUriConstructor = hierarchicalUriClass.getDeclaredConstructors()[0];
+        hierarchicalUriConstructor.setAccessible(true);
+        return (Uri) hierarchicalUriConstructor.newInstance("https", authority, path, null, null);
+    }
+
+    /** Attempting to unparcel a legacy parcel format of Uri.{,Path}Part should fail. */
+    public void testUnparcelLegacyPart_fails() throws Exception {
+        assertUnparcelLegacyPart_fails(Class.forName("android.net.Uri$Part"));
+        assertUnparcelLegacyPart_fails(Class.forName("android.net.Uri$PathPart"));
+    }
+
+    private static void assertUnparcelLegacyPart_fails(Class partClass) throws Exception {
+        Parcel parcel = Parcel.obtain();
+        parcel.writeInt(0 /* BOTH */);
+        parcel.writeString("encoded");
+        parcel.writeString("decoded");
+        parcel.setDataPosition(0);
+
+        Method readFromMethod = partClass.getDeclaredMethod("readFrom", Parcel.class);
+        readFromMethod.setAccessible(true);
+        try {
+            readFromMethod.invoke(null, parcel);
+            fail();
+        } catch (InvocationTargetException expected) {
+            Throwable targetException = expected.getTargetException();
+            // Check that the exception was thrown for the correct reason.
+            assertEquals("Unknown representation: 0", targetException.getMessage());
+        }
+    }
+
     public void testToSafeString() {
         checkToSafeString("tel:xxxxxx", "tel:Google");
         checkToSafeString("tel:xxxxxxxxxx", "tel:1234567890");