Bug 317046

Summary: Pre-shell command does not accept space in path
Product: [Applications] konversation Reporter: Reshad Patuck <reshad>
Component: generalAssignee: Konversation Developers <konversation-devel>
Status: RESOLVED FIXED    
Severity: normal CC: hein, reshad
Priority: NOR    
Version First Reported In: 1.4   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: http://commits.kde.org/konversation/af4c36335ea14c3273b48ab22f14723975e8d3e5 Version Fixed In:
Sentry Crash Report:

Description Reshad Patuck 2013-03-19 20:38:14 UTC 
When I try to try to execute a shell script to tunnel my connection over ssh, konversation fails to execute it.

my script is at /home/abc/Documents/Some Folder/ssh-tunel.sh

The script executed fine when I moved it to /home/abc/Documents/Some Folder

Reproducible: Always

Steps to Reproduce:
1.In the advanced tab under identities add a pre-shell command and specify a path with a space character in it.
2.Run that configuration by opening a connection to an irc server by using that script.
3.
Actual Results:  
The script is not executed and Konversation outputs this message " [Warning] There was a problem while executing the command!" to the screen.

Expected Results:  
Konversation shoud execute the script and post "[Info] Process executed successfully!" to the screen
Comment 1 Eike Hein 2013-03-19 21:04:50 UTC 
I'm actually not sure we can economically solve that programmatically without splitting the field in two for command and arguments respectively, because there's no good way to determine where command ends and arguments begin, other than doing filesystem existence tests.

Can you check if escaping the spaces with \ works?
Comment 2 Reshad Patuck 2013-03-19 21:07:49 UTC 
I can confirm that escaping with \ does not work.

Will try and dig into the code myself a bit too.

Cheers
Comment 3 Eike Hein 2013-03-19 21:11:12 UTC 
Thanks. Basically, QProcess/KProcess have methods that would take care of the quoting/escaping for us - you can hand it program and argument list separately, and spaces in the program or path work fine then, but that just changes the problem into "figuring out which part of the space-separated list is supposed to be the program". What we _could_ do, other than the two-field approach, is to require users to backslash escape, and then actually parse the escaping.
Comment 4 Eike Hein 2013-03-19 21:14:43 UTC 
The "Command" field in KDE .desktop links has the same problem btw, but backslash escape works there.
Comment 5 Reshad Patuck 2013-03-19 21:26:27 UTC 
correct me if I'm wrong, 
const QStringList commandList = command.split(' ');
basically says if you see a space treat the string as  a new command.
Comment 6 Eike Hein 2013-03-19 21:29:13 UTC 
Not really, rather it's that K/QProcess, like most subprocess APIs on POSIX-like platforms, want a list as input, where the first entry in the list becomes the program and every other entry an argument. (Ever written a C program and dealt with argv? That's the other side of that.) So we split by space to separate the arguments; the problem is that this also turns everything after the first space into an argument, and only the part before the first space is considered the command.
Comment 7 argonel 2013-04-02 21:48:56 UTC 
Git commit af4c36335ea14c3273b48ab22f14723975e8d3e5 by eli mackenzie.
Committed on 29/03/2013 at 09:58.
Pushed by argonel into branch '1.4'.

Handle spaces in pre-shell commands

Switches to using KShell::splitArgs to build the list of arguements
to feed to KProcess instead of doing it here. As a bonus, tilde
expansion works now as well.

This patch also adds a check to ensure the connection only gets
set up once.

M  +2    -0    ChangeLog
M  +30   -15   src/irc/server.cpp

http://commits.kde.org/konversation/af4c36335ea14c3273b48ab22f14723975e8d3e5