Bug 154819 - Entering applications:// crashes konqueror
Summary: Entering applications:// crashes konqueror
Status: RESOLVED FIXED
Alias: None
Product: kdelibs
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR crash
Target Milestone: ---
Assignee: kdelibs bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-12-29 17:08 UTC by Albert Astals Cid
Modified: 2008-01-04 19:01 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch for fixing the crash (1.05 KB, patch)
2007-12-30 02:27 UTC, Rafael Fernández López
Details
Patch improved for fixing the crash. (1.12 KB, patch)
2007-12-30 03:07 UTC, Rafael Fernández López
Details
Patch improved for fixing the crash (642 bytes, patch)
2007-12-30 16:24 UTC, Rafael Fernández López
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Albert Astals Cid 2007-12-29 17:08:04 UTC
Version:            (using KDE KDE 3.97.0)
Installed from:    Compiled From Sources

Entering applications:// (note the double /) crashes konqueror with this bracktrace

#0  0x00002b301d21c9e6 in QSharedDataPointer<KFileItemPrivate>::operator-> (this=0x0)
    at /home/kdesvn/qt-copy/include/QtCore/../../src/corelib/tools/qshareddata.h:69
#1  0x00002b301d218b8f in KFileItem::isDir (this=0x0) at /home/kdesvn/kdelibs/kio/kio/kfileitem.cpp:893
#2  0x00002b301d20bb4d in KDirModelPrivate::isDir (this=0x14021e0, node=0x0) at /home/kdesvn/kdelibs/kio/kio/kdirmodel.cpp:132
#3  0x00002b301d209af1 in KDirModelPrivate::_k_slotNewItems (this=0x14021e0, items=@0xc4e710) at /home/kdesvn/kdelibs/kio/kio/kdirmodel.cpp:298
#4  0x00002b301d20a1a0 in KDirModel::qt_metacall (this=0x13fd8d0, _c=QMetaObject::InvokeMetaMethod, _id=1, _a=0x7fff8e5841f0)
    at /home/kdesvn/build-cmake/kdelibs/kio/kdirmodel.moc:75
#5  0x00002b3020b6a1b2 in QMetaObject::activate (sender=0xc3d300, from_signal_index=13, to_signal_index=13, argv=0x7fff8e581890) at kernel/qobject.cpp:3083
#6  0x00002b301d1f394b in KDirLister::newItems (this=0xc3d300, _t1=@0xc4e710) at /home/kdesvn/build-cmake/kdelibs/kio/kdirlister.moc:252
#7  0x00002b301d1f3ee4 in KDirLister::Private::emitItems (this=0x13ff420) at /home/kdesvn/kdelibs/kio/kio/kdirlister.cpp:2184
Comment 1 Rafael Fernández López 2007-12-30 02:27:27 UTC
Created attachment 22768 [details]
Patch for fixing the crash

This patch will fix the crash. The problem remains on setEncodedUrl with
QUrl::TolerantMode. I don't know if this should be reported to TT, since maybe
they are doing fine. Anyway comments that deserve being told: setEncodedUrl
will leave "protocol:/" as it is, and "protocol:///" will be converted to
"protocol:/". Maybe there is a rationale behind the behavior of converting
"protocol://" to "protocol:" (what makes it an invalid URL for us, and thus,
crashing). Makes sense to me that we could check for this cases of more than
one tracking slashes and convert them to only one, if it existed.
Comment 2 Rafael Fernández López 2007-12-30 03:07:01 UTC
Created attachment 22769 [details]
Patch improved for fixing the crash.

This new patch is far safer, but I'm not sure if it will comply with all
standards. Basically, safer compared to the previous because in the case that
_str wasn't finishing in '/', I had trailingSlashesToRemove = -1, and it was
later being called with a count() - (-1), resulting in a bye-bye ranges.

It also prevents from crashing when calling "protocol:", by adding a '/'. This
is the part I'm not sure of taking care of standards...

Do you think this patch can be committed ? If so, please speak up.
Comment 3 Rafael Fernández López 2007-12-30 12:30:39 UTC
Previous patches are wrong because they make some KUrl tests to fail. Working on a different solution.
Comment 4 Rafael Fernández López 2007-12-30 16:24:26 UTC
Created attachment 22775 [details]
Patch improved for fixing the crash

This is another way of proceeding for fixing this crash.

I really think David Faure should take a look here before taking any decision.
Comment 5 David Faure 2008-01-04 12:21:36 UTC
Can you explain the patch from comment #4?
Comment 6 Rafael Fernández López 2008-01-04 13:16:26 UTC
The traceback (by word) was:

- void KDirModelPrivate::_k_slotNewItems(const KFileItemList& items) is called (we have new items)
- const QPair<int, KDirModelNode*> result = nodeForUrl(dir); // O(n*m)
- QList<KDirModelNode *>::const_iterator it = dirNode->m_childNodes.begin();
  const QList<KDirModelNode *>::const_iterator end = dirNode->m_childNodes.end();
- it == end (empty list) => foundChild = false
- return qMakePair(0, static_cast<KDirModelNode*>(0));
- assert at Q_ASSERT(result.second); // Are you calling KDirLister::openUrl(url,true,false)? Please use expandToUrl() instead.
Comment 7 David Faure 2008-01-04 13:47:35 UTC
Hello gdb :-)
The real reasoning behind this is "KDirModel can't put items under a root that has no path".
So we need a path for the root, for instance by doing a redirect in the slave; or maybe we can
change KDirModel to assume "/" as root path if the root path was empty --- and the slave hasn't
redirected us elsewhere, like kio_ftp does.
Comment 8 David Faure 2008-01-04 19:01:38 UTC
SVN commit 757313 by dfaure:

Assume "/" when the URL has no path (and the slave didn't redirect)
BUG: 154819


 M  +9 -2      kdirmodel.cpp  


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