[PATCH v2] Fix cp's target path for ext* filesystems

[PATCH v2] Fix cp's target path for ext* filesystems

[Leonardo Augusto]

Python subprocess' shell=True defaults to /bin/sh[1][2], which
often refers to a POSIX-compliant shell. As the -e flag is
not defined in the POSIX standard[3], some shells may
interpret "-e" as the first argument to the echo command,
causing the rest of the command line to fail.

In this particular case, "echo -e 'cd {}'" is interpreted
as "-e cd {}", which causes the first line of the command
to fail, and causing cp to always place the source file
in the filesystem's root.

This issue only affects "wic cp" for ext* filesystems.

[1] https://docs.python.org/2/library/subprocess.html
[2] https://docs.python.org/3/library/subprocess.html
[3] http://pubs.opengroup.org/onlinepubs/9699919799

Signed-off-by: Leonardo Augusto <leobsd@gmail.com>
---
 scripts/lib/wic/engine.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py
index 4662c665c0..b4be796752 100644
--- a/scripts/lib/wic/engine.py
+++ b/scripts/lib/wic/engine.py
@@ -331,7 +331,7 @@ class Disk:
     def copy(self, src, pnum, path):
         """Copy partition image into wic image."""
         if self.partitions[pnum].fstype.startswith('ext'):
-            cmd = "echo -e 'cd {}\nwrite {} {}' | {} -w {}".\
+            cmd = "echo 'cd {}\nwrite {} {}' | {} -w {}".\
                       format(path, src, os.path.basename(src),
                              self.debugfs, self._get_part_image(pnum))
         else: # fat
-- 
2.17.1

Re: [PATCH v2] Fix cp's target path for ext* filesystems

[André Draszik]

On Fri, 2019-02-08 at 00:20 -0800, Leonardo Augusto wrote:
> Python subprocess' shell=True defaults to /bin/sh[1][2], which
> often refers to a POSIX-compliant shell. As the -e flag is
> not defined in the POSIX standard[3], some shells may
> interpret "-e" as the first argument to the echo command,
> causing the rest of the command line to fail.
> 
> In this particular case, "echo -e 'cd {}'" is interpreted
> as "-e cd {}", which causes the first line of the command
> to fail, and causing cp to always place the source file
> in the filesystem's root.
> 
> This issue only affects "wic cp" for ext* filesystems.
> 
> [1] https://docs.python.org/2/library/subprocess.html
> [2] https://docs.python.org/3/library/subprocess.html
> [3] http://pubs.opengroup.org/onlinepubs/9699919799
> 
> Signed-off-by: Leonardo Augusto <leobsd@gmail.com>
> ---
>  scripts/lib/wic/engine.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py
> index 4662c665c0..b4be796752 100644
> --- a/scripts/lib/wic/engine.py
> +++ b/scripts/lib/wic/engine.py
> @@ -331,7 +331,7 @@ class Disk:
>      def copy(self, src, pnum, path):
>          """Copy partition image into wic image."""
>          if self.partitions[pnum].fstype.startswith('ext'):
> -            cmd = "echo -e 'cd {}\nwrite {} {}' | {} -w {}".\
> +            cmd = "echo 'cd {}\nwrite {} {}' | {} -w {}".\

This causes some shell's echos to output a literal \n (two characters, '\'
and 'n').

The solution should be to use printf instead of echo...

Cheers,
Andre'

>                        format(path, src, os.path.basename(src),
>                               self.debugfs, self._get_part_image(pnum))
>          else: # fat
> -- 
> 2.17.1
> 

Re: [PATCH v2] Fix cp's target path for ext* filesystems

[Leonardo Rangel]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

> On Fri, 2019-02-08 at 00:20 -0800, Leonardo Augusto wrote:
[...]
> >
> > diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py
> > index 4662c665c0..b4be796752 100644
> > --- a/scripts/lib/wic/engine.py
> > +++ b/scripts/lib/wic/engine.py
> > @@ -331,7 +331,7 @@ class Disk:
> >      def copy(self, src, pnum, path):
> >          """Copy partition image into wic image."""
> >          if self.partitions[pnum].fstype.startswith('ext'):
> > -            cmd = "echo -e 'cd {}\nwrite {} {}' | {} -w {}".\
> > +            cmd = "echo 'cd {}\nwrite {} {}' | {} -w {}".\
>
> This causes some shell's echos to output a literal \n (two characters,
'\'
> and 'n').
>
> The solution should be to use printf instead of echo...

Yes, indeed. I've just sent out a v3 based on your suggestion.

Thanks!

[-- Attachment #2: Type: text/html, Size: 1147 bytes --]