D11067: Fix 3 more cases of incorrect parameter to findProtocol

2018-03-06 Thread David Faure 
dfaure added inline comments.

INLINE COMMENTS

> jtamate wrote in kurlcompletion.cpp:626
> 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
> 
> }

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

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11067

To: jtamate, #frameworks, dfaure
Cc: michaelh

D11067: Fix 3 more cases of incorrect parameter to findProtocol

2018-03-06 Thread Jaime Torres Amate 
jtamate added inline comments.

INLINE COMMENTS

> dfaure wrote in kurlcompletion.cpp:626
> 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

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

}

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11067

To: jtamate, #frameworks, dfaure
Cc: michaelh

D11067: Fix 3 more cases of incorrect parameter to findProtocol

2018-03-05 Thread David Faure 
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kprotocolmanager.cpp:1141
> +}
>  KProtocolInfoPrivate *prot = findProtocol(url);
>  if (!prot) {

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

> kurlcompletion.cpp:626
>  
> +if (!url.kurl().isValid()) {
> +return QString();

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

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11067

To: jtamate, #frameworks, dfaure
Cc: michaelh

D11067: Fix 3 more cases of incorrect parameter to findProtocol

2018-03-05 Thread Jaime Torres Amate 
jtamate added a comment.


  The backtraces:
  
  deleting characters from the beginning
  
  #10 0x7fdc893f249a 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 0x7fdc893f1c07 in KProtocolInfo::proxiedBy (_protocol=...) at 
/virtual/kde5/5kde/frameworks/kio/src/core/kprotocolinfo.cpp:380
  #12 0x7fdc893da776 in findProtocol (url=...) at 
/virtual/kde5/5kde/frameworks/kio/src/core/kprotocolmanager.cpp:1108
  #13 0x7fdc893da8ce in KProtocolManager::supportsListing (url=...) at 
/virtual/kde5/5kde/frameworks/kio/src/core/kprotocolmanager.cpp:1148
  #14 0x7fdc8d583f31 in 
DolphinViewContainer::slotUrlNavigatorLocationChanged (this=0x7fdc70007610, 
url=...) at /g/5kde/kde/applications/dolphin/src/dolphinviewcontainer.cpp:568
  #15 0x7fdc85abe0cc in QMetaObject::activate(QObject*, int, int, void**) 
() from /usr/lib64/libQt5Core.so.5
  #16 0x7fdc8c1d6611 in KUrlNavigator::urlChanged (this=0x2de9730, _t1=...) 
at 
/virtual/kde5/5kde/build/frameworks/kio/src/filewidgets/KF5KIOFileWidgets_autogen/include/moc_kurlnavigator.cpp:318
  #17 0x7fdc8c1d54a4 in KUrlNavigator::setLocationUrl (this=0x2de9730, 
newUrl=...) at 
/virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:1092
  #18 0x7fdc8c1d13cb in KUrlNavigator::Private::applyUncommittedUrl 
(this=0x373c6f0) at 
/virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:322
  #19 0x7fdc8c1d1447 in KUrlNavigator::Private::slotReturnPressed 
(this=0x373c6f0) at 
/virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:331
  
  an url like ~:/
  
  #10 0x7f553750949a 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 0x7f55375089a9 in KProtocolInfo::protocolClass (_protocol=...) at 
/virtual/kde5/5kde/frameworks/kio/src/core/kprotocolinfo.cpp:331
  #12 0x7f553956acab in isLocalProtocol (protocol=...) at 
/virtual/kde5/5kde/frameworks/kio/src/widgets/kurlcompletion.cpp:1001
  #13 0x7f553956ae6e in KUrlCompletionPrivate::urlCompletion 
(this=0x45b4e30, url=..., pMatch=0x7fff73123518) at 
/virtual/kde5/5kde/frameworks/kio/src/widgets/kurlcompletion.cpp:1024
  #14 0x7f55395693f1 in KUrlCompletion::makeCompletion (this=0x45b44c0, 
text=...) at 
/virtual/kde5/5kde/frameworks/kio/src/widgets/kurlcompletion.cpp:686
  #15 0x7f55388449f6 in KLineEdit::makeCompletion (this=0x45b66a0, 
text=...) at /g/5kde/frameworks/kcompletion/src/klineedit.cpp:432
  #16 0x7f553884a8ab in KLineEdit::doCompletion (this=0x45b66a0, text=...) 
at /g/5kde/frameworks/kcompletion/src/klineedit.cpp:1665
  #17 0x7f5538846d5a in KLineEdit::keyPressEvent (this=0x45b66a0, 
e=0x7fff73124020) at /g/5kde/frameworks/kcompletion/src/klineedit.cpp:901
  
  an url like ~:/  (second attempt)
  
  #10 0x7fe8140c74b4 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 0x7fe8140c6c21 in KProtocolInfo::proxiedBy (_protocol=...) at 
/virtual/kde5/5kde/frameworks/kio/src/core/kprotocolinfo.cpp:380
  #12 0x7fe8140af776 in findProtocol (url=...) at 
/virtual/kde5/5kde/frameworks/kio/src/core/kprotocolmanager.cpp:1108
  #13 0x7fe8140af896 in KProtocolManager::isSourceProtocol (url=...) at 
/virtual/kde5/5kde/frameworks/kio/src/core/kprotocolmanager.cpp:1138
  #14 0x7fe818258f88 in 
DolphinViewContainer::slotUrlNavigatorLocationChanged (this=0x24b01e0, url=...) 
at /g/5kde/kde/applications/dolphin/src/dolphinviewcontainer.cpp:579
  #15 0x7fe8107930cc in QMetaObject::activate(QObject*, int, int, void**) 
() from /usr/lib64/libQt5Core.so.5
  #16 0x7fe816eab611 in KUrlNavigator::urlChanged (this=0x25d6fc0, _t1=...) 
at 
/virtual/kde5/5kde/build/frameworks/kio/src/filewidgets/KF5KIOFileWidgets_autogen/include/moc_kurlnavigator.cpp:318
  #17 0x7fe816eaa4a4 in KUrlNavigator::setLocationUrl (this=0x25d6fc0, 
newUrl=...) at 
/virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:1092
  #18 0x7fe816ea63cb in KUrlNavigator::Private::applyUncommittedUrl 
(this=0x1f49730) at 
/virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:322
  #19 0x7fe816ea6447 in KUrlNavigator::Private::slotReturnPressed 
(this=0x1f49730) at 
/virtual/kde5/5kde/frameworks/kio/src/filewidgets/kurlnavigator.cpp:331

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11067

To: jtamate, #frameworks, dfaure
Cc: michaelh

D11067: Fix 3 more cases of incorrect parameter to findProtocol

2018-03-05 Thread Jaime Torres Amate 
jtamate created this revision.
jtamate added reviewers: Frameworks, dfaure.
Restricted Application added a project: Frameworks.
jtamate requested review of this revision.

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

REPOSITORY
  R241 KIO

BRANCH
  protocol (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11067

AFFECTED FILES
  src/core/kprotocolmanager.cpp
  src/widgets/kurlcompletion.cpp

To: jtamate, #frameworks, dfaure
Cc: michaelh