Austin Group Defect Tracker

Aardvark Mark IV

Main | My View | View Issues | Change Log | Docs
  

Viewing Issue Simple Details [ Jump to Notes ] [ Issue History ] [ Print ]
ID Category Severity Type Date Submitted Last Update
0001440 [1003.1(2016/18)/Issue7+TC2] System Interfaces Comment Omission 2021-01-04 15:49 2024-06-11 09:08
Reporter ciprian_craciun View Status public  
Assigned To
Priority normal Resolution Accepted As Marked  
Status Closed  
Name Ciprian Dorin Craciun
Organization
User Reference
Section system(3)
Page Number N/A
Line Number N/A
Interp Status Approved
Final Accepted Text Note: 0005510
Summary 0001440: Calling `system("-some-tool")` fails (although it is a valid `sh` command)
Description This report is in relation to the following `glibc` and Linux man-pages issues:
* https://sourceware.org/bugzilla/show_bug.cgi?id=27143 [^]
* https://bugzilla.kernel.org/show_bug.cgi?id=211029 [^]

However it seems that the root cause is the interaction mandated by the POSIX standard between `system(3)` and `sh(1)`.


In simple terms, calling `system("-some-tool")` has an unexpected behaviour when delegating that to `sh`, as in `sh -c -some-tool`. (For how to replicate this just see the first link.)


In more details:

* the `system(3)` specification requires that:
~~~~
# from: https://pubs.opengroup.org/onlinepubs/9699919799/functions/system.html [^]

The system() function shall behave as if a child process were created using fork(), and the child process invoked the sh utility using execl() as follows:

execl(<shell path>, "sh", "-c", command, (char *)0);
~~~~

* however the `sh(1)` specification requires that:
~~~~
# from https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sh.html [^]

sh -c [-abCefhimnuvx] [-o option]... [+abCefhimnuvx] [+o option]...
       command_string [command_name [argument...]]
~~~~

I.e. `sh(1)` allows extra flags between `-c` and the `command_string`; however the `system(3)` doesn't account for a command that might start with a hyphen.

Thus, depending on what the command might be, the outcome is either a missing command (if the given command resembles an option accepted by `sh`), or an unknown option (if the given command doesn't resemble an option accepted by `sh`).
Desired Action The "optimal" solution would be to require `system(3)` to call `sh(1)` with an extra `--` between `-c` and the `command`, as in:
~~~~
execl(<shell path>, "sh", "-c", "--", command, (char *)0);
                                ^^^^
~~~~

Given that `sh(1)` already mandates a compliant shell to correctly handle `--`, this shouldn't have many unintended consequences.

Alternatively add a warning in the specification stating that a command starting with `-` has an undefined behaviour and should be worked-around by preperding a space to any such command.
Tags issue8
Attached Files

- Relationships
related to 0000252Closedajosey 1003.1(2008)/Issue 7 dot should follow Utility Syntax Guidelines 
related to 0000163Closedajosey 1003.1(2008)/Issue 7 Problems in rationale of trap shell builtin 
related to 0001529Closed 1003.1(2016/18)/Issue7+TC2 ex: follow-up to issue #1440 

-  Notes
(0005191)
Vincent Lefevre (reporter)
2021-01-06 13:20

 Since with -c, there must be at least a non-option argument, how about doing a correction/improvement in sh as follows? If -c is given and there is a single argument after -c, then this argument should be regarded as a command string.

(This could be done in addition to adding "--" on the system() side.)
(0005192)
ciprian_craciun (reporter)
2021-01-06 15:41

 > Since with -c, there must be at least a non-option argument, how about doing a correction/improvement in sh as follows? If -c is given and there is a single argument after -c, then this argument should be regarded as a command string.


I think adding such a requirement is not advisable for multiple reasons:

* first, none of the existing `sh` implementations (as reported in one of the tickets none of `bash`, `dash` or `zsh`) exhibit such a behaviour, therefore this change will all of a sudden break their POSIX compliance;

* second, although it solves the `system` corner-case, it opens other potential issues; imagine for example a script that calls `sh -c -x "${@}"`; now if the user doesn't provide an argument, then all of a sudden `sh` executes `-x` as a command, as opposed to until now when it didn't execute anything;


Personally I don't know what is the best course of action here, besides updating `system` to use the `--` separator which is backward compatible and shouldn't break virtually anything.

However, given how much "legacy" there is based on `sh`, I wouldn't change its behaviour, except perhaps giving a warning in the man page about the situation.
(0005193)
joerg (reporter)
2021-01-06 16:12

 Please note that this problem has been introduced by POSIX. It does not
exist in pre-POSIX shells.

In the classical Bourne Shell, -c wasn't even seen as a real option but
as some kind of prefix to the command argument.

Since the problem has been introduced by shells while implementing POSIX
requirements, I am fully open to set up a new POSIX requirement for shell
option parsing that would fix the parsing to make it compatible to the
historic Bourne Shell again.

This would however require to find a simple and still always working
algorithm. Just counting the arguments after -c does not work.

Note that with the classical Bourne Shell

    sh -x -c 'echo foo'

does not work, but requiring it to work by POSIX seems to be a useful
enhancement compared to the classical Bourne Shell and the fact that
POSIX requires

    sh -c -x 'echo foo'

to work also seems to be a useful extension.

As a result, it may be hard to fix this problem in the shell...
(0005194)
kre (reporter)
2021-01-06 16:13
edited on: 2021-01-06 16:22

 Re Note: 0005191 and Note: 0005192

Not only is the suggestion in Note: 0005191 not the right thing to do,
as showin in Note: 0005192 (it would also cause sh to not satisfy the
utility guidelines in XBD 12.1)

But beyond that, it would also be insufficient to solve the problem
in general, as there can be more than one arg after -c, and the first
of those might begin with a "-" - which means the shell would either
fail just the same way (just not perhaps when called from system()) or
would run the wrong thing as a command.

This needs to be fixed in system(), or in the application, not in sh.

That could be done in system() adding a -- in the arg list, perhaps only if
the command string starts with a '-', or by prefixing a space to the arg
(though that would mean copying it, so probably would not be the method of
choice), or however else the system() function wants to do it.

Or we could simply note the potential issue in the APPLICATION USAGE of
system() and tell application authors not to attempt system() (or popen())
with a string starting with a '-' - there are a zillion ways (including
prependiong a space, or quoting the command name, or ...) the application
can avoid that ever happening.

Personally, I would put the onus on the application - the way system()
works has neevr really changed, the way sh works has never changed, it
has always been necessary for the application to deal with the combination
that results from that if they're doing something unusual (like starting
a command with a '-').
(0005199)
Vincent Lefevre (reporter)
2021-01-13 10:50

 And what about the invocation of $EDITOR and $VISUAL?

A proposed manpages patch for Debian at https://bugs.debian.org/866060 [^] has:

 .BR EDITOR / VISUAL
 The user's preferred utility to edit text files.
+Any string acceptable as a command_string operand to the
+.I sh -c
+command shall be valid.

Executing the command with "sh -c" will fail if it starts with "-". But I wonder how this is handled in practice in various implementations and whether something is missing in the spec, which is not very explicit.
(0005510)
geoffclare (manager)
2021-10-28 15:35
edited on: 2021-11-04 15:36

 Interpretation response
------------------------
The standard states that system() behaves as if it passes "-c" followed by the command string to execl() , and conforming implementations must conform to this. However, concerns have been raised about this which are being referred to the sponsor.

Rationale:
-------------
There is nothing known that applications can usefully do if the "--" is omitted, therefore there is no reason that the standard should not require the "--".
The standard warns applications not to use filenames that start with <hyphen-minus> ('-'). (See the definition of Portable Filename in XBD chapter 3). However, there is no equivalent warning for filenames beginning with <plus-sign> ('+'), and applications should be able to use system() to execute commands where the command word begins with '+' (without the need for special precautions such as adding a <space> at the beginning).

Notes to the Editor (not part of this interpretation):
-------------------------------------------------------

On page 1437 line 47730 section popen(), change:
execl(shell path, "sh", "-c", command, (char *)0);
where shell path is ...
to:
execl(<shell path>, "sh", "-c", "--", command, (ch
ar *)0);
where <shell path> is ...

    
On page 2107 line 67568 section system(), change:
execl(<shell path>, "sh", "-c", command, (char *)0);
to:
execl(<shell path>, "sh", "-c", "--", command
, (char *)0);


(Note: unwanted line splitting in the above is a Mantis artifact.)

Add on page 1439 after line 47807 and on page 2110 after line 67736:
Earlier versions of this standard required the command string to be passed as the next argument after "-c" (omitting "--"). This meant that portable applications needed to take care not to pass a command string beginning with <hyphen-minus> ('-') or <plus-sign> ('+'), as it would then be interpreted as containing options. Now that implementations are required to pass the "--", applications no longer need to do this.
(0005512)
agadmin (administrator)
2021-10-29 08:44
edited on: 2021-11-04 17:56

 An interpretation is proposed: 4th November 2021 (note the timer is restarted)
(0005548)
agadmin (administrator)
2021-12-09 11:47

 Interpretation approved: 9 Dec 2021
(0005597)
geoffclare (manager)
2022-01-13 09:58

 When applying this bug I noticed that the example implementations (in RATIONALE) of popen() and system() also needed "--" to be added, and I made those additions. (Two for popen() and one for system().)

- Issue History
Date Modified Username Field Change
2021-01-04 15:49 ciprian_craciun New Issue
2021-01-04 15:49 ciprian_craciun Name => Ciprian Dorin Craciun
2021-01-04 15:49 ciprian_craciun Section => system(3)
2021-01-04 15:49 ciprian_craciun Page Number => N/A
2021-01-04 15:49 ciprian_craciun Line Number => N/A
2021-01-06 13:20 Vincent Lefevre Note Added: 0005191
2021-01-06 15:41 ciprian_craciun Note Added: 0005192
2021-01-06 16:12 joerg Note Added: 0005193
2021-01-06 16:13 kre Note Added: 0005194
2021-01-06 16:22 kre Note Edited: 0005194
2021-01-13 10:50 Vincent Lefevre Note Added: 0005199
2021-10-28 15:35 geoffclare Note Added: 0005510
2021-10-28 15:36 geoffclare Note Edited: 0005510
2021-10-28 15:37 geoffclare Interp Status => Pending
2021-10-28 15:37 geoffclare Final Accepted Text => Note: 0005510
2021-10-28 15:37 geoffclare Status New => Interpretation Required
2021-10-28 15:37 geoffclare Resolution Open => Accepted As Marked
2021-10-28 15:37 geoffclare Tag Attached: tc3-2008
2021-10-29 08:44 agadmin Interp Status Pending => Proposed
2021-10-29 08:44 agadmin Note Added: 0005512
2021-11-04 15:17 eblake Relationship added related to 0000252
2021-11-04 15:28 eblake Relationship added related to 0000163
2021-11-04 15:36 geoffclare Note Edited: 0005510
2021-11-04 15:37 geoffclare Tag Detached: tc3-2008
2021-11-04 15:37 geoffclare Tag Attached: issue8
2021-11-04 17:56 agadmin Note Edited: 0005512
2021-12-05 01:11 Don Cragun Relationship added related to 0001529
2021-12-09 11:47 agadmin Interp Status Proposed => Approved
2021-12-09 11:47 agadmin Note Added: 0005548
2022-01-13 09:58 geoffclare Note Added: 0005597
2022-01-13 09:58 geoffclare Status Interpretation Required => Applied
2024-06-11 09:08 agadmin Status Applied => Closed

Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker