Bug 21830 – Invalid history entries could cause a crash in QT Webkit

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Yael]

When the application requests a history item that does not exist, it would cause a crash.

[Yael]

Created attachment 24602 [details]
Fix the crash on invalid history item.

Check for NULL HistoryItem whenever it is accessed.

[Simon Hausmann]

Comment on attachment 24602 [details]
Fix the crash on invalid history item.

> Index: WebKit/qt/tests/qwebpage/tst_qwebpage.cpp
> ===================================================================
> --- WebKit/qt/tests/qwebpage/tst_qwebpage.cpp	(revision 37814)
> +++ WebKit/qt/tests/qwebpage/tst_qwebpage.cpp	(working copy)
> @@ -294,6 +294,9 @@
>      QVERIFY(m_page->history()->canGoBack());
>      QVERIFY(!m_page->history()->canGoForward());
>      QCOMPARE(m_page->history()->count(), 2);
> +    QVERIFY(m_page->history()->backItem().isValid());
> +    QVERIFY(!m_page->history()->forwardItem().isValid());
> +
>      m_page->history()->back();
>      QVERIFY(::waitForSignal(m_view, SIGNAL(loadFinished(bool))));
>  
> Index: WebKit/qt/ChangeLog
> ===================================================================
> --- WebKit/qt/ChangeLog	(revision 37814)
> +++ WebKit/qt/ChangeLog	(working copy)
> @@ -1,3 +1,23 @@
> [phone]  Yael Aharon <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Invalid history entries could cause a crash in QT Webkit
> +
> +        * Api/qwebhistory.cpp:
> +        (QWebHistoryItem::originalUrl):
> +        (QWebHistoryItem::url):
> +        (QWebHistoryItem::title):
> +        (QWebHistoryItem::lastVisited):
> +        (QWebHistoryItem::icon):
> +        (QWebHistoryItem::isValid):
> +        * Api/qwebhistory.h:
> +        * Api/qwebhistory_p.h:
> +        (QWebHistoryItemPrivate::QWebHistoryItemPrivate):
> +        (QWebHistoryItemPrivate::~QWebHistoryItemPrivate):
> +        * tests/qwebpage/tst_qwebpage.cpp:
> +        (tst_QWebPage::modified):
> +
>  2008-10-22  Yael Aharon <[e-mail]>
>  
>          Reviewed by Simon Hausmann.
> Index: WebKit/qt/Api/qwebhistory_p.h
> ===================================================================
> --- WebKit/qt/Api/qwebhistory_p.h	(revision 37814)
> +++ WebKit/qt/Api/qwebhistory_p.h	(working copy)
> @@ -28,14 +28,15 @@
>  public:
>      QWebHistoryItemPrivate(WebCore::HistoryItem *i)
>      {
> -        i->ref();
> +        if (i)
> +            i->ref();
>          item = i;
>      }
>      ~QWebHistoryItemPrivate()
>      {
> -        item->deref();
> +        if (item)
> +            item->deref();
>      }
> -    
>      WebCore::HistoryItem *item;
>  };
>  
> Index: WebKit/qt/Api/qwebhistory.cpp
> ===================================================================
> --- WebKit/qt/Api/qwebhistory.cpp	(revision 37814)
> +++ WebKit/qt/Api/qwebhistory.cpp	(working copy)
> @@ -85,7 +85,9 @@
>  */
>  QUrl QWebHistoryItem::originalUrl() const
>  {
> -    return QUrl(d->item->originalURL().string());
> +    if (d->item)
> +        return QUrl(d->item->originalURL().string());
> +    return QUrl();
>  }
>  
>  
> @@ -96,7 +98,9 @@
>  */
>  QUrl QWebHistoryItem::url() const
>  {
> -    return QUrl(d->item->url().string());
> +    if (d->item)
> +        return QUrl(d->item->url().string());
> +    return QUrl();
>  }
>  
>  
> @@ -107,7 +111,9 @@
>  */
>  QString QWebHistoryItem::title() const
>  {
> -    return d->item->title();
> +    if (d->item)
> +        return d->item->title();
> +    return QString();
>  }
>  
>  
> @@ -119,7 +125,9 @@
>  QDateTime QWebHistoryItem::lastVisited() const
>  {
>      //FIXME : this will be wrong unless we correctly set lastVisitedTime ourselves
> -    return QDateTime::fromTime_t((uint)d->item->lastVisitedTime());
> +    if (d->item)
> +        return QDateTime::fromTime_t((uint)d->item->lastVisitedTime());
> +    return QDateTime();
>  }
>  
>  
> @@ -130,7 +138,9 @@
>  */
>  QIcon QWebHistoryItem::icon() const
>  {
> -    return *d->item->icon()->nativeImageForCurrentFrame();
> +    if (d->item)
> +        return *d->item->icon()->nativeImageForCurrentFrame();
> +    return QIcon();
>  }
>  
>  /*!
> @@ -142,6 +152,15 @@
>  }
>  
>  /*!
> +    \since 4.5
> +    Returns whether this is a valid history item.
> +*/
> +bool QWebHistoryItem::isValid() const
> +{
> +    return d->item;
> +}
> +
> +/*!
>    \class QWebHistory
>    \since 4.4
>    \brief The QWebHistory class represents the history of a QWebPage
> Index: WebKit/qt/Api/qwebhistory.h
> ===================================================================
> --- WebKit/qt/Api/qwebhistory.h	(revision 37814)
> +++ WebKit/qt/Api/qwebhistory.h	(working copy)
> @@ -46,6 +46,8 @@
>  
>      QIcon icon() const;
>  
> +    bool isValid() const;
> +
>  private:
>      QWebHistoryItem(QWebHistoryItemPrivate *priv);
>      friend class QWebHistory;

[Simon Hausmann]

Landed in r37843 :)