bugfindutils - Bugs: bug #61009, xargs need option to immediately...

 
 

bug #61009: xargs need option to immediately stop on command fail

Submitter:  None
Submitted:  Wed 04 Aug 2021 08:50:38 AM UTC
   
 
Category:  xargs Severity:  3 - Normal
Item Group:  None Status:  None
Privacy:  Public Assigned to:  berny
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  Open Release:  None
Fixed Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 11 Aug 2021 10:38:23 AM UTC, comment #9: 

comment #5:

> find . -type f \( -exec cp -t "${IMGDIR_DST}" {} + -o -quit \)


That will never evaluate -quit because -exec ... {} + is always true, as required by POSIX. (With the '+' terminator, command failure is communicated via find's exit status instead.)

$ find --version 2>&1 | head -n 1
find (GNU findutils) 4.6.0.225-235f
$ find . -type d \( -exec sh -c 'exit 1' {} + -o -print \)
$ echo $?
1
$ find . -type d \( -exec sh -c 'exit 1' {} \; -o -print \)
.
$ echo $?
0

Geoff Clare <geoffclare>
Tue 10 Aug 2021 08:57:02 AM UTC, comment #8: 

comment #7:

> The idiom 'find -type f | xargs -IX cp X ...' is per se unsafe:
> `xargs -I` reads the input line by line - but yes, files can
> have a newline in their name!


> Alternatively, use 'find ... -print0 | xargs -0 ...' instead.


Newlines in file names - of course. I forgot about it :-). But we can use -print0/-0 for this. No problem. -exec find option is good, but it can cause complex contructs with parentheses and etc. For simple cases -exec is good.

Anonymous
Tue 10 Aug 2021 06:46:23 AM UTC, comment #7: 


>> find . -type f | xargs -F -IX -n1 cp -f X $IMGDIR_DST/X
>>
>> I can't find any problem with unsafe filenames. Am i wrong?


Yes:

The idiom 'find -type f | xargs -IX cp X ...' is per se unsafe:
`xargs -I` reads the input line by line - but yes, files can
have a newline in their name!

Here's a reproducer using exactly your command line (without the
hypothetical -F option, obviously) to copy /etc/passwd ... although
that's for sure not what the user wants:


$ rm -rf src dst  # cleanup.

# Create a directory with in the SRC directory with a newline in the name,
# and initialize the DST directory.
$ mkdir -pv src/file$'\n'/etc dst/etc
mkdir: created directory 'src'
mkdir: created directory 'src/file'$'\n'
mkdir: created directory 'src/file'$'\n''/etc'
mkdir: created directory 'dst'
mkdir: created directory 'dst/etc'

$ cd src

$ IMGDIR_DST=../dst

# Create a dummy 'passwd' file therein.
$ echo DUMMY > file$'\n'/etc/passwd

# Add a dummy file which will hide that cp(1) will copy the wrong file.
$ echo HACKED > file

# Run the vulnerable command.
$ find . -type f | xargs -IX -n1 cp -f X $IMGDIR_DST/X

$ grep -R HACKED $IMGDIR_DST
../dst/file:HACKED

$ grep -RF $(whoami) $IMGDIR_DST
../dst/etc/passwd:victim:x:1003:100::/home/victim:/bin/bash


The safest way to avoid this problem is to let find(1) execute the program
directly, i.e., without the 'find | xargs' idiom.
Alternatively, use 'find ... -print0 | xargs -0 ...' instead.

Bernhard Voelker <berny>
Group administrator
Mon 09 Aug 2021 09:44:58 AM UTC, comment #6: 


comment #5:

> Sorry, that had an obvious bug, here's a fix:
>
> find . -type f \( -exec cp -t "${IMGDIR_DST}" {} + -o -quit \)


IMHO, this isn't less_complex :-). There are:
- \( \)
- \+
- -o -quit

This is complex. There are more than one place where i can mistake.

Anonymous
Sat 07 Aug 2021 12:13:56 PM UTC, comment #5: 

Sorry, that had an obvious bug, here's a fix:

find . -type f \( -exec cp -t "${IMGDIR_DST}" {} + -o -quit \)

James Youngman <jay>
Group administrator
Sat 07 Aug 2021 12:12:20 PM UTC, comment #4: 

The proposed

find . -type f | xargs -IX -n1 sh -c "cp -f X $IMGDIR_DST/X || exit 255" || exit 1


can be implemented more efficiently without using the hypothetical -F flag:

find . -type f -exec cp -t "${IMGDIR_DST}" {} + -o -quit

This approach is also less "complex and error prone" than the approach using xargs and lots of flags.

James Youngman <jay>
Group administrator
Thu 05 Aug 2021 10:17:44 AM UTC, comment #3: 


> This is my guess because the shown use of find/xargs
> is quite excessive (one cp(1) invocation per file) and unsafe (unusual filenames
> would break the construct).


About unsafe filenames. If you talk about this:

find . -type f | xargs -IX -n1 sh -c "cp -f X $IMGDIR_DST/X || exit 255"

I understand how unsafe filenames can cause a wrond behaviour. But here(if -F/-S would be implemented):

find . -type f | xargs -F -IX -n1 cp -f X $IMGDIR_DST/X

I can't find any problem with unsafe filenames. Am i wrong?

Anonymous
Thu 05 Aug 2021 09:22:48 AM UTC, comment #2: 

comment #1:

> POSIX [1] specifies the following conditions when xargs shall terminate processing:
> * child exits with 255,
> * child was killed by a signal, or
> * when reading the 'eofstr' string if the '-E eofstr' option is given, or
> * when regularly reaching EOF when reading from stdin.


POSIX standardize those features that are already implemented by somebody ;-).

> Still, our xargs might implement such an option as a GNU extension.


That's great!

> I would recommend a long-option like --stop-on-error for such an extension.


This would be great if we have short option in addition to long, because this is a normal(usual) case when we want to early stop on error, i think.

> Why did you propose '-F'?  Is there any precedence in other xargs implementations?


No. This is my invention just for demonstration :-). I think about it like -F = --fail-on-error. But -S for --stop-on-error will be good too.

> Regarding the examples - and just to be sure:
> I assume that the reproducer with 'cp' was just a simplification to demonstrate your actual use case, right?


Yes. You are right.


Anonymous
Wed 04 Aug 2021 11:04:00 PM UTC, comment #1: 

I have to confess that I like the idea of having an option to immediately stop processing
when an invocation of the command exits with a non-Zero status.

POSIX [1] specifies the following conditions when xargs shall terminate processing:

  • child exits with 255,
  • child was killed by a signal, or
  • when reading the 'eofstr' string if the '-E eofstr' option is given, or
  • when regularly reaching EOF when reading from stdin.

Still, our xargs might implement such an option as a GNU extension.
I would recommend a long-option like --stop-on-error for such an extension.
Why did you propose '-F'?  Is there any precedence in other xargs implementations?

[1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/xargs.html

Regarding the examples - and just to be sure:
I assume that the reproducer with 'cp' was just a simplification to demonstrate
your actual use case, right?  This is my guess because the shown use of find/xargs
is quite excessive (one cp(1) invocation per file) and unsafe (unusual filenames
would break the construct).
In such a case - when launching one process per file is required -, I'd maybe
better use the `find -exec ... '{}' \;' syntax without xargs, which could also be
used in conjunction with -quit to terminate the processing when 'cp' fails:


# Create source and destination directory.
$ mkdir src dst

# Create a regular file 'dst/f' which will cause 'cp' to fail later.
$ touch dst/f

# Create test sub-directories 'a' .. 'g', and a file 'x' in each of them.
$ cd src
$ mkdir a b c d e f g
$ touch a/x b/x c/x d/x e/x f/x g/x

# Run find to copy all files to 'dst', but stopping if 'cp' hits an error.
# Note the use of the '(' ... ')' to enforce the wanted OR-precedence for -quit
# when cp(1) fails.
$ find . -type f '(' -exec cp -fv '{}' '../dst/{}' \; -o -quit ')'
'./b/x' -> '../dst/./b/x'
cp: cannot create regular file '../dst/./b/x': No such file or directory

# Alternatively, avoid the OR-syntax with -not before -exec:
$ find . -type f -not -exec cp -fv '{}' '../dst/{}' \; -quit


Bernhard Voelker <berny>
Group administrator
Wed 04 Aug 2021 08:50:38 AM UTC, original submission:  

Hello.

If we want xargs stop on command fail we need to do the next complex construct with sh:

xargs -IX -n1 sh -c "cp -f X $IMGDIR_DST/X || exit 255"

instead of simply:

xargs -F -IX -n1 cp -f X $IMGDIR_DST/X

(imagine the -F turns on stop on command fail). This is simpler and less error prone.

Thus, we can do in our scripts:

find . -type f | xargs -F -IX -n1 cp -f X $IMGDIR_DST/X || exit 1

Instead of more complex and error prone:

find . -type f | xargs -IX -n1 sh -c "cp -f X $IMGDIR_DST/X || exit 255" || exit 1

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by geoffclare (Posted a comment)
  • -email is unavailable- added by jay (Posted a comment)
  • -email is unavailable- added by berny (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-08-04 berny Assigned toNone berny

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code