Bug#931719: 40-systemd: line 11: 1: unbound variable when sourcing /lib/lsb/init-functions with $1 unset

[Mert Dirik]

On 7/18/19, Michael Biebl <biebl at debian.org> wrote:
> Am 18.07.19 um 13:55 schrieb Michael Biebl:
>> Am 11.07.19 um 17:41 schrieb Mert Dirik:
>>> On 7/11/19, Thomas Bätzler <thomas at baetzler.de> wrote:
>>>>
>>>> To wit:
>>>>
>>>>   root at kvm1:~# /etc/init.d/rsync
>>>>   Usage: /etc/init.d/rsync
>>>> {start|stop|reload|force-reload|restart|status}
>>>>
>>>> vs.
>>>>
>>>>   root at kvm1:~# /etc/init.d/mysql
>>>>   /lib/lsb/init-functions.d/40-systemd: line 11: 1: unbound variable
>>>>
>>>>
>>>
>>> Thanks, I've been able to confirm that.
>>>
>>>> In this case, line 16 has to be changed to 'argument="${1:-}', too
>>>> (last
>>>> line of your patch), since the call without argument falls through to
>>>> the else branch, i.e.
>>>
>>> Indeed. "argument=$1" line had need to be changed, not the
>>> "argument=$2" one. Can you try this patch:
>>>


>>
>>>      elif [ "${0##*/}" = "init-d-script" ] ||
>>> -         [ "${0##*/}" = "${1##*/}" ]; then # scripts run with old
>>> init-d-script
>>> +         [ "${0}" = "${1:-}" ]; then # scripts run with old
>>> init-d-script
>>
>> Why  do we need ${1:-1} here...
>>
>>>          executable="$1"
>>>          argument="$2"
>>
>> ..but no checks here?
>
> Nvm, I see now why [ "${0}" = "${1:-}" ] is necessary for the
> fallthrough case.
>
>
> I will apply Mert's patch, as is then.
>

Sorry for the late reply.

> fwiw, trying to use -u with old style init-d-script and new style
> __init_d_script_name yields a lot of unset errors in /lib/init/vars.sh
> and /lib/init/init-d-script.
> I guess this needs someone interested to address this.
>

> I will apply Mert's patch, as is then.
>

I agree. As you have noticed it doesn't solve all the potential issues
with set -u but at least it does solve this specific issue without
touching other stuff.


>>          executable="$__init_d_script_name"
>>          argument="$1"
>
> Is $1 guaranteed to be set/not an issue with __init_d_script_name?

Actually, not.
I think it should be OK in practice, as /lib/init/init-d-script is
sourced as the first thing in the template in the man page, so
hopefully nobody puts a "set -u" before that. I *guess* it doesn't
hurt changing it into "${1:-}" either, just not in this patch.


>>>          executable="$1"
>>>          argument="$2"
>>
>> ..but no checks here?

Here, "$1" is guaranteed to be set (at least after the patch is
applied), but $2 is not. So by the same reasoning above, I don't think
that would be an issue in practice but I guess it wouldn't hurt
changing these to "${1:-}" and "${2:-}", either.