Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Parcel only the canonical Uri.Part representation, not both.

Browse Source 
Before this CL, Uri.AbstractPart's implementation of Parcelable
was parceling and unparceling both the encoded and the decoded
representation. A Uri with inconsistent decoded/encoded
representation of its Parts would have remained inconsistent
across parcel/unparcel cycles. For example, such a Uri's
uri.getDecodedAuthority() might have returned "good.com" while
url.getEncodedAuthority() (used e.g. for toString()) returned
"evil.com".

After this CL, AbstractPart's constructor allows at most one of
the representations to be set (exception: NULL and EMPTY); this
means that no Part instance with inconsistent values can be
constructed via the constructor (e.g. by unparceling parceled
data). The historical parcel representation of a Part with both
values present can no longer be unparceled, which is safe because
Parcel does not guarantee backwards compatibility (the parceled
form must not be persisted across Android version upgrades).

When parceling, only one of the values is now stored, namely
the (canonical) one that was passed to the constructor.

Fixes: 124526860
Test: atest FrameworksCoreTests:android.net.UriTest
Test: Checked that if run before this CL, the added tests
      would fail with a failure along the lines of:
      ComparisonFailure: expected:</foo/[b]> but was:</foo/[a]>
      or (if the first assertion was commented out):
      ComparisonFailure: expected:<[b].com> but was:<[a].com>

Change-Id: I2bc2008e49de5a66641ecdbd8e5354dfa647269d
Merged-In: I2bc2008e49de5a66641ecdbd8e5354dfa647269d
(cherry picked from commit c9afa38f97)
This commit is contained in:
Tobias Thierer
2019-05-08 22:00:38 +01:00
parent 7891cda862
commit f35f2913f8
2 changed files with 94 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
core/java/android/net/Uri.java
View File

@@ -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) {
this.encoded = encoded;
this.decoded = 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")
boolean hasEncoded = encoded != NOT_CACHED;
@SuppressWarnings("StringEquality")
boolean hasDecoded = decoded != NOT_CACHED;
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);
final String canonicalValue;
if (mCanonicalRepresentation == Representation.ENCODED) {
canonicalValue = encoded;
} else if (mCanonicalRepresentation == Representation.DECODED) {
canonicalValue = 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);
}
}