From e763059a29c4b637ca926cbe69d54432f6910d83 Mon Sep 17 00:00:00 2001 From: Ahaan Ugale Date: Fri, 17 Apr 2020 21:38:47 -0700 Subject: [PATCH] Autofill: Fix unsafe usages of mCurrentViewId related to Inline UI. With this change, the value is captured locally before being used in any lambdas related to inline suggestions. Otherwise, the lambda can be executed for a different view than intended, which can also cause an NPE if a VIEW_EXITED event occurs (see linked bug). This change also includes a null-check in requestShowInlineSuggestionsLocked. It's unclear if it's possible for the value to be null there, but the check is added to be safe. There are other usages of mCurrentViewId that should ideally be guarded by null-checks, but those shall be fixed separately (or refactored later). Test: manual - (1) add a Thread.sleep at line 3146, (2) tap on url bar to trigger Augmented request, (3) close keyboard to trigger the NPE. Test: atest InlineLoginActivityTest InlineAugmentedLoginActivityTest Fix: 153877905 Change-Id: Ibcf8f17417ec7a3fa854816783b63879c4d18669 --- .../com/android/server/autofill/Session.java | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/services/autofill/java/com/android/server/autofill/Session.java b/services/autofill/java/com/android/server/autofill/Session.java index 9d1ad4239a246..b27c5d54a6fb3 100644 --- a/services/autofill/java/com/android/server/autofill/Session.java +++ b/services/autofill/java/com/android/server/autofill/Session.java @@ -717,10 +717,11 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState Consumer inlineSuggestionsRequestConsumer = mAssistReceiver.newAutofillRequestLocked(/*isInlineRequest=*/ true); if (inlineSuggestionsRequestConsumer != null) { + final AutofillId focusedId = mCurrentViewId; remoteRenderService.getInlineSuggestionsRendererInfo( new RemoteCallback((extras) -> { mInlineSessionController.onCreateInlineSuggestionsRequestLocked( - mCurrentViewId, inlineSuggestionsRequestConsumer, extras); + focusedId, inlineSuggestionsRequestConsumer, extras); } )); } @@ -2786,6 +2787,12 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState */ private boolean requestShowInlineSuggestionsLocked(@NonNull FillResponse response, @Nullable String filterText) { + if (mCurrentViewId == null) { + Log.w(TAG, "requestShowInlineSuggestionsLocked(): no view currently focused"); + return false; + } + final AutofillId focusedId = mCurrentViewId; + final Optional inlineSuggestionsRequest = mInlineSessionController.getInlineSuggestionsRequestLocked(); if (!inlineSuggestionsRequest.isPresent()) { @@ -2800,17 +2807,17 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState return false; } - final ViewState currentView = mViewStates.get(mCurrentViewId); + final ViewState currentView = mViewStates.get(focusedId); if ((currentView.getState() & ViewState.STATE_INLINE_DISABLED) != 0) { response.getDatasets().clear(); } InlineSuggestionsResponse inlineSuggestionsResponse = InlineSuggestionFactory.createInlineSuggestionsResponse( - inlineSuggestionsRequest.get(), response, filterText, mCurrentViewId, + inlineSuggestionsRequest.get(), response, filterText, focusedId, this, () -> { synchronized (mLock) { mInlineSessionController.hideInlineSuggestionsUiLocked( - mCurrentViewId); + focusedId); } }, remoteRenderService); if (inlineSuggestionsResponse == null) { @@ -2818,7 +2825,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState return false; } - return mInlineSessionController.onInlineSuggestionsResponseLocked(mCurrentViewId, + return mInlineSessionController.onInlineSuggestionsResponseLocked(focusedId, inlineSuggestionsResponse); } @@ -3107,19 +3114,19 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState remoteService.getComponentName().getPackageName()); mAugmentedRequestsLogs.add(log); - final AutofillId focusedId = AutofillId.withoutSession(mCurrentViewId); + final AutofillId focusedId = mCurrentViewId; final Consumer requestAugmentedAutofill = (inlineSuggestionsRequest) -> { remoteService.onRequestAutofillLocked(id, mClient, taskId, mComponentName, - focusedId, + AutofillId.withoutSession(focusedId), currentValue, inlineSuggestionsRequest, /*inlineSuggestionsCallback=*/ response -> { synchronized (mLock) { return mInlineSessionController .onInlineSuggestionsResponseLocked( - mCurrentViewId, response); + focusedId, response); } }, /*onErrorCallback=*/ () -> { @@ -3144,7 +3151,7 @@ final class Session implements RemoteFillService.FillServiceCallbacks, ViewState remoteRenderService.getInlineSuggestionsRendererInfo(new RemoteCallback( (extras) -> { mInlineSessionController.onCreateInlineSuggestionsRequestLocked( - mCurrentViewId, /*requestConsumer=*/ requestAugmentedAutofill, + focusedId, /*requestConsumer=*/ requestAugmentedAutofill, extras); }, mHandler)); } else {