Commit 4c5f7cf6 authored by Masayuki Nakano's avatar Masayuki Nakano
Browse files

Bug 1825693 - Make `ContentCache` stop assigning invalid data r=m_kato,nika 

There are some crash reports crashed in TSF module which may be caused by
passing invalid selection range (e.g., out of bounds of text).  However,
the cache is created in the child process and that causes the invalid cache
creation does not appear in the crash reports.  Therefore, let's try to
crash as soon as possible if `ContentCache` has invalid data.

Note that this does not detect all of the invalid cases because it's hard to
(re-)understand the edge cases.  Therefore, this tries to detect the cases
checked in `ContentCacheInParent::HandleQueryContentEvent` (*1) and some other
obvious odd cases.

1. https://searchfox.org/mozilla-central/rev/0ffaecaa075887ab07bf4c607c61ea2faa81b172/widget/ContentCache.cpp#776-778

Differential Revision: https://phabricator.services.mozilla.com/D176747
parent 163c90b2

dom/ipc/BrowserParent.cpp

+18 −2
Original line number Diff line number Diff line
@@ -2303,7 +2303,9 @@ mozilla::ipc::IPCResult BrowserParent::RecvNotifyIMEFocus( 
    aResolve(IMENotificationRequests());

    return IPC_OK();

  }



  if (NS_WARN_IF(!aContentCache.IsValid())) {

    return IPC_FAIL(this, "Invalid content cache data");

  }

  mContentCache.AssignContent(aContentCache, widget, &aIMENotification);

  IMEStateManager::NotifyIME(aIMENotification, widget, this);
@@ -2323,6 +2325,9 @@ mozilla::ipc::IPCResult BrowserParent::RecvNotifyIMETextChange( 
  if (!widget || !IMEStateManager::DoesBrowserParentHaveIMEFocus(this)) {

    return IPC_OK();

  }

  if (NS_WARN_IF(!aContentCache.IsValid())) {

    return IPC_FAIL(this, "Invalid content cache data");

  }

  mContentCache.AssignContent(aContentCache, widget, &aIMENotification);

  mContentCache.MaybeNotifyIME(widget, aIMENotification);

  return IPC_OK();
@@ -2335,6 +2340,9 @@ mozilla::ipc::IPCResult BrowserParent::RecvNotifyIMECompositionUpdate( 
  if (!widget || !IMEStateManager::DoesBrowserParentHaveIMEFocus(this)) {

    return IPC_OK();

  }

  if (NS_WARN_IF(!aContentCache.IsValid())) {

    return IPC_FAIL(this, "Invalid content cache data");

  }

  mContentCache.AssignContent(aContentCache, widget, &aIMENotification);

  mContentCache.MaybeNotifyIME(widget, aIMENotification);

  return IPC_OK();
@@ -2347,6 +2355,9 @@ mozilla::ipc::IPCResult BrowserParent::RecvNotifyIMESelection( 
  if (!widget || !IMEStateManager::DoesBrowserParentHaveIMEFocus(this)) {

    return IPC_OK();

  }

  if (NS_WARN_IF(!aContentCache.IsValid())) {

    return IPC_FAIL(this, "Invalid content cache data");

  }

  mContentCache.AssignContent(aContentCache, widget, &aIMENotification);

  mContentCache.MaybeNotifyIME(widget, aIMENotification);

  return IPC_OK();
@@ -2358,7 +2369,9 @@ mozilla::ipc::IPCResult BrowserParent::RecvUpdateContentCache( 
  if (!widget || !IMEStateManager::DoesBrowserParentHaveIMEFocus(this)) {

    return IPC_OK();

  }



  if (NS_WARN_IF(!aContentCache.IsValid())) {

    return IPC_FAIL(this, "Invalid content cache data");

  }

  mContentCache.AssignContent(aContentCache, widget);

  return IPC_OK();

}
@@ -2382,6 +2395,9 @@ mozilla::ipc::IPCResult BrowserParent::RecvNotifyIMEPositionChange( 
  if (!widget || !IMEStateManager::DoesBrowserParentHaveIMEFocus(this)) {

    return IPC_OK();

  }

  if (NS_WARN_IF(!aContentCache.IsValid())) {

    return IPC_FAIL(this, "Invalid content cache data");

  }

  mContentCache.AssignContent(aContentCache, widget, &aIMENotification);

  mContentCache.MaybeNotifyIME(widget, aIMENotification);

  return IPC_OK();

widget/ContentCache.cpp

+65 −11
Original line number Diff line number Diff line
@@ -12,6 +12,7 @@ 
#include "IMEData.h"

#include "TextEvents.h"



#include "mozilla/Assertions.h"

#include "mozilla/IMEStateManager.h"

#include "mozilla/IntegerPrintfMacros.h"

#include "mozilla/Logging.h"
@@ -41,6 +42,40 @@ static const char* GetNotificationName(const IMENotification* aNotification) { 


LazyLogModule sContentCacheLog("ContentCacheWidgets");



bool ContentCache::IsValid() const {

  if (mText.isNothing()) {

    // mSelection and mCaret depend on mText.

    if (NS_WARN_IF(mSelection.isSome()) || NS_WARN_IF(mCaret.isSome())) {

      return false;

    }

  } else {

    // mSelection depends on mText.

    if (mSelection.isSome() && (NS_WARN_IF(mText.isNothing()) ||

                                NS_WARN_IF(!mSelection->IsValidIn(*mText)))) {

      return false;

    }



    // mCaret depends on mSelection.

    if (mCaret.isSome() &&

        (NS_WARN_IF(mSelection.isNothing()) ||

         NS_WARN_IF(!mSelection->mHasRange) ||

         NS_WARN_IF(mSelection->StartOffset() != mCaret->Offset()))) {

      return false;

    }

  }



  // mTextRectArray stores character rects around composition string.

  // Note that even if we fail to collect the rects, we may keep storing

  // mCompositionStart.

  if (mTextRectArray.isSome()) {

    if (NS_WARN_IF(mCompositionStart.isNothing())) {

      return false;

    }

  }



  return true;

}



/*****************************************************************************

 * mozilla::ContentCacheInChild

 *****************************************************************************/
@@ -113,17 +148,24 @@ bool ContentCacheInChild::CacheAll(nsIWidget* aWidget, 


  const bool textCached = CacheText(aWidget, aNotification);

  const bool editorRectCached = CacheEditorRect(aWidget, aNotification);

  MOZ_DIAGNOSTIC_ASSERT(IsValid());

  return textCached || editorRectCached;

}



bool ContentCacheInChild::CacheSelection(nsIWidget* aWidget,

                                         const IMENotification* aNotification) {

  MOZ_LOG(sContentCacheLog, LogLevel::Info,

          ("0x%p CacheSelection(aWidget=0x%p, aNotification=%s)", this, aWidget,

           GetNotificationName(aNotification)));

  MOZ_LOG(

      sContentCacheLog, LogLevel::Info,

      ("0x%p CacheSelection(aWidget=0x%p, aNotification=%s), mText=%s", this,

       aWidget, GetNotificationName(aNotification),

       PrintStringDetail(mText, PrintStringDetail::kMaxLengthForEditor).get()));



  mSelection.reset();



  if (mText.isNothing()) {

    return false;

  }



  nsEventStatus status = nsEventStatus_eIgnore;

  WidgetQueryContentEvent querySelectedTextEvent(true, eQuerySelectedText,

                                                 aWidget);
@@ -133,6 +175,8 @@ bool ContentCacheInChild::CacheSelection(nsIWidget* aWidget, 
        sContentCacheLog, LogLevel::Error,

        ("0x%p CacheSelection(), FAILED, couldn't retrieve the selected text",

         this));

    // XXX Allowing selection-independent character rects makes things

    //     complicated in the parent...

  }

  // ContentCache should store only editable content.  Therefore, if current

  // selection root is not editable, we don't need to store the selection, i.e.,
@@ -140,12 +184,12 @@ bool ContentCacheInChild::CacheSelection(nsIWidget* aWidget, 
  // previously editable text, let's store the selection even if it becomes

  // uneditable because not doing so would create odd situation.  E.g., IME may

  // fail only querying selection after succeeded querying text.

  else if (NS_WARN_IF(mText.isNothing() &&

                      !querySelectedTextEvent.mReply->mIsEditableContent)) {

  else if (NS_WARN_IF(!querySelectedTextEvent.mReply->mIsEditableContent)) {

    MOZ_LOG(sContentCacheLog, LogLevel::Error,

            ("0x%p CacheSelection(), FAILED, editable content had already been "

             "blurred",

             this));

    MOZ_DIAGNOSTIC_ASSERT(IsValid());

    return false;

  } else {

    mSelection.emplace(querySelectedTextEvent);
@@ -153,6 +197,7 @@ bool ContentCacheInChild::CacheSelection(nsIWidget* aWidget, 


  const bool caretCached = CacheCaret(aWidget, aNotification);

  const bool textRectsCached = CacheTextRects(aWidget, aNotification);

  MOZ_DIAGNOSTIC_ASSERT(IsValid());

  return caretCached || textRectsCached || querySelectedTextEvent.Succeeded();

}
@@ -188,6 +233,7 @@ bool ContentCacheInChild::CacheCaret(nsIWidget* aWidget, 
  MOZ_LOG(sContentCacheLog, LogLevel::Info,

          ("0x%p   CacheCaret(), Succeeded, mSelection=%s, mCaret=%s", this,

           ToString(mSelection).c_str(), ToString(mCaret).c_str()));

  MOZ_DIAGNOSTIC_ASSERT(IsValid());

  return true;

}
@@ -278,11 +324,11 @@ bool ContentCacheInChild::CacheText(nsIWidget* aWidget, 
  // If we fail to get editable text content, it must mean that there is no

  // focused element anymore or focused element is not editable.  In this case,

  // we should not get selection of non-editable content

  if (MOZ_UNLIKELY(queryTextContentEvent.Failed() ||

                   !queryTextContentEvent.mReply->mIsEditableContent)) {

  if (MOZ_UNLIKELY(mText.isNothing())) {

    mSelection.reset();

    mCaret.reset();

    mTextRectArray.reset();

    MOZ_DIAGNOSTIC_ASSERT(IsValid());

    return false;

  }
@@ -564,10 +610,11 @@ bool ContentCacheInChild::CacheTextRects(nsIWidget* aWidget, 
       ToString(mTextRectArray).c_str(), ToString(mSelection).c_str(),

       ToString(mFirstCharRect).c_str(),

       ToString(mLastCommitStringTextRectArray).c_str()));

  MOZ_DIAGNOSTIC_ASSERT(IsValid());

  return true;

}



void ContentCacheInChild::SetSelection(

bool ContentCacheInChild::SetSelection(

    nsIWidget* aWidget,

    const IMENotification::SelectionChangeDataBase& aSelectionChangeData) {

  MOZ_LOG(
@@ -576,6 +623,10 @@ void ContentCacheInChild::SetSelection( 
       ToString(aSelectionChangeData).c_str(),

       PrintStringDetail(mText, PrintStringDetail::kMaxLengthForEditor).get()));



  if (MOZ_UNLIKELY(mText.isNothing())) {

    return false;

  }



  mSelection = Some(Selection(aSelectionChangeData));



  if (mLastCommit.isSome()) {
@@ -594,6 +645,8 @@ void ContentCacheInChild::SetSelection( 


  CacheCaret(aWidget);

  CacheTextRects(aWidget);



  return mSelection.isSome();

}



/*****************************************************************************
@@ -614,6 +667,8 @@ ContentCacheInParent::ContentCacheInParent(BrowserParent& aBrowserParent) 
void ContentCacheInParent::AssignContent(const ContentCache& aOther,

                                         nsIWidget* aWidget,

                                         const IMENotification* aNotification) {

  MOZ_DIAGNOSTIC_ASSERT(aOther.IsValid());



  mText = aOther.mText;

  mSelection = aOther.mSelection;

  mFirstCharRect = aOther.mFirstCharRect;
@@ -773,9 +828,8 @@ bool ContentCacheInParent::HandleQueryContentEvent( 
                 this));

        return false;

      }

      MOZ_DIAGNOSTIC_ASSERT_IF(!mSelection->IsCollapsed(), mText.isSome());

      MOZ_DIAGNOSTIC_ASSERT_IF(!mSelection->IsCollapsed(),

                               mSelection->EndOffset() <= mText->Length());

      MOZ_DIAGNOSTIC_ASSERT(mText.isSome());

      MOZ_DIAGNOSTIC_ASSERT(mSelection->IsValidIn(*mText));

      aEvent.EmplaceReply();

      aEvent.mReply->mFocusedWidget = aWidget;

      if (mSelection->mHasRange) {

widget/ContentCache.h

+14 −1
Original line number Diff line number Diff line
@@ -45,6 +45,8 @@ class ContentCache { 


  ContentCache() = default;



  [[nodiscard]] bool IsValid() const;



 protected:

  // Whole text in the target

  Maybe<nsString> mText;
@@ -90,6 +92,11 @@ class ContentCache { 
      }

    }



    [[nodiscard]] bool IsValidIn(const nsAString& aText) const {

      return !mHasRange ||

             (mAnchor <= aText.Length() && mFocus <= aText.Length());

    }



    explicit Selection(const WidgetQueryContentEvent& aQuerySelectedTextEvent);



    void ClearRects() {
@@ -194,6 +201,10 @@ class ContentCache { 
    uint32_t Offset() const { return mOffset; }

    bool HasRect() const { return !mRect.IsEmpty(); }



    [[nodiscard]] bool IsValidIn(const nsAString& aText) const {

      return mOffset <= aText.Length();

    }



    friend std::ostream& operator<<(std::ostream& aStream,

                                    const Caret& aCaret) {

      aStream << "{ mOffset=" << aCaret.mOffset;
@@ -332,8 +343,10 @@ class ContentCacheInChild final : public ContentCache { 
  /**

   * SetSelection() modifies selection with specified raw data. And also this

   * tries to retrieve text rects too.

   *

   * @return true if the selection is cached.  Otherwise, false.

   */

  void SetSelection(

  [[nodiscard]] bool SetSelection(

      nsIWidget* aWidget,

      const IMENotification::SelectionChangeDataBase& aSelectionChangeData);

widget/PuppetWidget.cpp

+7 −1
Original line number Diff line number Diff line
@@ -833,7 +833,13 @@ nsresult PuppetWidget::NotifyIMEOfSelectionChange( 


  // Note that selection change must be notified after text change if it occurs.

  // Therefore, we don't need to query text content again here.

  mContentCache.SetSelection(this, aIMENotification.mSelectionChangeData);

  if (MOZ_UNLIKELY(!mContentCache.SetSelection(

          this, aIMENotification.mSelectionChangeData))) {

    // If there is no text cache yet, caching text will cache selection too.

    // Therefore, in the case, we don't need to notify IME of selection change

    // right now.

    return NS_OK;

  }



  mBrowserChild->SendNotifyIMESelection(mContentCache, aIMENotification);