Bug 173927 - [patch] Crash in KDirModelPrivate::nodeForUrl() if path contains two or more consecutive slashes
Summary: [patch] Crash in KDirModelPrivate::nodeForUrl() if path contains two or more ...
Status: RESOLVED FIXED
Alias: None
Product: kio
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: unspecified Linux
: NOR crash
Target Milestone: ---
Assignee: David Faure
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-30 21:27 UTC by Jonathan Thomas
Modified: 2008-11-05 00:59 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch (including unit test) (2.68 KB, patch)
2008-11-01 14:39 UTC, Frank Reininghaus
Details
updated patch (2.18 KB, patch)
2008-11-05 00:09 UTC, Frank Reininghaus
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Thomas 2008-10-30 21:27:12 UTC
Version:           1.1 (using 4.1.2 (KDE 4.1.2), Kubuntu packages)
Compiler:          cc
OS:                Linux (i686) release 2.6.26-5-generic

Steps to reproduce:
-Enable the Folders panel.
-Navigate to a URL, but insert one more slash than usual (ex. /home//jonathan)

Without the Folders panel, Dolphin navigates to the folder just fine, but with the Folder panel enabled, Dolphin crashes with the following backtrace:

Application: Dolphin (dolphin), signal SIGABRT
[Thread debugging using libthread_db enabled]
[New Thread 0xb5fce6c0 (LWP 15848)]
[KCrash handler]
#6  0xb80f2424 in __kernel_vsyscall ()
#7  0xb685c880 in raise () from /lib/tls/i686/cmov/libc.so.6
#8  0xb685e248 in abort () from /lib/tls/i686/cmov/libc.so.6
#9  0xb763c795 in qt_message_output () from /usr/lib/libQtCore.so.4
#10 0xb763c872 in qFatal () from /usr/lib/libQtCore.so.4
#11 0xb763c915 in qt_assert () from /usr/lib/libQtCore.so.4
#12 0xb7e9af68 in KDirModelPrivate::nodeForUrl (this=0x997cdb0, 
    _url=@0x981629c, returnLastParent=false)
    at /build/buildd/kde4libs-4.1.2/kio/kio/kdirmodel.cpp:187
#13 0xb7e9ede7 in KDirModel::indexForUrl (this=0x96936f0, url=@0x981629c)
    at /build/buildd/kde4libs-4.1.2/kio/kio/kdirmodel.cpp:705
#14 0x0808cef9 in TreeViewSidebarPage::loadSubTree (this=0x9816268)
    at /build/buildd/kdebase-4.1.2/apps/dolphin/src/treeviewsidebarpage.cpp:250
#15 0x0808d00c in TreeViewSidebarPage::loadTree (this=0x9816268, 
    url=@0xbfdf260c)
    at /build/buildd/kdebase-4.1.2/apps/dolphin/src/treeviewsidebarpage.cpp:294
#16 0x0808d640 in TreeViewSidebarPage::qt_metacall (this=0x9816268, 
    _c=QMetaObject::InvokeMetaMethod, _id=3, _a=0xbfdf23e0)
    at /build/buildd/kdebase-4.1.2/obj-i486-linux-gnu/apps/dolphin/src/treeviewsidebarpage.moc:90
#17 0xb7749a60 in QMetaObject::activate () from /usr/lib/libQtCore.so.4
#18 0xb774a7e2 in QMetaObject::activate () from /usr/lib/libQtCore.so.4
#19 0x08066f85 in DolphinMainWindow::urlChanged (this=0x96aa148, 
    _t1=@0xbfdf260c)
    at /build/buildd/kdebase-4.1.2/obj-i486-linux-gnu/apps/dolphin/src/dolphinmainwindow.moc:244
#20 0x0806a80e in DolphinMainWindow::changeUrl (this=0x96aa148, 
    url=@0xbfdf260c)
    at /build/buildd/kdebase-4.1.2/apps/dolphin/src/dolphinmainwindow.cpp:184
#21 0x08071de4 in DolphinMainWindow::qt_metacall (this=0x96aa148, 
    _c=QMetaObject::InvokeMetaMethod, _id=6, _a=0xbfdf251c)
    at /build/buildd/kdebase-4.1.2/obj-i486-linux-gnu/apps/dolphin/src/dolphinmainwindow.moc:160
#22 0xb7749a60 in QMetaObject::activate () from /usr/lib/libQtCore.so.4
#23 0xb774a7e2 in QMetaObject::activate () from /usr/lib/libQtCore.so.4
#24 0xb75db713 in KUrlNavigator::urlChanged (this=0x976bfd0, _t1=@0xbfdf260c)
    at /build/buildd/kde4libs-4.1.2/obj-i486-linux-gnu/kfile/kurlnavigator.moc:137
#25 0xb75dfa22 in KUrlNavigator::setUrl (this=0x976bfd0, url=@0xbfdf26fc)
    at /build/buildd/kde4libs-4.1.2/kfile/kurlnavigator.cpp:1074
#26 0xb75e0b51 in KUrlNavigator::Private::slotReturnPressed (this=0x976c620, 
    text=@0xbfdf2748)
    at /build/buildd/kde4libs-4.1.2/kfile/kurlnavigator.cpp:397
#27 0xb75e0d73 in KUrlNavigator::Private::slotReturnPressed (this=0x976c620)
    at /build/buildd/kde4libs-4.1.2/kfile/kurlnavigator.cpp:415
#28 0xb75e136b in KUrlNavigator::qt_metacall (this=0x976bfd0, 
    _c=QMetaObject::InvokeMetaMethod, _id=12, _a=0xbfdf27f8)
    at /build/buildd/kde4libs-4.1.2/obj-i486-linux-gnu/kfile/kurlnavigator.moc:112
#29 0xb7749a60 in QMetaObject::activate () from /usr/lib/libQtCore.so.4
#30 0xb774a7e2 in QMetaObject::activate () from /usr/lib/libQtCore.so.4
#31 0xb7cab3a7 in KComboBox::returnPressed (this=0x9764708)
    at /build/buildd/kde4libs-4.1.2/obj-i486-linux-gnu/kdeui/kcombobox.moc:160
#32 0xb7cac873 in KComboBox::qt_metacall (this=0x9764708, 
    _c=QMetaObject::InvokeMetaMethod, _id=0, _a=0xbfdf2928)
    at /build/buildd/kde4libs-4.1.2/obj-i486-linux-gnu/kdeui/kcombobox.moc:104
#33 0xb7fb2e8a in KUrlComboBox::qt_metacall (this=0x9764708, 
    _c=QMetaObject::InvokeMetaMethod, _id=51, _a=0xbfdf2928)
    at /build/buildd/kde4libs-4.1.2/obj-i486-linux-gnu/kio/kurlcombobox.moc:69
#34 0xb7749a60 in QMetaObject::activate () from /usr/lib/libQtCore.so.4
#35 0xb774a7e2 in QMetaObject::activate () from /usr/lib/libQtCore.so.4
#36 0xb7072727 in QLineEdit::returnPressed () from /usr/lib/libQtGui.so.4
#37 0xb7ccf058 in KLineEdit::event (this=0x977b808, ev=0xbfdf310c)
    at /build/buildd/kde4libs-4.1.2/kdeui/widgets/klineedit.cpp:1267
#38 0xb6cd58ec in QApplicationPrivate::notify_helper ()
   from /usr/lib/libQtGui.so.4
#39 0xb6cde846 in QApplication::notify () from /usr/lib/libQtGui.so.4
#40 0xb7c0372d in KApplication::notify (this=0xbfdf3b64, receiver=0x977b808, 
    event=0xbfdf310c)
    at /build/buildd/kde4libs-4.1.2/kdeui/kernel/kapplication.cpp:311
#41 0xb7734e61 in QCoreApplication::notifyInternal ()
   from /usr/lib/libQtCore.so.4
#42 0xb7cae373 in KCompletionBox::eventFilter (this=0xa7e3200, o=0xa7e3200, 
    e=0xbfdf310c) at /usr/include/qt4/QtCore/qcoreapplication.h:209
#43 0xb77340f4 in QCoreApplicationPrivate::sendThroughApplicationEventFilters
    () from /usr/lib/libQtCore.so.4
#44 0xb6cd5863 in QApplicationPrivate::notify_helper ()
   from /usr/lib/libQtGui.so.4
#45 0xb6cde846 in QApplication::notify () from /usr/lib/libQtGui.so.4
#46 0xb7c0372d in KApplication::notify (this=0xbfdf3b64, receiver=0xa7e3200, 
    event=0xbfdf310c)
    at /build/buildd/kde4libs-4.1.2/kdeui/kernel/kapplication.cpp:311
#47 0xb7734e61 in QCoreApplication::notifyInternal ()
   from /usr/lib/libQtCore.so.4
#48 0xb6d3641e in ?? () from /usr/lib/libQtGui.so.4
#49 0xb6d6cbf0 in ?? () from /usr/lib/libQtGui.so.4
#50 0xb6d6ed66 in ?? () from /usr/lib/libQtGui.so.4
#51 0xb6d45b35 in QApplication::x11ProcessEvent () from /usr/lib/libQtGui.so.4
#52 0xb6d707ea in ?? () from /usr/lib/libQtGui.so.4
#53 0xb638a6f8 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#54 0xb638dda3 in ?? () from /usr/lib/libglib-2.0.so.0
#55 0xb638df61 in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#56 0xb775f478 in QEventDispatcherGlib::processEvents ()
   from /usr/lib/libQtCore.so.4
#57 0xb6d6fee5 in ?? () from /usr/lib/libQtGui.so.4
#58 0xb773352a in QEventLoop::processEvents () from /usr/lib/libQtCore.so.4
#59 0xb77336ea in QEventLoop::exec () from /usr/lib/libQtCore.so.4
#60 0xb7735da5 in QCoreApplication::exec () from /usr/lib/libQtCore.so.4
#61 0xb6cd5767 in QApplication::exec () from /usr/lib/libQtGui.so.4
#62 0x0808519f in main (argc=1, argv=0xbfdf3d14)
    at /build/buildd/kdebase-4.1.2/apps/dolphin/src/main.cpp:94
#0  0xb80f2424 in __kernel_vsyscall ()
Comment 1 Frank Reininghaus 2008-10-31 22:22:12 UTC
Confirmed in trunk rev. 878290. The problem is that KDirModelPrivate::nodeForUrl() does not handle paths with two or more consecutive slashes properly. The patch below fixes it for me. Maybe I should also write a simple unit test to make sure that there is no regression in the future...

Index: kio/kio/kdirmodel.cpp
===================================================================
--- kio/kio/kdirmodel.cpp       (revision 878290)
+++ kio/kio/kdirmodel.cpp       (working copy)
@@ -186,7 +186,7 @@
     if (url.protocol() != nodeUrl.protocol())
         return 0;

-    const QString pathStr = url.path(); // no trailing slash
+    const QString pathStr = url.path().replace(QRegExp("//+"), "/"); // pathStr has no trailing slash, but multiple slashes must be removed
     KDirModelDirNode* dirNode = m_rootNode;

     if (!pathStr.startsWith(nodeUrl.path())) {
@@ -203,7 +203,7 @@

         // E.g. pathStr is /a/b/c and nodePath is /a. We want to find the child "b" in dirNode.
         const QString relativePath = pathStr.mid(nodePath.length());
-        Q_ASSERT(!relativePath.startsWith('/')); // huh? we need double-slash simplification?
+        Q_ASSERT(!relativePath.startsWith('/')); // check if multiple slashes have been removed (see above)
         const int nextSlash = relativePath.indexOf('/');
         const QString fileName = relativePath.left(nextSlash); // works even if nextSlash==-1
         KDirModelNode* node = dirNode->m_childNodesByName.value(fileName);
Comment 2 Frank Reininghaus 2008-11-01 14:39:47 UTC
Created attachment 28261 [details]
Patch (including unit test)

Trying to write a unit test was a good idea - it turned out that additional work is needed to avoid that another assert is hit in some cases. The problem was that QUrl::operator == () does not consider "/a/b" and "/a//b" to be equal. This means that the comparison

if (nodeUrl == url)

failed, and instead of returning the node that was finally found, the following assert Q_ASSERT(isDir(node)) was hit.

David, can there actually be URLs where replacing multiple slashes by a single slash makes a difference, or should we report this to the Qt people?
Comment 3 David Faure 2008-11-04 10:19:05 UTC
Thanks for the patch. However QRegExp is awfully slow, I don't want it in KDirModel :-)

How about KUrl::cleanPath()?
Comment 4 Frank Reininghaus 2008-11-05 00:09:51 UTC
Created attachment 28334 [details]
updated patch

David, you're right, KUrl::cleanPath() looks certainly better, and it also ensures that KDirModelPrivate::nodeForUrl() works properly for paths that contain "/../". I'll try to remember to have a look at the API docs of the involved classes the next time before I come up with a hack of my own ;-)

I've changed the patch accordingly. It's a one-liner now (except for the suggested unit test).
Comment 5 David Faure 2008-11-05 00:59:02 UTC
SVN commit 880241 by dfaure:

Apply patch by Frank Reininghaus to fix "Crash in KDirModelPrivate::nodeForUrl() if path contains two or more consecutive slashes"
BUG: 173927
Will backport to 4.1 branch.


 M  +2 -1      kio/kdirmodel.cpp  
 M  +14 -0     tests/kdirmodeltest.cpp  
 M  +1 -0      tests/kdirmodeltest.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=880241