Making vcard photo parsing more robust to decode errors.
Base64.decode throws IllegalArgumentException which is not handled
and results in crash reports for users. Switching to VCardException
which should have better UI handling.
Change-Id: I3fa12b8a703c3eed181caade0bfda2271435b377
diff --git a/java/com/android/vcard/VCardParserImpl_V21.java b/java/com/android/vcard/VCardParserImpl_V21.java
index 378b1c8..410cf4f 100644
--- a/java/com/android/vcard/VCardParserImpl_V21.java
+++ b/java/com/android/vcard/VCardParserImpl_V21.java
@@ -638,7 +638,12 @@
// It is very rare, but some BASE64 data may be so big that
// OutOfMemoryError occurs. To ignore such cases, use try-catch.
try {
- property.setByteValue(Base64.decode(getBase64(propertyRawValue), Base64.DEFAULT));
+ final String base64Property = getBase64(propertyRawValue);
+ try {
+ property.setByteValue(Base64.decode(base64Property, Base64.DEFAULT));
+ } catch (IllegalArgumentException e) {
+ throw new VCardException("Decode error on base64 photo: " + propertyRawValue);
+ }
for (VCardInterpreter interpreter : mInterpreterList) {
interpreter.onPropertyCreated(property);
}
diff --git a/tests/res/raw/v21_malformed_photo.vcf b/tests/res/raw/v21_malformed_photo.vcf
new file mode 100644
index 0000000..56fa99f
--- /dev/null
+++ b/tests/res/raw/v21_malformed_photo.vcf
@@ -0,0 +1,7 @@
+BEGIN:VCARD
+VERSION:2.1
+N:name
+FN:fullname
+PHOTO;ENCODING=BASE64;TYPE=JPEG:qgEPAAIAAAAHAAAAugEQAAIAAAAG:ASDF==
+
+END:VCARD
diff --git a/tests/src/com/android/vcard/tests/VCardImporterTests.java b/tests/src/com/android/vcard/tests/VCardImporterTests.java
index 8c7bd68..3279430 100644
--- a/tests/src/com/android/vcard/tests/VCardImporterTests.java
+++ b/tests/src/com/android/vcard/tests/VCardImporterTests.java
@@ -1231,6 +1231,13 @@
new TypeSet("INTERNET"));
}
+ public void testMalformedBase64PhotoThrowsVCardException() {
+ mVerifier.initForImportTest(V21, R.raw.v21_malformed_photo);
+
+ String expectedMsgContent = "qgEPAAIAAAAHAAAAugEQAAIAAAAG:ASDF==";
+ mVerifier.addVCardExceptionVerifier(expectedMsgContent);
+ }
+
public void testAndroidCustomPropertyV21() {
mVerifier.initForImportTest(V21, R.raw.v21_android_custom_prop);
final ContentValuesVerifierElem elem = mVerifier.addContentValuesVerifierElem();
diff --git a/tests/src/com/android/vcard/tests/testutils/VCardVerifier.java b/tests/src/com/android/vcard/tests/testutils/VCardVerifier.java
index b07564c..3e787de 100644
--- a/tests/src/com/android/vcard/tests/testutils/VCardVerifier.java
+++ b/tests/src/com/android/vcard/tests/testutils/VCardVerifier.java
@@ -15,14 +15,6 @@
*/
package com.android.vcard.tests.testutils;
-import com.android.vcard.VCardComposer;
-import com.android.vcard.VCardConfig;
-import com.android.vcard.VCardEntryConstructor;
-import com.android.vcard.VCardInterpreter;
-import com.android.vcard.VCardParser;
-import com.android.vcard.VCardUtils;
-import com.android.vcard.exception.VCardException;
-
import android.content.ContentResolver;
import android.content.EntityIterator;
import android.database.Cursor;
@@ -32,13 +24,19 @@
import android.text.TextUtils;
import android.util.Log;
+import com.android.vcard.VCardComposer;
+import com.android.vcard.VCardConfig;
+import com.android.vcard.VCardEntryConstructor;
+import com.android.vcard.VCardParser;
+import com.android.vcard.VCardUtils;
+import com.android.vcard.exception.VCardException;
+
import junit.framework.TestCase;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Method;
-import java.util.Arrays;
/**
* <p>
@@ -93,6 +91,8 @@
private boolean mVerified = false;
private String mCharset;
+ private String mExceptionContents;
+
// Called by VCardTestsBase
public VCardVerifier(AndroidTestCase androidTestCase) {
mAndroidTestCase = androidTestCase;
@@ -225,6 +225,10 @@
return mContentValuesVerifier.addElem(mAndroidTestCase);
}
+ public void addVCardExceptionVerifier(String contents) {
+ mExceptionContents = contents;
+ }
+
/**
* Sets up sub-verifiers correctly and tries to parse vCard as {@link InputStream}.
* Errors around InputStream must be handled outside this method.
@@ -246,9 +250,17 @@
parser.addInterpreter(mPropertyNodesVerifier);
}
parser.parse(is);
+ if (mExceptionContents != null) {
+ // exception contents exists, we expect an exception to occur.
+ AndroidTestCase.fail();
+ }
} catch (VCardException e) {
- Log.e(LOG_TAG, "VCardException", e);
- AndroidTestCase.fail("Unexpected VCardException: " + e.getMessage());
+ if (mExceptionContents != null) {
+ AndroidTestCase.assertTrue(e.getMessage().contains(mExceptionContents));
+ } else {
+ Log.e(LOG_TAG, "VCardException", e);
+ AndroidTestCase.fail("Unexpected VCardException: " + e.getMessage());
+ }
}
}
