| Summary: | [patch] Crash in KDirModelPrivate::nodeForUrl() if path contains two or more consecutive slashes | ||
|---|---|---|---|
| Product: | [Unmaintained] kio | Reporter: | Jonathan Thomas <echidnaman> |
| Component: | general | Assignee: | David Faure <faure> |
| Status: | RESOLVED FIXED | ||
| Severity: | crash | CC: | frank78ac |
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | unspecified | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: |
Patch (including unit test)
updated patch |
||
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);
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?
Thanks for the patch. However QRegExp is awfully slow, I don't want it in KDirModel :-) How about KUrl::cleanPath()? 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).
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 |
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 ()