Fix NPE in SyncMessage
Integer parsing in SyncMessage is throwing NPE if a field is missing.
Some fields are mandatory in a SYNC SMS, but carriers sometimes does
not send them all.
This CL refactors SyncMessage and StatusMessage so missing fields are
accounted for and all fields will have non null default value.
+ @NeededForTesting to prevent proguard from removing unused methods
Change-Id: I8af34de18cbed4484d592292aaec03456ed05af5
Fixes:29218479
diff --git a/proguard.flags b/proguard.flags
index 41e26a1..e8646eb 100644
--- a/proguard.flags
+++ b/proguard.flags
@@ -3,4 +3,8 @@
-keepclassmembers class * {
@**.VisibleForTesting *;
}
+-keep @**.NeededForTesting class *
+-keepclassmembers class * {
+@**.NeededForTesting *;
+}
-verbose
diff --git a/src/com/android/phone/NeededForTesting.java b/src/com/android/phone/NeededForTesting.java
new file mode 100644
index 0000000..576598b
--- /dev/null
+++ b/src/com/android/phone/NeededForTesting.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.phone;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+@Retention(RetentionPolicy.SOURCE)
+public @interface NeededForTesting {
+
+}
diff --git a/src/com/android/phone/vvm/omtp/sms/StatusMessage.java b/src/com/android/phone/vvm/omtp/sms/StatusMessage.java
index ee1f07d..f9d972f 100644
--- a/src/com/android/phone/vvm/omtp/sms/StatusMessage.java
+++ b/src/com/android/phone/vvm/omtp/sms/StatusMessage.java
@@ -18,6 +18,7 @@
import android.os.Bundle;
import android.telecom.Log;
+import com.android.phone.NeededForTesting;
import com.android.phone.vvm.omtp.OmtpConstants;
/**
@@ -61,19 +62,19 @@
}
public StatusMessage(Bundle wrappedData) {
- mProvisioningStatus = unquote(wrappedData.getString(OmtpConstants.PROVISIONING_STATUS));
- mStatusReturnCode = wrappedData.getString(OmtpConstants.RETURN_CODE);
- mSubscriptionUrl = wrappedData.getString(OmtpConstants.SUBSCRIPTION_URL);
- mServerAddress = wrappedData.getString(OmtpConstants.SERVER_ADDRESS);
- mTuiAccessNumber = wrappedData.getString(OmtpConstants.TUI_ACCESS_NUMBER);
- mClientSmsDestinationNumber = wrappedData.getString(
+ mProvisioningStatus = unquote(getString(wrappedData, OmtpConstants.PROVISIONING_STATUS));
+ mStatusReturnCode = getString(wrappedData, OmtpConstants.RETURN_CODE);
+ mSubscriptionUrl = getString(wrappedData, OmtpConstants.SUBSCRIPTION_URL);
+ mServerAddress = getString(wrappedData, OmtpConstants.SERVER_ADDRESS);
+ mTuiAccessNumber = getString(wrappedData, OmtpConstants.TUI_ACCESS_NUMBER);
+ mClientSmsDestinationNumber = getString(wrappedData,
OmtpConstants.CLIENT_SMS_DESTINATION_NUMBER);
- mImapPort = wrappedData.getString(OmtpConstants.IMAP_PORT);
- mImapUserName = wrappedData.getString(OmtpConstants.IMAP_USER_NAME);
- mImapPassword = wrappedData.getString(OmtpConstants.IMAP_PASSWORD);
- mSmtpPort = wrappedData.getString(OmtpConstants.SMTP_PORT);
- mSmtpUserName = wrappedData.getString(OmtpConstants.SMTP_USER_NAME);
- mSmtpPassword = wrappedData.getString(OmtpConstants.SMTP_PASSWORD);
+ mImapPort = getString(wrappedData, OmtpConstants.IMAP_PORT);
+ mImapUserName = getString(wrappedData, OmtpConstants.IMAP_USER_NAME);
+ mImapPassword = getString(wrappedData, OmtpConstants.IMAP_PASSWORD);
+ mSmtpPort = getString(wrappedData, OmtpConstants.SMTP_PORT);
+ mSmtpUserName = getString(wrappedData, OmtpConstants.SMTP_USER_NAME);
+ mSmtpPassword = getString(wrappedData, OmtpConstants.SMTP_PASSWORD);
}
private static String unquote(String string) {
@@ -104,6 +105,7 @@
* @return the URL of the voicemail server. This is the URL to send the users to for subscribing
* to the visual voicemail service.
*/
+ @NeededForTesting
public String getSubscriptionUrl() {
return mSubscriptionUrl;
}
@@ -120,6 +122,7 @@
* @return the Telephony User Interface number to call to access voicemails directly from the
* IVR.
*/
+ @NeededForTesting
public String getTuiAccessNumber() {
return mTuiAccessNumber;
}
@@ -127,6 +130,7 @@
/**
* @return the number to which client originated SMSes should be sent to.
*/
+ @NeededForTesting
public String getClientSmsDestinationNumber() {
return mClientSmsDestinationNumber;
}
@@ -155,6 +159,7 @@
/**
* @return the SMTP server port to talk to.
*/
+ @NeededForTesting
public String getSmtpPort() {
return mSmtpPort;
}
@@ -162,6 +167,7 @@
/**
* @return the SMTP user name to be used for SMTP authentication.
*/
+ @NeededForTesting
public String getSmtpUserName() {
return mSmtpUserName;
}
@@ -169,7 +175,16 @@
/**
* @return the SMTP password to be used for SMTP authentication.
*/
+ @NeededForTesting
public String getSmtpPassword() {
return mSmtpPassword;
}
+
+ private static String getString(Bundle bundle, String key) {
+ String value = bundle.getString(key);
+ if (value == null) {
+ return "";
+ }
+ return value;
+ }
}
\ No newline at end of file
diff --git a/src/com/android/phone/vvm/omtp/sms/SyncMessage.java b/src/com/android/phone/vvm/omtp/sms/SyncMessage.java
index 4dddba4..632ff9e 100644
--- a/src/com/android/phone/vvm/omtp/sms/SyncMessage.java
+++ b/src/com/android/phone/vvm/omtp/sms/SyncMessage.java
@@ -18,6 +18,7 @@
import android.annotation.Nullable;
import android.os.Bundle;
+import com.android.phone.NeededForTesting;
import com.android.phone.vvm.omtp.OmtpConstants;
import java.text.ParseException;
@@ -33,18 +34,17 @@
// Sync event that triggered this message.
private final String mSyncTriggerEvent;
// Total number of new messages on the server.
- private final Integer mNewMessageCount;
+ private final int mNewMessageCount;
// UID of the new message.
private final String mMessageId;
// Length of the message.
- @Nullable
- private final Integer mMessageLength;
+ private final int mMessageLength;
// Content type (voice, video, fax...) of the new message.
private final String mContentType;
// Sender of the new message.
private final String mSender;
// Timestamp (in millis) of the new message.
- private final Long mMsgTimeMillis;
+ private final long mMsgTimeMillis;
@Override
public String toString() {
@@ -58,21 +58,19 @@
}
public SyncMessage(Bundle wrappedData) {
- mSyncTriggerEvent = wrappedData.getString(OmtpConstants.SYNC_TRIGGER_EVENT);
- mMessageId = wrappedData.getString(OmtpConstants.MESSAGE_UID);
- if (wrappedData.getString(OmtpConstants.MESSAGE_LENGTH) != null) {
- mMessageLength = Integer.parseInt(wrappedData.getString(OmtpConstants.MESSAGE_LENGTH));
- } else {
- // Optional field
- mMessageLength = null;
- }
- mContentType = wrappedData.getString(OmtpConstants.CONTENT_TYPE);
- mSender = wrappedData.getString(OmtpConstants.SENDER);
- mNewMessageCount = Integer.parseInt(wrappedData.getString(OmtpConstants.NUM_MESSAGE_COUNT));
+ mSyncTriggerEvent = getString(wrappedData, OmtpConstants.SYNC_TRIGGER_EVENT);
+ mMessageId = getString(wrappedData, OmtpConstants.MESSAGE_UID);
+ mMessageLength = getInt(wrappedData, OmtpConstants.MESSAGE_LENGTH);
+ mContentType = getString(wrappedData, OmtpConstants.CONTENT_TYPE);
+ mSender = getString(wrappedData, OmtpConstants.SENDER);
+ mNewMessageCount = getInt(wrappedData, OmtpConstants.NUM_MESSAGE_COUNT);
mMsgTimeMillis = parseTime(wrappedData.getString(OmtpConstants.TIME));
}
- static Long parseTime(String value) {
+ private static long parseTime(@Nullable String value) {
+ if (value == null) {
+ return 0L;
+ }
try {
return new SimpleDateFormat(
OmtpConstants.DATE_TIME_FORMAT, Locale.US)
@@ -92,8 +90,9 @@
/**
* @return the number of new messages stored on the voicemail server.
*/
+ @NeededForTesting
public int getNewMessageCount() {
- return mNewMessageCount != null ? mNewMessageCount : 0;
+ return mNewMessageCount;
}
/**
@@ -112,6 +111,7 @@
* Expected to be set only for
* {@link com.android.phone.vvm.omtp.OmtpConstants#NEW_MESSAGE}
*/
+ @NeededForTesting
public String getContentType() {
return mContentType;
}
@@ -123,7 +123,7 @@
* {@link com.android.phone.vvm.omtp.OmtpConstants#NEW_MESSAGE}
*/
public int getLength() {
- return mMessageLength != null ? mMessageLength : 0;
+ return mMessageLength;
}
/**
@@ -143,6 +143,26 @@
* {@link com.android.phone.vvm.omtp.OmtpConstants#NEW_MESSAGE}
*/
public long getTimestampMillis() {
- return mMsgTimeMillis != null ? mMsgTimeMillis : 0;
+ return mMsgTimeMillis;
+ }
+
+ private static int getInt(Bundle wrappedData, String key) {
+ String value = wrappedData.getString(key);
+ if (value == null) {
+ return 0;
+ }
+ try {
+ return Integer.parseInt(value);
+ } catch (NumberFormatException e) {
+ return 0;
+ }
+ }
+
+ private static String getString(Bundle wrappedData, String key) {
+ String value = wrappedData.getString(key);
+ if (value == null) {
+ return "";
+ }
+ return value;
}
}
\ No newline at end of file
diff --git a/tests/src/com/android/phone/vvm/omtp/StatusMessageTest.java b/tests/src/com/android/phone/vvm/omtp/StatusMessageTest.java
new file mode 100644
index 0000000..fd3aa2c
--- /dev/null
+++ b/tests/src/com/android/phone/vvm/omtp/StatusMessageTest.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.phone.vvm.omtp;
+
+import android.os.Bundle;
+
+import com.android.internal.annotations.VisibleForTesting;
+import com.android.phone.vvm.omtp.sms.StatusMessage;
+
+import junit.framework.TestCase;
+
+@VisibleForTesting
+public class StatusMessageTest extends TestCase {
+
+ public void testStatusMessage() {
+ Bundle bundle = new Bundle();
+ bundle.putString(OmtpConstants.PROVISIONING_STATUS, "status");
+ bundle.putString(OmtpConstants.RETURN_CODE, "code");
+ bundle.putString(OmtpConstants.SUBSCRIPTION_URL, "url");
+ bundle.putString(OmtpConstants.SERVER_ADDRESS, "address");
+ bundle.putString(OmtpConstants.TUI_ACCESS_NUMBER, "tui");
+ bundle.putString(OmtpConstants.CLIENT_SMS_DESTINATION_NUMBER, "sms");
+ bundle.putString(OmtpConstants.IMAP_PORT, "1234");
+ bundle.putString(OmtpConstants.IMAP_USER_NAME, "username");
+ bundle.putString(OmtpConstants.IMAP_PASSWORD, "password");
+ bundle.putString(OmtpConstants.SMTP_PORT, "s1234");
+ bundle.putString(OmtpConstants.SMTP_USER_NAME, "susername");
+ bundle.putString(OmtpConstants.SMTP_PASSWORD, "spassword");
+
+ StatusMessage message = new StatusMessage(bundle);
+ assertEquals("status", message.getProvisioningStatus());
+ assertEquals("code", message.getReturnCode());
+ assertEquals("url", message.getSubscriptionUrl());
+ assertEquals("address", message.getServerAddress());
+ assertEquals("tui", message.getTuiAccessNumber());
+ assertEquals("sms", message.getClientSmsDestinationNumber());
+ assertEquals("1234", message.getImapPort());
+ assertEquals("username", message.getImapUserName());
+ assertEquals("password", message.getImapPassword());
+ assertEquals("s1234", message.getSmtpPort());
+ assertEquals("susername", message.getSmtpUserName());
+ assertEquals("spassword", message.getSmtpPassword());
+ }
+
+ public void testSyncMessage_EmptyBundle() {
+ StatusMessage message = new StatusMessage(new Bundle());
+ assertEquals("", message.getProvisioningStatus());
+ assertEquals("", message.getReturnCode());
+ assertEquals("", message.getSubscriptionUrl());
+ assertEquals("", message.getServerAddress());
+ assertEquals("", message.getTuiAccessNumber());
+ assertEquals("", message.getClientSmsDestinationNumber());
+ assertEquals("", message.getImapPort());
+ assertEquals("", message.getImapUserName());
+ assertEquals("", message.getImapPassword());
+ assertEquals("", message.getSmtpPort());
+ assertEquals("", message.getSmtpUserName());
+ assertEquals("", message.getSmtpPassword());
+ }
+}
diff --git a/tests/src/com/android/phone/vvm/omtp/SyncMessageTest.java b/tests/src/com/android/phone/vvm/omtp/SyncMessageTest.java
new file mode 100644
index 0000000..61ae400
--- /dev/null
+++ b/tests/src/com/android/phone/vvm/omtp/SyncMessageTest.java
@@ -0,0 +1,67 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.phone.vvm.omtp;
+
+import android.os.Bundle;
+
+import com.android.phone.vvm.omtp.sms.SyncMessage;
+
+import junit.framework.TestCase;
+
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
+import java.util.Locale;
+
+public class SyncMessageTest extends TestCase {
+
+ public void testSyncMessage() {
+ Bundle bundle = new Bundle();
+ bundle.putString(OmtpConstants.SYNC_TRIGGER_EVENT, "event");
+ bundle.putString(OmtpConstants.MESSAGE_UID, "uid");
+ bundle.putString(OmtpConstants.MESSAGE_LENGTH, "1");
+ bundle.putString(OmtpConstants.CONTENT_TYPE, "type");
+ bundle.putString(OmtpConstants.SENDER, "sender");
+ bundle.putString(OmtpConstants.NUM_MESSAGE_COUNT, "2");
+ bundle.putString(OmtpConstants.TIME, "29/08/1997 02:14 -0400");
+
+ SyncMessage message = new SyncMessage(bundle);
+ assertEquals("event", message.getSyncTriggerEvent());
+ assertEquals("uid", message.getId());
+ assertEquals(1, message.getLength());
+ assertEquals("type", message.getContentType());
+ assertEquals("sender", message.getSender());
+ assertEquals(2, message.getNewMessageCount());
+ try {
+ assertEquals(new SimpleDateFormat(
+ OmtpConstants.DATE_TIME_FORMAT, Locale.US)
+ .parse("29/08/1997 02:14 -0400").getTime(), message.getTimestampMillis());
+ } catch (ParseException e) {
+ throw new AssertionError(e.toString());
+ }
+ }
+
+ public void testSyncMessage_EmptyBundle() {
+ SyncMessage message = new SyncMessage(new Bundle());
+ assertEquals("", message.getSyncTriggerEvent());
+ assertEquals("", message.getId());
+ assertEquals(0, message.getLength());
+ assertEquals("", message.getContentType());
+ assertEquals("", message.getSender());
+ assertEquals(0, message.getNewMessageCount());
+ assertEquals(0, message.getTimestampMillis());
+ }
+}