Remove 'login' builtin?

[Martijn Dekker]

Does anyone care about having a 'login' builtin?

This command is already provided by every Unixy OS. I don't know of any
system that relies on the ksh 'login' builtin. Maybe it was different in
the 80s, but I would be surprised if anything actually uses it now.

On my Mac, it does manage to verify my password and apparently start a
login subsession. But of course I can only do this if I'm already logged
in anyway, and in any case, the OS has a perfectly good /usr/bin/login.

The builtin is almost completely undocumented. The manual page doesn't
mention it at all, and 'login --man' only produces an uninformative
usage message.

What's worse, ksh pointlessly classes it as a special builtin, so (among
a few other things) it is impossible to use 'login' as a shell function
name. At minimum, that should be fixed (which is very easily done by
removing the BLT_SPC flag from the definition in data/builtins.c).

What say you? Get rid?

- Martijn (ksh 93u+m maintainer)

--
|| modernish -- harness the shell
|| https://github.com/modernish/modernish
||
|| KornShell lives!
|| https://github.com/modernish/ksh

[Martijn Dekker]

Op 11-06-20 om 13:48 schreef Martijn Dekker:

> Does anyone care about having a 'login' builtin?

I'll take the silence as a no.

In future I should really look at the code before posting, though.
These misfeatures aren't what I thought they were:

commit d8eba9d11286f8d2167ab6b3077326c8388f579f (HEAD -> master)
Author: Martijn Dekker <mar...@inlv.org>
Date: Fri Jun 12 06:57:57 2020 +0200

Remove 'login' and 'newgrp' builtins: not sane default behaviour

This commit removes the undocumented 'login' and 'newgrp' builtin
commands. They already stopped blocking shell functions by that
name by changing from special to regular builtins in 04b91718 (a
change I forgot to mention in that commit message), but there is
another obnoxious aspect to these: being glorified hooks into
'exec', they replaced your shell session with the external commands
by the same name. This makes argument and error checking
impossible, so if you made so much as a typo, you would be
immediately logged out.

Even if that behaviour is wanted by a few, having it as the default
is user-hostile enough to be called a bug. It also violates the
POSIX definition of the 'newgrp' utility which explicitly says that
it "shall create a new shell execution environment", not replace
the existing one.
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/newgrp.html

Users who do want this behaviour can easily restore it by setting:
alias login='exec login'
alias newgrp='exec newgrp'

src/cmd/ksh93/bltins/misc.c:
- As there is no more 'login' builtin, combine b_exec() and
B_login() functions, which allows eliminating a few variables.
Note that most of 'exec' was actually implemented in B_login()!

src/cmd/ksh93/data/builtins.c:
- Remove "login" and "newgrp" table entries.

src/cmd/ksh93/include/builtins.h:
- Remove SYSLOGIN parser ID. As this was the first, all the others
needed renumbering.

src/cmd/ksh93/sh/xec.c:
- Remove SYSLOGIN parser check that made 'login' and 'newgrp' act
like 'exec' and replace the shell.

[Danny Weldon]

> I'll take the silence as a no.

You didn't wait very long. :) I for one am very busy.

> The builtin is almost completely undocumented. The manual page doesn't
> mention it at all, and 'login --man' only produces an uninformative
> usage message.

It is briefly mentioned in the first paragraph under the "Built-in
Commands." section:

is no syntax error, is zero. Except for :, true, false, echo, newgrp,
and login, all built-in commands accept -- to indicate end of options.

I imagine that login and newgrp were written to replace the current
shell for convenience, but perhaps since they weren't properly
documented, the other shells diverged, so removing it seems the
correct way to go in this case to maintain compatibility. Personally,
I would still consider moving them to libcmd instead.

So don't forget to remove them from the man page, if you haven't already.

> --
> You received this message because you are subscribed to the Google Groups "Korn Shell" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to korn-shell+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/korn-shell/fc4ab96f-d9a9-d7f9-1ec2-cae2a0923cfc%40inlv.org.



--
Regards

Danny

[Martijn Dekker]

Op 14-06-20 om 15:43 schreef Danny Weldon:

>> I'll take the silence as a no.
>
> You didn't wait very long. :) I for one am very busy.

Fair. Sorry about that. I will admit to being rather impatient to get as
much fixed as possible before I stop having as much time myself.

[quoth the manual]

> is no syntax error, is zero. Except for :, true, false, echo, newgrp,
> and login, all built-in commands accept -- to indicate end of options.

That info is wrong anyway. They were nothing but a couple of glorified
hooks into 'exec', so whether they accept '--' or not depends entirely
on the OS's implementation of 'login' and 'newgrp'. On my Mac, they do
accept it, as they should do on any POSIX-compliant system.

> I imagine that login and newgrp were written to replace the current
> shell for convenience, but perhaps since they weren't properly
> documented, the other shells diverged, so removing it seems the
> correct way to go in this case to maintain compatibility. Personally,
> I would still consider moving them to libcmd instead.

First -- not possible without pretty much duplicating 'exec', which is
rather pointless.

Second -- nah. The correct way to have this behaviour, if wanted, is for
the user to set a couple of very simple aliases:

alias login='exec login'
alias newgrp='exec newgrp'

...or functions:
function login { exec login "$@"; }
function newgrp { exec newgrp "$@"; }
This makes the nonstandard behaviour explicit in whatever file sets
those and show up in the output of 'alias' or 'functions', as it should
do. Anything else violates POSIX (for newgrp) and the principle of least
astonishment (for both commands).

> So don't forget to remove them from the man page, if you haven't already.

I did in fact forget about that. Done now. Thanks.

- Martijn

--
|| modernish -- harness the shell
|| https://github.com/modernish/modernish
||
|| KornShell lives!

|| https://github.com/ksh93/ksh

[Thomas Swan]

The built-in is also a rescue facility.  It also provides a consistent set of options which may not be available on all supported operating systems.  One of the things a good shell provides, is a consistent interface to specific commands or a reasonable fallback.  I would leave it alone for the time being. If somebody wants to use the operating system login command, they can specify the full path otherwise they would use the built in.

--
You received this message because you are subscribed to the Google Groups "Korn Shell" group.
To unsubscribe from this group and stop receiving emails from it, send an email to korn-shell+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/korn-shell/c0448e31-30d4-6ab2-1a70-c0e165f157ab%40inlv.org.

[Martijn Dekker]

Op 14-06-20 om 23:10 schreef Thomas Swan:

> The built-in is also a rescue facility.  It also provides a consistent
> set of options which may not be available on all supported operating
> systems.

Please read the relevant thread on the list. You seem to have missed the
fact that the 'login' and 'newgrp' builtins did absolutely nothing
except 'exec login' and 'exec newgrp'. Meaning, they provided no
functionality or options of their own -- simply replaced the current
shell session by the operating system's external commands.

So, if the external commands by the same name don't exist, then not only
do these commands not run anything, they kill your shell session. The
same happens if you make some typo in the arguments.

Which is sheer sadism towards the user. It should absolutely not be the
shell's default behaviour.

- M.

--
|| modernish -- harness the shell
|| https://github.com/modernish/modernish
||
|| KornShell lives!

|| https://github.com/ksh93/ksh

[Thomas Swan]

On Sun, Jun 14, 2020 at 4:18 PM Martijn Dekker <mar...@inlv.org> wrote:

Please read the relevant thread on the list. You seem to have missed the
fact that the 'login' and 'newgrp' builtins did absolutely nothing
except 'exec login' and 'exec newgrp'. Meaning, they provided no
functionality or options of their own -- simply replaced the current
shell session by the operating system's external commands.

So, if the external commands by the same name don't exist, then not only
do these commands not run anything, they kill your shell session. The
same happens if you make some typo in the arguments.

Which is sheer sadism towards the user. It should absolutely not be the
shell's default behaviour.

 

The statement was generalized for most built-ins, and I apologize for the ambiguity. For login and newgrp you are correct in that it simply calls exec ...; however, it is important to note that the login and newgrp built-ins replace the current process which is distinct from spawning a new process and returning to the current process.  That latter provides a command continuation path beyond the built-in command which is where my concern lies.

If someone is expecting their script or .profile to stop processing after calling newgrp (not using the full path) then they are in for surprise when the .profile or script exits from newgrp and continues processing.  Either they are presented with a shell as the original calling user for logins in the .profile context, or the script will continue.  Changing the behavior could very well result in root exposure given that login and newgrp would probably be used by a privileged user.

An existing ksh script or .profile would be expecting the exec behavior because the built-in commands are documented as such.  There is no surprise here except for new users not expecting the behavior.  In the name of no backward incompatible changes, I would recommend against the change.

If the exec without an executable is a major concern, I would rather the shell terminate and exit with an appropriate error such as "no such file" perhaps with a note pointing to the ksh documentation if the real command is not present on the system.  It must exit, not continuing any processing as if it had exec'd to avoid causing the aforementioned issues.

--

Thomas Swan