Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 11694b06 authored by Yohei Yukawa's avatar Yohei Yukawa
Browse files

Fix error swallowing in InputConnectionCommand unparcelling 

This is a follow up CL to my previous CL [1], which added
InputConnectionCommand class with parcelling support.

A Parcel object may contain multiple primitive and Parcelable values.
Neither shallowing exceptions nor early-returning null solves the
fundomental problem that any subsequent read from the same Parcel
object is at risk.

Just let the higher layer decide how to deal with it.

 [1]: I86eba7185b4b0664c1b0b3da794dfc5eeddc725c
      2757c248

Bug: 194151409
Fix: 194567417
Test: atest CtsInputMethodTestCases:InputConnectionEndToEndTest
Test: atest FrameworksCoreTests:InputConnectionCommandTest
Change-Id: Icd2b40e63771085cb8d88721fbdca7a089256b28
parent b72545e7

core/java/com/android/internal/inputmethod/InputConnectionCommand.java

+21 −21
Original line number Diff line number Diff line
@@ -23,12 +23,12 @@ import android.annotation.IntDef; 
import android.annotation.IntRange;

import android.annotation.NonNull;

import android.annotation.Nullable;

import android.os.BadParcelableException;

import android.os.Bundle;

import android.os.IBinder;

import android.os.Parcel;

import android.os.Parcelable;

import android.text.TextUtils;

import android.util.Log;

import android.view.KeyEvent;

import android.view.inputmethod.CompletionInfo;

import android.view.inputmethod.CorrectionInfo;
@@ -356,14 +356,24 @@ public final class InputConnectionCommand implements Parcelable { 
                | (mResultCallbackType != ResultCallbackType.NULL ? FieldMask.CALLBACK : 0);

    }





    /**

     * A utility method to unparcel {@link InputConnectionCommand} from the given {@link Parcel}.

     *

     * <p>When this method throws any {@link RuntimeException} or its derived class, notably

     * {@link BadParcelableException}, {@code source} is considered to be in an unexpected state and

     * unsafe to continue reading any subsequent data.</p>

     *

     * @param source {@link Parcel} to read the data from.

     * @return {@link InputConnectionCommand} that is parcelled from {@code source}.

     */

    @AnyThread

    @Nullable

    @NonNull

    private static InputConnectionCommand createFromParcel(@NonNull Parcel source) {

        final int type = source.readInt();

        if (type < InputConnectionCommandType.FIRST_COMMAND

                || InputConnectionCommandType.LAST_COMMAND < type) {

            Log.e(TAG, "Invalid InputConnectionCommand type=" + type);

            return null;

            throw new BadParcelableException("Invalid InputConnectionCommandType=" + type);

        }



        @FieldMask final int fieldMask = source.readInt();
@@ -380,9 +390,6 @@ public final class InputConnectionCommand implements Parcelable { 
        if ((fieldMask & FieldMask.PARCELABLE) != 0) {

            parcelableType = source.readInt();

            switch (parcelableType) {

                case ParcelableType.NULL:

                    Log.e(TAG, "Unexpected ParcelableType=NULL");

                    return null;

                case ParcelableType.EXTRACTED_TEXT_REQUEST:

                    parcelable = source.readTypedObject(ExtractedTextRequest.CREATOR);

                    break;
@@ -399,8 +406,8 @@ public final class InputConnectionCommand implements Parcelable { 
                    parcelable = source.readTypedObject(InputContentInfo.CREATOR);

                    break;

                default:

                    Log.e(TAG, "Unknown ParcelableType=" + parcelableType);

                    return null;

                    throw new BadParcelableException(

                            "Invalid InputConnectionCommand.ParcelableType=" + parcelableType);

            }

        } else {

            parcelableType = ParcelableType.NULL;
@@ -411,9 +418,6 @@ public final class InputConnectionCommand implements Parcelable { 
        if ((fieldMask & FieldMask.CALLBACK) != 0) {

            resultCallbackType = source.readInt();

            switch (resultCallbackType) {

                case ResultCallbackType.NULL:

                    Log.e(TAG, "Unexpected ResultCallbackType=NULL");

                    return null;

                case ResultCallbackType.BOOLEAN:

                case ResultCallbackType.INT:

                case ResultCallbackType.CHAR_SEQUENCE:
@@ -422,8 +426,9 @@ public final class InputConnectionCommand implements Parcelable { 
                    resultCallback = source.readStrongBinder();

                    break;

                default:

                    Log.e(TAG, "Unknown ResultCallbackType=" + resultCallbackType);

                    return null;

                    throw new BadParcelableException(

                            "Invalid InputConnectionCommand.ResultCallbackType="

                                    + resultCallbackType);

            }

        } else {

            resultCallbackType = ResultCallbackType.NULL;
@@ -439,15 +444,10 @@ public final class InputConnectionCommand implements Parcelable { 
    public static final Parcelable.Creator<InputConnectionCommand> CREATOR =

            new Parcelable.Creator<InputConnectionCommand>() {

                @AnyThread

                @Nullable

                @NonNull

                @Override

                public InputConnectionCommand createFromParcel(Parcel source) {

                    try {

                    return InputConnectionCommand.createFromParcel(source);

                    } catch (Exception e) {

                        Log.e(TAG, "Returning null due to exception.", e);

                        return null;

                    }

                }



                @AnyThread

core/tests/coretests/src/com/android/internal/inputmethod/InputConnectionCommandTest.java

0 → 100644
+47 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2021 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.internal.inputmethod;



import static org.junit.Assert.assertThrows;



import android.os.Parcel;

import android.platform.test.annotations.Presubmit;



import androidx.test.filters.SmallTest;

import androidx.test.runner.AndroidJUnit4;



import org.junit.Test;

import org.junit.runner.RunWith;



@SmallTest

@Presubmit

@RunWith(AndroidJUnit4.class)

public class InputConnectionCommandTest {

    @Test

    public void testCreateFromParcelDoesNotSwallowExceptions() {

        final Parcel parcel = Parcel.obtain();

        try {

            parcel.writeInt(InputConnectionCommandType.FIRST_COMMAND);

            parcel.writeInt(InputConnectionCommand.FieldMask.PARCELABLE);

            parcel.writeInt(InputConnectionCommand.ParcelableType.NULL);  // invalid

            assertThrows(RuntimeException.class,

                    () -> InputConnectionCommand.CREATOR.createFromParcel(parcel));

        } finally {

            parcel.recycle();

        }

    }

}