Bug#773085: Regression in escaped url handling with patch applied for #773085
❦ 8 janvier 2015 08:50 +0100, Vincent Bernat ber...@debian.org : Per, I see that you committed to git yesterday. Would you take the patch as is or do you want me to do an NMU? I have just uploaded an NMU to delayed/2. Works for me. -- Zounds! I was never so bethumped with words since I first called my brother's father dad. -- William Shakespeare, Kind John signature.asc Description: PGP signature
Bug#773085: Regression in escaped url handling with patch applied for #773085
Control: tags -1 + patch
❦ 3 janvier 2015 23:22 +0100, Vincent Bernat ber...@debian.org :
However, the whole stuff is quite fragile. I can't say for sure if
spaces would do something good or bad, but a star would not work. Here
is an improved version which is easier to understand.
I have found a way to be even more concise and don't rely on any magic
(other that $@ being the only array in a POSIX shell). Here is a
patch.
The first change is to let the shell do the splitting of the command in
the .desktop file (set -- $(sed ...)).
The second change is to use $@ behaving like an array. We cannot
modify this array but we can append to it (with set -- $@
$newarg). Basically, we take $command_exec and then shift. Then, we
iterate on each argument using a counter and if the argument needs to be
modified (because this is the place holder), we append the modified
version, otherwise, we append it unmodified. At the end, $@ is the
array of arguments to be passed to $command_exec. If no replacement
has happened, we also append the target file.
No magic quoting is done, no evaluation. I think this is a safe
alternative to the current script. I can also push it upstream.
Per, I see that you committed to git yesterday. Would you take the patch
as is or do you want me to do an NMU?
--- /usr/bin/xdg-open 2015-01-03 22:22:18.513474060 +0100
+++ ./xdg-open 2015-01-08 08:42:47.513093876 +0100
@@ -526,6 +526,7 @@
open_generic_xdg_mime()
{
+target=$1
filetype=$2
default=`xdg-mime query default $filetype`
if [ -n $default ] ; then
@@ -546,17 +547,34 @@
fi
if [ -r $file ] ; then
-command=`grep -E ^Exec(\[[^]=]*])?= $file | cut -d= -f 2- | first_word`
-command_exec=`which $command 2/dev/null`
-arguments=`grep -E ^Exec(\[[^]=]*])?= $file | cut -d= -f 2- | last_word`
-local sed_escaped_url=$(echo $1 | sed -e 's/[\\]/\\/g')
-arguments_exec=$(echo $arguments | sed -e 's*%[fFuU]*'$sed_escaped_url'*g')
+set -- $(sed -n 's/^Exec\(\[[^]]*\]\)\{0,1\}=//p' $file)
+command_exec=$(which $1 2 /dev/null)
if [ -x $command_exec ] ; then
-if echo $arguments | grep -iq '%[fFuU]' ; then
-eval '$command_exec' '$arguments_exec'
-else
-eval '$command_exec' '$arguments_exec' '$1'
-fi
+shift
+# We need to replace any occurrence of %f, %F and
+# the like by the target file. We examine each
+# argument and append the modified argument to the
+# end then shift.
+args=$#
+replaced=0
+while [ $args -gt 0 ]; do
+case $1 in
+%[fFuU])
+replaced=1
+arg=$target
+shift
+set -- $@ $arg
+;;
+*)
+arg=$1
+shift
+set -- $@ $arg
+;;
+esac
+args=$(( $args - 1 ))
+done
+[ $replaced -eq 1 ] || set -- $@ $target
+$command_exec $@
if [ $? -eq 0 ]; then
exit_success
--
Make sure all variables are initialised before use.
- The Elements of Programming Style (Kernighan Plauger)
signature.asc
Description: PGP signature
Bug#773085: Regression in escaped url handling with patch applied for #773085
❦ 4 janvier 2015 14:29 -0500, Michael Gilbert mgilb...@debian.org : I played around today for checking the xdg-open issue also for wheezy, and noticed that the approach introduces a regression. Hi Salvatore, Thanks for the review. The upstream patch as Vincent mentions has the same design flaw, and there is a straightforward fix he also suggests, but the fragility overall is a concern and time should be spent on a more robust fix. My simple modification doesn't work as expected if the file name contains spaces. I wont' have time to look at this for a couple days, so help is appreciated. I'll come with a patch then. -- Make sure comments and code agree. - The Elements of Programming Style (Kernighan Plauger) signature.asc Description: PGP signature
Bug#773085: Regression in escaped url handling with patch applied for #773085
control: tag -1 help On Sat, Jan 3, 2015 at 11:31 AM, Salvatore Bonaccorso wrote: I played around today for checking the xdg-open issue also for wheezy, and noticed that the approach introduces a regression. Hi Salvatore, Thanks for the review. The upstream patch as Vincent mentions has the same design flaw, and there is a straightforward fix he also suggests, but the fragility overall is a concern and time should be spent on a more robust fix. I wont' have time to look at this for a couple days, so help is appreciated. Best wishes, Mike -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#773085: Regression in escaped url handling with patch applied for #773085
❦ 3 janvier 2015 17:31 +0100, Salvatore Bonaccorso car...@debian.org :
Steps for reproducing the issue:
$ xdg-mime default chromium.desktop x-scheme-handler/http
$ xdg-mime query default x-scheme-handler/http
chromium.desktop
$ DE='generic' XDG_CURRENT_DESKTOP= xdg-open
'http://bugs.debian.org/cgi-bin/pkgreport.cgi?src=xdg-utilsrepeatmerged=no'
Without the patch applied, the page correctly is opened. If doing so
the same with the applied patch chromium get passed as argument
'$sed_escaped_url', and xdg-open executes /usr/bin/chromium
'$sed_escaped_url'.
I don't understand how the proposed patch would work. $arg_one (or
$sed_escaped_url) is singly quoted and therefore cannot be
expanded.
If I modify the first chunk of the patch, it works as expected:
arguments_exec=$(echo $arguments | sed -e
's*%[fFuU]*'$sed_escaped_url'*g')
(this is not like the initial chunk, I don't quote the argument.
xdg-open 'http://www.example.com/$(xterm)' works as expected.
However, the whole stuff is quite fragile. I can't say for sure if
spaces would do something good or bad, but a star would not work. Here
is an improved version which is easier to understand.
#+begin_src sh
file=/usr/share/applications/chromium.desktop
# Safe quoting. We just enclose into single quotes the given argument
# and escape single quotes.
quote() {
printf %s\\n $1 | sed s/'/'''/g;1s/^/'/;\$s/\$/'/
}
arg=$1
set -- $(sed -n 's/^Exec\(\[[^]]*\]\)\{0,1\}=//p' $file)
cmd=$(which $1 2 /dev/null)
[ -n $cmd ] || exit 2
shift
args=
while [ $# -gt 0 ]; do
case $1 in
%[fFuU])
args=$args $(quote $arg)
;;
*)
args=$args $(quote $1)
;;
esac
shift
done
$cmd $args
#+end_src
The set is just here to let the shell do the quoting. If no
replacement was needed, we could just $cmd $@ after the first shift
and be done. Unfortunately, with just a POSIX shell, the replacement of
the positional argument is difficult. Instead, we build the list of args
by quoting correctly each of them. Then, it can be executed.
Using bash would be more straightforward since we could stack our
arguments into an array and modify this array to substitute %U and the
like.
--
The surest protection against temptation is cowardice.
-- Mark Twain
signature.asc
Description: PGP signature
Bug#773085: Regression in escaped url handling with patch applied for #773085
Control: reopen -1 Hi Mike I played around today for checking the xdg-open issue also for wheezy, and noticed that the approach introduces a regression. Steps for reproducing the issue: $ xdg-mime default chromium.desktop x-scheme-handler/http $ xdg-mime query default x-scheme-handler/http chromium.desktop $ DE='generic' XDG_CURRENT_DESKTOP= xdg-open 'http://bugs.debian.org/cgi-bin/pkgreport.cgi?src=xdg-utilsrepeatmerged=no' Without the patch applied, the page correctly is opened. If doing so the same with the applied patch chromium get passed as argument '$sed_escaped_url', and xdg-open executes /usr/bin/chromium '$sed_escaped_url'. I have not checked yet, but it might be that upstream had some additional commits in the surrounding code for handling the arguments differently. Regards, Salvatore -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
