Differential D11067

Fix more cases of incorrect parameter to findProtocol
ClosedPublic
Actions

Authored by jtamate on Mar 5 2018, 8:05 PM.

Details

Reviewers
dfaure
Group Reviewers
Frameworks
Commits
R241:c25b6745366f: Fix more cases of incorrect parameter to findProtocol
Summary

deleting characters from the beginning in a url in dolphin
trying to use this url: ~:/

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jtamate created this revision.Mar 5 2018, 8:05 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 5 2018, 8:05 PM
jtamate requested review of this revision.Mar 5 2018, 8:05 PM
jtamate added a comment.Mar 5 2018, 8:07 PM

The backtraces:

deleting characters from the beginning

#10 0x00007fdc893f249a in KProtocolInfoFactory::findProtocol (this=0x7fdc896b9720 <(anonymous namespace)::Q_QGS_kProtocolInfoFactoryInstance::innerFunction()::holder>, protocol=...) at /virtual/kde5/5kde/frameworks/kio/src/core/kprotocolinfofactory.cpp:72
#11 0x00007fdc893f1c07 in KProtocolInfo::proxiedBy (_protocol=...) at /virtual/kde5/5kde/frameworks/kio/src/core/kprotocolinfo.cpp:380
#12 0x00007fdc893da776 in findProtocol (url=...) at /virtual/kde5/5kde/frameworks/kio/src/core/kprotocolmanager.cpp:1108
#13 0x00007fdc893da8ce in KProtocolManager::supportsListing (url=...) at /virtual/kde5/5kde/frameworks/kio/src/core/kprotocolmanager.cpp:1148
#14 0x00007fdc8d583f31 in DolphinViewContainer::slotUrlNavigatorLocationChanged (this=0x7fdc70007610, url=...) at /g/5kde/kde/applications/dolphin/src/dolphinviewcontainer.cpp:568
#15 0x00007fdc85abe0cc in QMetaObject::activate(QObject*, int, int, void**) () from /usr/lib64/libQt5Core.so.5
#16 0x00007fdc8c1d6611 in KUrlNavigator::urlChanged (this=0x2de9730, _t1=...) at /virtual/kde5/5kde/build/frameworks/kio/src/filewidgets/KF5KIOFileWidgets_autogen/include/moc_kurlnavigator.cpp:318
#17 0x00007fdc8c1d54a4 in KUrlNavigator::setLocationUrl (this=0x2de9730, newUrl=...) at /virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:1092
#18 0x00007fdc8c1d13cb in KUrlNavigator::Private::applyUncommittedUrl (this=0x373c6f0) at /virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:322
#19 0x00007fdc8c1d1447 in KUrlNavigator::Private::slotReturnPressed (this=0x373c6f0) at /virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:331

an url like ~:/

#10 0x00007f553750949a in KProtocolInfoFactory::findProtocol (this=0x7f55377d0720 <(anonymous namespace)::Q_QGS_kProtocolInfoFactoryInstance::innerFunction()::holder>, protocol=...) at /virtual/kde5/5kde/frameworks/kio/src/core/kprotocolinfofactory.cpp:72
#11 0x00007f55375089a9 in KProtocolInfo::protocolClass (_protocol=...) at /virtual/kde5/5kde/frameworks/kio/src/core/kprotocolinfo.cpp:331
#12 0x00007f553956acab in isLocalProtocol (protocol=...) at /virtual/kde5/5kde/frameworks/kio/src/widgets/kurlcompletion.cpp:1001
#13 0x00007f553956ae6e in KUrlCompletionPrivate::urlCompletion (this=0x45b4e30, url=..., pMatch=0x7fff73123518) at /virtual/kde5/5kde/frameworks/kio/src/widgets/kurlcompletion.cpp:1024
#14 0x00007f55395693f1 in KUrlCompletion::makeCompletion (this=0x45b44c0, text=...) at /virtual/kde5/5kde/frameworks/kio/src/widgets/kurlcompletion.cpp:686
#15 0x00007f55388449f6 in KLineEdit::makeCompletion (this=0x45b66a0, text=...) at /g/5kde/frameworks/kcompletion/src/klineedit.cpp:432
#16 0x00007f553884a8ab in KLineEdit::doCompletion (this=0x45b66a0, text=...) at /g/5kde/frameworks/kcompletion/src/klineedit.cpp:1665
#17 0x00007f5538846d5a in KLineEdit::keyPressEvent (this=0x45b66a0, e=0x7fff73124020) at /g/5kde/frameworks/kcompletion/src/klineedit.cpp:901

an url like ~:/ (second attempt)

#10 0x00007fe8140c74b4 in KProtocolInfoFactory::findProtocol (this=0x7fe81438e720 <(anonymous namespace)::Q_QGS_kProtocolInfoFactoryInstance::innerFunction()::holder>, protocol=...) at /virtual/kde5/5kde/frameworks/kio/src/core/kprotocolinfofactory.cpp:72
#11 0x00007fe8140c6c21 in KProtocolInfo::proxiedBy (_protocol=...) at /virtual/kde5/5kde/frameworks/kio/src/core/kprotocolinfo.cpp:380
#12 0x00007fe8140af776 in findProtocol (url=...) at /virtual/kde5/5kde/frameworks/kio/src/core/kprotocolmanager.cpp:1108
#13 0x00007fe8140af896 in KProtocolManager::isSourceProtocol (url=...) at /virtual/kde5/5kde/frameworks/kio/src/core/kprotocolmanager.cpp:1138
#14 0x00007fe818258f88 in DolphinViewContainer::slotUrlNavigatorLocationChanged (this=0x24b01e0, url=...) at /g/5kde/kde/applications/dolphin/src/dolphinviewcontainer.cpp:579
#15 0x00007fe8107930cc in QMetaObject::activate(QObject*, int, int, void**) () from /usr/lib64/libQt5Core.so.5
#16 0x00007fe816eab611 in KUrlNavigator::urlChanged (this=0x25d6fc0, _t1=...) at /virtual/kde5/5kde/build/frameworks/kio/src/filewidgets/KF5KIOFileWidgets_autogen/include/moc_kurlnavigator.cpp:318
#17 0x00007fe816eaa4a4 in KUrlNavigator::setLocationUrl (this=0x25d6fc0, newUrl=...) at /virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:1092
#18 0x00007fe816ea63cb in KUrlNavigator::Private::applyUncommittedUrl (this=0x1f49730) at /virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:322
#19 0x00007fe816ea6447 in KUrlNavigator::Private::slotReturnPressed (this=0x1f49730) at /virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:331

dfaure requested changes to this revision.Mar 5 2018, 9:33 PM
dfaure added inline comments.
src/core/kprotocolmanager.cpp
1141

You could move the check to inside findProtocol, so it's done in a central location.

src/widgets/kurlcompletion.cpp
626 ↗(On Diff #28751)

I'm not 100% sure about this one because kurl() is only supposed to be called if url.isURL().

Also, I tried to reproduce this assert with a unittest for kcompletion, but it works here !?

http://www.davidfaure.fr/2018/kurlcompletion_test.diff

This revision now requires changes to proceed.Mar 5 2018, 9:33 PM
jtamate added inline comments.Mar 6 2018, 8:11 AM
src/widgets/kurlcompletion.cpp
626 ↗(On Diff #28751)

I missed one key stroke. With this test, in a non empty current dirt, it fails.

void KUrlCompletionTest::testInvalidProtocol()
{

KUrlCompletion* completionHomeCwd = new KUrlCompletion;
completionHomeCwd->setDir(QUrl("/home/user"));

completionHomeCwd->makeCompletion(QLatin1String(":/"));
waitForCompletion(completionHomeCwd);
const auto matches = completionHomeCwd->allMatches();
// just don't crash

}

dfaure added inline comments.Mar 6 2018, 9:16 AM
src/widgets/kurlcompletion.cpp
626 ↗(On Diff #28751)

Thanks for finding this. I made a copy/paste error, the "EmptyCwd" should have been removed, there's already a completion object set up with a base dir, in this test.

Here's the simpler test, with a fix that seems safer to me: https://phabricator.kde.org/D11089

jtamate updated this revision to Diff 28801.Mar 6 2018, 9:44 AM
jtamate retitled this revision from Fix 3 more cases of incorrect parameter to findProtocol to Fix more cases of incorrect parameter to findProtocol.

David's comment done.

dfaure accepted this revision.Mar 6 2018, 9:53 AM
This revision is now accepted and ready to land.Mar 6 2018, 9:53 AM
Closed by commit R241:c25b6745366f: Fix more cases of incorrect parameter to findProtocol (authored by jtamate). · Explain WhyMar 6 2018, 5:58 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/core/kprotocolmanager.cpp (3 lines)

Diff 28855

View Options

src/core/kprotocolmanager.cpp

Loading...