On Sat, 2019-09-21 at 18:21 +0200, Nicolas Boulenguez wrote: > Could you give an example of unsafety not yet covered by Confvar tests? > I fail to understand what you mean by reading your commits.
My commits were about establishing a protocol between the generated
config parser bash script and the Perl code calling that bash script
such that the Perl code does not have to have knowledge of how shell
quoting works, only of how to establish the protocol from shell.
It does that by avoiding layering violations between shell and Perl;
passing filenames as arguments and returning variables NUL separated.
> My point was that your sentence suggests
> - that several scripts use Config. Actually, only one does.
I count three scripts that use Config (salsa, uscan, mk-origtargz).
> Could you detail some of the needed differences?
A few thoughts:
Don't use an absolute path to bash.
Unsetting the environment represents a change in the interface between
devscripts and people's config files. An example of what could break:
if someone sets HOME in their running zsh to a subdirectory, and in
that directory has a devscripts config that uses ~/ then unsetting HOME
could mean that the devscripts config would use their main home
directory instead of the subdirectory they wanted to use.
Checking the syntax of the config files represents a change in the
interface between devscripts and people's config files; config files
that contain valid variables at the start and syntax errors at the end
would stop setting the valid variables from the start.
Telling bash to exit on errors by passing the -e option to bash
represents a change in the interface between devscripts and people's
config files. It could cause later parts of the config files to
silently stop working if they have a command near the start that does
not succeed.
In the perl code use q{}/qq{} instead of ''/"" for readability; this is
so that you can use quoting characters in the shell code without
escaping them with backslash characters, which reduce the readability.
Use the spawn function from Dpkg::IPC instead of backticks because they
introduce an extra unnecessary shell process while spawn does not use
the shell so there can be no quoting issues no matter what is passed.
Pass the config filenames as arguments to the generated bash script and
load the files from those arguments instead of from quoted strings.
This avoids any possible mismatch between bash/Perl quoting code.
Use NUL characters (\0) to split the output up, this allows all
characters to be represented instead of relying on Perl code to parse
the output of the bash set command.
One enhancement I should have added to the Config module but didn't
think of until now is to ensure that the stderr isn't parsed and that
any output the devscripts config generates also isn't parsed by the
Perl code. Probably the way to do this is to print a few NUL characters
in a row to indicate the start of the exported variables and discard
any output before those NUL characters. Of course the devscripts config
could print the NULs too and thus break the parsing, but outputting
NULs from one's devscripts config seems unlikely to exist today.
> It seemed right to answer a bug requesting help in the bug log,
> but let us try a merge request instead.
> https://salsa.debian.org/debian/devscripts/merge_requests/162
This seems like a few separate issues all on one branch, usually they
would be separate merge requests. I'd also squash the new code plus
fixes to and perltidy of the new code into one commit.
--
bye,
pabs
https://wiki.debian.org/PaulWise
signature.asc
Description: This is a digitally signed message part
