Invoke SMS filters for messages from blocked numbers.
Users block numbers to avoid seeing visible messages from those
numbers. However, SMS is used for visual voicemail notifications and
other carrier system messages; if a user happens to block the number
used to send these messages, then these important messages would be
dropped before the relevant filters would be invoked.
We now allow the filters to run independent of block state, which
is applied after the filters. This permits filters to run while
still ensuring that the user never sees a message from a blocked
number in their SMS app.
Bug: 136262737
Test: atest GsmInboundSmsHandlerTest; on-device test of blocked SMS
Change-Id: Ia04334fccae25ffc93459de425b613201705ef48
Merged-In: Ia04334fccae25ffc93459de425b613201705ef48
diff --git a/src/java/com/android/internal/telephony/InboundSmsHandler.java b/src/java/com/android/internal/telephony/InboundSmsHandler.java
index 43b42dd..2ea3020 100644
--- a/src/java/com/android/internal/telephony/InboundSmsHandler.java
+++ b/src/java/com/android/internal/telephony/InboundSmsHandler.java
@@ -1057,18 +1057,23 @@
}
}
- if (block) {
- deleteFromRawTable(tracker.getDeleteWhere(), tracker.getDeleteWhereArgs(),
- DELETE_PERMANENTLY);
- log("processMessagePart: returning false as the phone number is blocked",
- tracker.getMessageId());
- return false;
- }
-
+ // Always invoke SMS filters, even if the number ends up being blocked, to prevent
+ // surprising bugs due to blocking numbers that happen to be used for visual voicemail SMS
+ // or other carrier system messages.
boolean filterInvoked = filterSms(
- pdus, destPort, tracker, resultReceiver, true /* userUnlocked */);
+ pdus, destPort, tracker, resultReceiver, true /* userUnlocked */, block);
if (!filterInvoked) {
+ // Block now if the filter wasn't invoked. Otherwise, it will be the responsibility of
+ // the filter to delete the SMS once processing completes.
+ if (block) {
+ deleteFromRawTable(tracker.getDeleteWhere(), tracker.getDeleteWhereArgs(),
+ DELETE_PERMANENTLY);
+ log("processMessagePart: returning false as the phone number is blocked",
+ tracker.getMessageId());
+ return false;
+ }
+
dispatchSmsDeliveryIntent(pdus, format, destPort, resultReceiver,
tracker.isClass0(), tracker.getSubId(), tracker.getMessageId());
}
@@ -1094,7 +1099,8 @@
if (destPort == -1) {
// This is a regular SMS - hand it to the carrier or system app for filtering.
boolean filterInvoked = filterSms(
- pdus, destPort, tracker, resultReceiver, false /* userUnlocked */);
+ pdus, destPort, tracker, resultReceiver, false /* userUnlocked */,
+ false /* block */);
if (filterInvoked) {
// filter invoked, wait for it to return the result.
return true;
@@ -1153,13 +1159,14 @@
private List<SmsFilter> createDefaultSmsFilters() {
List<SmsFilter> smsFilters = new ArrayList<>(3);
smsFilters.add(
- (pdus, destPort, tracker, resultReceiver, userUnlocked, remainingFilters) -> {
+ (pdus, destPort, tracker, resultReceiver, userUnlocked, block, remainingFilters)
+ -> {
CarrierServicesSmsFilterCallback filterCallback =
new CarrierServicesSmsFilterCallback(
pdus, destPort, tracker, tracker.getFormat(), resultReceiver,
userUnlocked,
tracker.isClass0(), tracker.getSubId(), tracker.getMessageId(),
- remainingFilters);
+ block, remainingFilters);
CarrierServicesSmsFilter carrierServicesFilter = new CarrierServicesSmsFilter(
mContext, mPhone, pdus, destPort, tracker.getFormat(),
filterCallback, getName() + "::CarrierServicesSmsFilter",
@@ -1172,23 +1179,25 @@
}
});
smsFilters.add(
- (pdus, destPort, tracker, resultReceiver, userUnlocked, remainingFilters) -> {
+ (pdus, destPort, tracker, resultReceiver, userUnlocked, block, remainingFilters)
+ -> {
if (VisualVoicemailSmsFilter.filter(
mContext, pdus, tracker.getFormat(), destPort, tracker.getSubId())) {
logWithLocalLog("Visual voicemail SMS dropped", tracker.getMessageId());
- dropSms(resultReceiver);
+ dropFilteredSms(tracker, resultReceiver, block);
return true;
}
return false;
});
smsFilters.add(
- (pdus, destPort, tracker, resultReceiver, userUnlocked, remainingFilters) -> {
+ (pdus, destPort, tracker, resultReceiver, userUnlocked, block, remainingFilters)
+ -> {
MissedIncomingCallSmsFilter missedIncomingCallSmsFilter =
new MissedIncomingCallSmsFilter(mPhone);
if (missedIncomingCallSmsFilter.filter(pdus, tracker.getFormat())) {
logWithLocalLog("Missed incoming call SMS received",
tracker.getMessageId());
- dropSms(resultReceiver);
+ dropFilteredSms(tracker, resultReceiver, block);
return true;
}
return false;
@@ -1196,6 +1205,18 @@
return smsFilters;
}
+ private void dropFilteredSms(
+ InboundSmsTracker tracker, SmsBroadcastReceiver resultReceiver, boolean block) {
+ if (block) {
+ deleteFromRawTable(
+ tracker.getDeleteWhere(), tracker.getDeleteWhereArgs(),
+ DELETE_PERMANENTLY);
+ sendMessage(EVENT_BROADCAST_COMPLETE);
+ } else {
+ dropSms(resultReceiver);
+ }
+ }
+
/**
* Filters the SMS.
*
@@ -1205,17 +1226,18 @@
* @return true if a filter is invoked and the SMS processing flow is diverted, false otherwise.
*/
private boolean filterSms(byte[][] pdus, int destPort,
- InboundSmsTracker tracker, SmsBroadcastReceiver resultReceiver, boolean userUnlocked) {
- return filterSms(pdus, destPort, tracker, resultReceiver, userUnlocked, mSmsFilters);
+ InboundSmsTracker tracker, SmsBroadcastReceiver resultReceiver, boolean userUnlocked,
+ boolean block) {
+ return filterSms(pdus, destPort, tracker, resultReceiver, userUnlocked, block, mSmsFilters);
}
private static boolean filterSms(byte[][] pdus, int destPort,
InboundSmsTracker tracker, SmsBroadcastReceiver resultReceiver, boolean userUnlocked,
- List<SmsFilter> filters) {
+ boolean block, List<SmsFilter> filters) {
ListIterator<SmsFilter> iterator = filters.listIterator();
while (iterator.hasNext()) {
SmsFilter smsFilter = iterator.next();
- if (smsFilter.filterSms(pdus, destPort, tracker, resultReceiver, userUnlocked,
+ if (smsFilter.filterSms(pdus, destPort, tracker, resultReceiver, userUnlocked, block,
filters.subList(iterator.nextIndex(), filters.size()))) {
return true;
}
@@ -1657,11 +1679,13 @@
private final boolean mIsClass0;
private final int mSubId;
private final long mMessageId;
+ private final boolean mBlock;
private final List<SmsFilter> mRemainingFilters;
CarrierServicesSmsFilterCallback(byte[][] pdus, int destPort, InboundSmsTracker tracker,
String smsFormat, SmsBroadcastReceiver smsBroadcastReceiver, boolean userUnlocked,
- boolean isClass0, int subId, long messageId, List<SmsFilter> remainingFilters) {
+ boolean isClass0, int subId, long messageId, boolean block,
+ List<SmsFilter> remainingFilters) {
mPdus = pdus;
mDestPort = destPort;
mTracker = tracker;
@@ -1671,35 +1695,53 @@
mIsClass0 = isClass0;
mSubId = subId;
mMessageId = messageId;
+ mBlock = block;
mRemainingFilters = remainingFilters;
}
@Override
public void onFilterComplete(int result) {
log("onFilterComplete: result is " + result, mMessageId);
- if ((result & CarrierMessagingService.RECEIVE_OPTIONS_DROP) == 0) {
- // Message isn't dropped, so run it through the remaining filters.
- if (filterSms(mPdus, mDestPort, mTracker, mSmsBroadcastReceiver, mUserUnlocked,
- mRemainingFilters)) {
- return;
- }
- if (mUserUnlocked) {
- dispatchSmsDeliveryIntent(
- mPdus, mSmsFormat, mDestPort, mSmsBroadcastReceiver, mIsClass0, mSubId,
- mMessageId);
- } else {
- // Don't do anything further, leave the message in the raw table if the
- // credential-encrypted storage is still locked and show the new message
- // notification if the message is visible to the user.
- if (!isSkipNotifyFlagSet(result)) {
- showNewMessageNotification();
- }
- sendMessage(EVENT_BROADCAST_COMPLETE);
- }
+ boolean carrierRequestedDrop =
+ (result & CarrierMessagingService.RECEIVE_OPTIONS_DROP) != 0;
+ if (carrierRequestedDrop) {
+ // Carrier app asked the platform to drop the SMS. Drop it from the database and
+ // complete processing.
+ dropFilteredSms(mTracker, mSmsBroadcastReceiver, mBlock);
+ return;
+ }
+
+ boolean filterInvoked = filterSms(mPdus, mDestPort, mTracker, mSmsBroadcastReceiver,
+ mUserUnlocked, mBlock, mRemainingFilters);
+ if (filterInvoked) {
+ // A remaining filter has assumed responsibility for further message processing.
+ return;
+ }
+
+ // Now that all filters have been invoked, drop the message if it is blocked.
+ // TODO(b/156910035): Remove mUserUnlocked once we stop showing the new message
+ // notification for blocked numbers.
+ if (mUserUnlocked && mBlock) {
+ log("onFilterComplete: dropping message as the sender is blocked",
+ mTracker.getMessageId());
+ dropFilteredSms(mTracker, mSmsBroadcastReceiver, mBlock);
+ return;
+ }
+
+ // Message matched no filters and is not blocked, so complete processing.
+ if (mUserUnlocked) {
+ dispatchSmsDeliveryIntent(
+ mPdus, mSmsFormat, mDestPort, mSmsBroadcastReceiver, mIsClass0, mSubId,
+ mMessageId);
} else {
- // Drop this SMS.
- dropSms(mSmsBroadcastReceiver);
+ // Don't do anything further, leave the message in the raw table if the
+ // credential-encrypted storage is still locked and show the new message
+ // notification if the message is visible to the user.
+ if (!isSkipNotifyFlagSet(result)) {
+ showNewMessageNotification();
+ }
+ sendMessage(EVENT_BROADCAST_COMPLETE);
}
}
}
@@ -2025,9 +2067,20 @@
/**
* Returns true if a filter is invoked and the SMS processing flow should be diverted, false
* otherwise.
+ *
+ * <p>If the filter can immediately determine that the message matches, it must call
+ * {@link #dropFilteredSms} to drop the message from the database once it has been
+ * processed.
+ *
+ * <p>If the filter must perform some asynchronous work to determine if the message matches,
+ * it should return true to defer processing. Once it has made a determination, if it finds
+ * the message matches, it must call {@link #dropFilteredSms}. If the message does not
+ * match, it must be passed through {@code remainingFilters} and either dropped if the
+ * remaining filters all return false or if {@code block} is true, or else it must be
+ * broadcast.
*/
boolean filterSms(byte[][] pdus, int destPort, InboundSmsTracker tracker,
- SmsBroadcastReceiver resultReceiver, boolean userUnlocked,
+ SmsBroadcastReceiver resultReceiver, boolean userUnlocked, boolean block,
List<SmsFilter> remainingFilters);
}
}
diff --git a/tests/telephonytests/src/com/android/internal/telephony/gsm/GsmInboundSmsHandlerTest.java b/tests/telephonytests/src/com/android/internal/telephony/gsm/GsmInboundSmsHandlerTest.java
index 913c434..54d4c80 100644
--- a/tests/telephonytests/src/com/android/internal/telephony/gsm/GsmInboundSmsHandlerTest.java
+++ b/tests/telephonytests/src/com/android/internal/telephony/gsm/GsmInboundSmsHandlerTest.java
@@ -374,9 +374,8 @@
verify(mContext, never()).sendBroadcast(any(Intent.class));
assertEquals("IdleState", getCurrentState().getName());
- // verify no filter was invoked.
- // TODO(b/136262737): Adjust test once blocked SMSes are passed through filters too.
- verifySmsFiltersInvoked(never());
+ // Filter should still be invoked.
+ verifySmsFiltersInvoked(times(1));
}
@Test
@@ -385,7 +384,7 @@
// Configure the first filter to drop the SMS.
when(mSmsFilter.filterSms(any(byte[][].class), anyInt(),
any(InboundSmsTracker.class), any(InboundSmsHandler.SmsBroadcastReceiver.class),
- anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any()))
+ anyBoolean(), anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any()))
.thenAnswer((Answer<Boolean>) invocation -> {
mGsmInboundSmsHandler.sendMessage(InboundSmsHandler.EVENT_BROADCAST_COMPLETE);
return true;
@@ -401,7 +400,7 @@
// verify second filter was never invoked.
verify(mSmsFilter2, never()).filterSms(any(byte[][].class), anyInt(),
any(InboundSmsTracker.class), any(InboundSmsHandler.SmsBroadcastReceiver.class),
- anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any());
+ anyBoolean(), anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any());
}
@Test
@@ -410,7 +409,8 @@
// Have the first filter indicate it matched without completing the flow.
when(mSmsFilter.filterSms(any(byte[][].class), anyInt(),
any(InboundSmsTracker.class), any(InboundSmsHandler.SmsBroadcastReceiver.class),
- anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any())).thenReturn(true);
+ anyBoolean(), anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any()))
+ .thenReturn(true);
transitionFromStartupToIdle();
@@ -423,12 +423,12 @@
// Verify the first filter was invoked with the right set of remaining filters.
verify(mSmsFilter).filterSms(any(byte[][].class), anyInt(),
any(InboundSmsTracker.class), any(InboundSmsHandler.SmsBroadcastReceiver.class),
- anyBoolean(), eq(Collections.singletonList(mSmsFilter2)));
+ anyBoolean(), anyBoolean(), eq(Collections.singletonList(mSmsFilter2)));
// Verify second filter was never invoked.
verify(mSmsFilter2, never()).filterSms(any(byte[][].class), anyInt(),
any(InboundSmsTracker.class), any(InboundSmsHandler.SmsBroadcastReceiver.class),
- anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any());
+ anyBoolean(), anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any());
// Clean up by completing the broadcast, as an asynchronous filter must do.
mGsmInboundSmsHandler.sendMessage(InboundSmsHandler.EVENT_BROADCAST_COMPLETE);
@@ -862,8 +862,8 @@
verify(mContext, never()).sendBroadcast(any(Intent.class));
assertEquals("IdleState", getCurrentState().getName());
- // TODO(b/136262737): Adjust test once blocked SMSes are passed through filters too.
- verifySmsFiltersInvoked(never());
+ // Filter should still be invoked.
+ verifySmsFiltersInvoked(times(1));
}
@Test
@@ -914,8 +914,8 @@
verify(mContext, never()).sendBroadcast(any(Intent.class));
assertEquals("IdleState", getCurrentState().getName());
- // TODO(b/136262737): Adjust test once blocked SMSes are passed through filters too.
- verifySmsFiltersInvoked(never());
+ // Filter should still be invoked.
+ verifySmsFiltersInvoked(times(1));
}
@Test
@@ -924,7 +924,7 @@
// Configure the first filter to drop the SMS.
when(mSmsFilter.filterSms(any(byte[][].class), anyInt(),
any(InboundSmsTracker.class), any(InboundSmsHandler.SmsBroadcastReceiver.class),
- anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any()))
+ anyBoolean(), anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any()))
.thenAnswer((Answer<Boolean>) invocation -> {
mGsmInboundSmsHandler.sendMessage(InboundSmsHandler.EVENT_BROADCAST_COMPLETE);
return true;
@@ -962,7 +962,7 @@
// verify second filter was never invoked.
verify(mSmsFilter2, never()).filterSms(any(byte[][].class), anyInt(),
any(InboundSmsTracker.class), any(InboundSmsHandler.SmsBroadcastReceiver.class),
- anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any());
+ anyBoolean(), anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any());
}
@Test
@@ -1114,9 +1114,9 @@
private void verifySmsFiltersInvoked(VerificationMode verificationMode) {
verify(mSmsFilter, verificationMode).filterSms(any(byte[][].class), anyInt(),
any(InboundSmsTracker.class), any(InboundSmsHandler.SmsBroadcastReceiver.class),
- anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any());
+ anyBoolean(), anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any());
verify(mSmsFilter2, verificationMode).filterSms(any(byte[][].class), anyInt(),
any(InboundSmsTracker.class), any(InboundSmsHandler.SmsBroadcastReceiver.class),
- anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any());
+ anyBoolean(), anyBoolean(), Mockito.<List<InboundSmsHandler.SmsFilter>>any());
}
}
