From a875271fccd3ff8a4c865163efcee3fa62b15da3 Mon Sep 17 00:00:00 2001 From: Mihai Popa Date: Mon, 12 Mar 2018 19:30:22 +0000 Subject: [PATCH] Check against null text in TextView constructor It is common for TextView subclasses to override the #setText method, and there is nothing preventing the new implementations from leaving mText and mTransformed unchanged. On the other hand, TextView assumes mText and mTransformed will be non null at all times after the first overridden implementations, these can remain null, causing null pointer exceptions later. Bug: 74411872 Test: atest FrameworksCoreTests:android.widget.TextViewTest Test: atest FrameworksCoreTests:android.widget.TextViewActivityTest Test: atest CtsWidgetTestCases:android.widget.cts.TextViewTest Change-Id: Ifaddb46d156c495a371789c6f32cfd67ffaaaef2 --- .../text/method/TransformationMethod.java | 2 ++ core/java/android/widget/TextView.java | 29 ++++++++++++++++ .../src/android/widget/TextViewTest.java | 33 +++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/core/java/android/text/method/TransformationMethod.java b/core/java/android/text/method/TransformationMethod.java index b542109d0beb9..8f3b334abbbda 100644 --- a/core/java/android/text/method/TransformationMethod.java +++ b/core/java/android/text/method/TransformationMethod.java @@ -32,6 +32,8 @@ public interface TransformationMethod * Beware that the returned text must be exactly the same length as * the source text, and that if the source text is Editable, the returned * text must mirror it dynamically instead of doing a one-time copy. + * The method should not return {@code null} unless {@code source} + * is {@code null}. */ public CharSequence getTransformation(CharSequence source, View view); diff --git a/core/java/android/widget/TextView.java b/core/java/android/widget/TextView.java index 4a2c2c50a7835..a538f7fa3c875 100644 --- a/core/java/android/widget/TextView.java +++ b/core/java/android/widget/TextView.java @@ -1508,6 +1508,13 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener } setText(text, bufferType); + if (mText == null) { + mText = ""; + } + if (mTransformed == null) { + mTransformed = ""; + } + if (textIsSetFromXml) { mTextSetFromXmlOrResourceId = true; } @@ -2191,6 +2198,14 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener return (mText instanceof Editable) ? (Editable) mText : null; } + /** + * @hide + */ + @VisibleForTesting + public CharSequence getTransformed() { + return mTransformed; + } + /** * Gets the vertical distance between lines of text, in pixels. * Note that markup within the text can cause individual lines @@ -5532,6 +5547,11 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener * PrecomputedText mismatches with this TextView, IllegalArgumentException is thrown. To ensure * the parameters match, you can call {@link TextView#setTextMetricsParams} before calling this. * + * Subclasses overriding this method should ensure that the following post condition holds, + * in order to guarantee the safety of the view's measurement and layout operations: + * regardless of the input, after calling #setText both {@code mText} and {@code mTransformed} + * will be different from {@code null}. + * * @param text text to be displayed * * @attr ref android.R.styleable#TextView_text @@ -5554,6 +5574,11 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener * {@link android.text.Editable.Factory} to create final or intermediate * {@link Editable Editables}. * + * Subclasses overriding this method should ensure that the following post condition holds, + * in order to guarantee the safety of the view's measurement and layout operations: + * regardless of the input, after calling #setText both {@code mText} and {@code mTransformed} + * will be different from {@code null}. + * * @param text text to be displayed * * @see #setText(CharSequence) @@ -5706,6 +5731,10 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener } else { mTransformed = mTransformation.getTransformation(text, this); } + if (mTransformed == null) { + // Should not happen if the transformation method follows the non-null postcondition. + mTransformed = ""; + } final int textLength = text.length(); diff --git a/core/tests/coretests/src/android/widget/TextViewTest.java b/core/tests/coretests/src/android/widget/TextViewTest.java index 4f1efbfed5f70..1c5610b1bc7da 100644 --- a/core/tests/coretests/src/android/widget/TextViewTest.java +++ b/core/tests/coretests/src/android/widget/TextViewTest.java @@ -23,6 +23,7 @@ import static junit.framework.Assert.assertTrue; import android.app.Activity; import android.app.Instrumentation; +import android.content.Context; import android.content.Intent; import android.graphics.Paint; import android.platform.test.annotations.Presubmit; @@ -44,6 +45,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import java.lang.reflect.Field; import java.util.Locale; /** @@ -320,6 +322,15 @@ public class TextViewTest { assertTrue(mTextView.useDynamicLayout()); } + @Test + @UiThreadTest + public void testConstructor_doesNotLeaveTextNull() { + mTextView = new NullSetTextTextView(mActivity); + // Check that mText and mTransformed are empty string instead of null. + assertEquals("", mTextView.getText().toString()); + assertEquals("", mTextView.getTransformed().toString()); + } + private String createLongText() { int size = 600 * 1000; final StringBuilder builder = new StringBuilder(size); @@ -328,4 +339,26 @@ public class TextViewTest { } return builder.toString(); } + + private class NullSetTextTextView extends TextView { + NullSetTextTextView(Context context) { + super(context); + } + + @Override + public void setText(CharSequence text, BufferType type) { + // #setText will be called from the TextView constructor. Here we reproduce + // the situation when the method sets mText and mTransformed to null. + try { + final Field textField = TextView.class.getDeclaredField("mText"); + textField.setAccessible(true); + textField.set(this, null); + final Field transformedField = TextView.class.getDeclaredField("mTransformed"); + transformedField.setAccessible(true); + transformedField.set(this, null); + } catch (NoSuchFieldException | IllegalAccessException e) { + // Empty. + } + } + } }