Kicker/RecentUsageModel: Allow to open smb:/ sftp:/ resources
Needs ReviewPublic

Authored by meven on Sat, Jan 11, 12:48 PM.

Details

Reviewers
broulik
aleksejshilin
hein
Group Reviewers
Plasma
Summary

Since D10835 opening urls that needed mimetype determination such as smb:/ or sftp:/ urls were always opened with a "Open With" dialog.
To keep the same security but adding some convenience, use KRun::setRunExecutables to disable executable run here.

Diff Detail

Repository
R120 Plasma Workspace
Branch
arcpatch-D26582
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20958
Build 20976: arc lint + arc unit
meven created this revision.Sat, Jan 11, 12:48 PM
Restricted Application added a project: Plasma. · View Herald TranscriptSat, Jan 11, 12:48 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
meven requested review of this revision.Sat, Jan 11, 12:48 PM

I thought about using KRun::setShowScriptExecutionPrompt() for D10835, but "Recent Documents" include only those files which were opened, and doesn't include ones which were executed, so an execution prompt there would be totally unexpected by a user. KRun currently doesn't seem to have a way to force opening of executables - one can only prohibit running them with setRunExecutables(), in which case an error message is displayed. That's why I decided to go the way it is now.

I think, the best solution would be to add a new KRun::RunFlag (like OpenExecutables) which'd make it open executable files instead of running them, and then use it here.

meven planned changes to this revision.Sat, Jan 11, 3:06 PM

I thought about using KRun::setShowScriptExecutionPrompt() for D10835, but "Recent Documents" include only those files which were opened, and doesn't include ones which were executed, so an execution prompt there would be totally unexpected by a user. KRun currently doesn't seem to have a way to force opening of executables - one can only prohibit running them with setRunExecutables(), in which case an error message is displayed. That's why I decided to go the way it is now.

I think, the best solution would be to add a new KRun::RunFlag (like OpenExecutables) which'd make it open executable files instead of running them, and then use it here.

Good point, I will probably offer a KRun patch.
This regression is very annoying to me.

meven edited the summary of this revision. (Show Details)Sat, Jan 11, 3:12 PM
meven updated this revision to Diff 73275.Sat, Jan 11, 3:13 PM

Use KRun::setRunExecutables

meven added a comment.Sat, Jan 11, 3:13 PM

Well there was already KRun::setRunExecutables :)

Well there was already KRun::setRunExecutables :)

I've already talked about it in my previous message:

KRun currently doesn't seem to have a way to force opening of executables - one can only prohibit running them with setRunExecutables(), in which case an error message is displayed. That's why I decided to go the way it is now.

For example:

  1. Install Okteta.
  2. Associate application/x-sharedlib mime type with Okteta.
  3. Open a shared library with Okteta.
  4. Try to open it again from 'Recent Documents'.

Instead of opening the file in Okteta, it'll show a message box saying that the executable file won't be started for security reasons.

I admit that it's not a likely scenario though, so let's see what others have to say.

Well there was already KRun::setRunExecutables :)

I've already talked about it in my previous message:

KRun currently doesn't seem to have a way to force opening of executables - one can only prohibit running them with setRunExecutables(), in which case an error message is displayed. That's why I decided to go the way it is now.

For example:

  1. Install Okteta.
  2. Associate application/x-sharedlib mime type with Okteta.
  3. Open a shared library with Okteta.
  4. Try to open it again from 'Recent Documents'.

    Instead of opening the file in Okteta, it'll show a message box saying that the executable file won't be started for security reasons.

    I admit that it's not a likely scenario though, so let's see what others have to say.

Thanks for testing this. There is a different treatment with nativeBinary in KRun,
We may want to change KRun::runUrl when a native binary is to be run but with runExecutable is false, around line 400:

} else if (isNativeBinary) {
    // Show warning for executables that aren't scripts
    noRun = true;
}

I guess in this case with could check for mime-type associated apps like oketa can be with any binary file, perhaps adding a flag.

We may want to change KRun::runUrl when a native binary is to be run but with runExecutable is false, around line 400:

} else if (isNativeBinary) {
    // Show warning for executables that aren't scripts
    noRun = true;
}

KRun currently doesn't seem to consider that there may be a reason to open native binaries in an application - its execution confirmation dialog doesn't give such option even if some application is associated with the corresponding mime type. I believe, they thought that an average user is unlikely to ever need this - and they may be right. :)

Given that such use case is probably orders of magnitude less likely than sftp: and especially smb:, I guess, it's OK to merge this, and address the issue with native binaries in KIO later. @broulik ?