Bug 34729 – [Qt] Exposing an QVariantMap containing QObjectStar to Javascript causes Segmentation Fault

• Component: JavaScriptCore
• Status: CLOSED FIXED

[Bruno Schmidt]

Created attachment 48370 [details]
Patch fixing the bug

If an QVariantMap containing QObjectStar is added to the to QtWebkit Javascript, it's use causes Segmentation Fault.
It happens because, in the case QMetaType::QVariantMap, the "root" object that is inside of a PassRefPtr is passed recursively inside a loop to recover the content of the map, but the PassRefPtr semantics prohibit its use inside a loop, so the "root" object mus be passed using the method "PassRefPtr::get" in order to keep the current reference.

[Tor Arne Vestbø]

Please follow the QtWebKit bug reporting guidelines when reporting bugs.

See http://trac.webkit.org/wiki/QtWebKitBugs

Specifically:

  - The 'QtWebKit' component should only be used for bugs/features in the
    public QtWebKit API layer, not to signify that the bug is specific to
    the Qt port of WebKit

      http://trac.webkit.org/wiki/QtWebKitBugs#Component

  - Add the keyword 'Qt' to signal that it's a Qt-related bug

      http://trac.webkit.org/wiki/QtWebKitBugs#Keywords

[Robert Hogan]

Hi Bruno,

You need to add a Changelog etc. for this. And also a test case or layout test. You then need to put the r=? flag on the revised patch so a reviewer will know to look at your patch.

See also :

http://webkit.org/coding/contributing.html

and

http://webkit.org/quality/reporting.html

Good spot though!

[Bruno Schmidt]

Created attachment 52919 [details]
Patch fixing the bug with unit tests and changelog

[Bruno Schmidt]

(In reply to comment #2)
> Hi Bruno,
> 
> You need to add a Changelog etc. for this. And also a test case or layout test.
> You then need to put the r=? flag on the revised patch so a reviewer will know
> to look at your patch.
> 
> See also :
> 
> http://webkit.org/coding/contributing.html
> 
> and
> 
> http://webkit.org/quality/reporting.html
> 
> Good spot though!

I think everything is in shape now, any news about the merge?

[Robert Hogan]

(In reply to comment #4)
> I think everything is in shape now, any news about the merge?

You're patch is waiting in the list for review!

https://bugs.webkit.org/request.cgi?action=queue&requester=&product=&type=all&requestee=&component=&group=type

[Bruno Schmidt]

Changing the component related to the bug.
It is actually more related to a Qt JavaScriptGlue bug than to a generic WebKit bug.

[Simon Hausmann]

A crash fix worth a release cherry-pick IMHO.

[WebKit Commit Bot]

Comment on attachment 52919 [details]
Patch fixing the bug with unit tests and changelog

Rejecting patch 52919 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Simon Hausmann', u'--force']" exit_code: 1
Last 500 characters of output:
 lines).
1 out of 2 hunks FAILED -- saving rejects to file WebCore/bridge/qt/qt_runtime.cpp.rej
patching file WebKit/qt/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebKit/qt/tests/qwebframe/tst_qwebframe.cpp
Hunk #2 FAILED at 105.
Hunk #3 succeeded at 137 (offset 2 lines).
Hunk #4 succeeded at 496 (offset 11 lines).
Hunk #5 succeeded at 773 (offset 13 lines).
Hunk #6 FAILED at 2849.
2 out of 6 hunks FAILED -- saving rejects to file WebKit/qt/tests/qwebframe/tst_qwebframe.cpp.rej

Full output: http://webkit-commit-queue.appspot.com/results/1848040

[Bruno Schmidt]

Created attachment 54067 [details]
Rebased version of the previous patch to fix the Commit Bot error

[WebKit Commit Bot]

Comment on attachment 54067 [details]
Rebased version of the previous patch to fix the Commit Bot error

Clearing flags on attachment: 54067

Committed r58250: <http://trac.webkit.org/changeset/58250>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Hausmann]

Revision r58250 cherry-picked into qtwebkit-2.0 with commit f737ab24aab5cb171853390f3dad549823ac185c

[Darin Adler]

Moving all JavaScriptGlue bugs to JavaScriptCore. The JavaScriptGlue framework itself is long gone. And most of the more recent bugs put in this component were put there by people who thought this was for some other aspect of “JavaScript glue” and have nothing to do with the actual original reason for the existence of this component, which was an OS-X-only framework named JavaScriptGlue.