On 23/06, Lukas Fleischer wrote:
On Tue, 23 Jun 2015 at 00:00:18, Johannes Löthberg wrote:Also add an utility function for formatting the ForceCommandMissing punctuation :)
Ah, silly me.
+def format_command(env_vars, command, ssh_opts, key): + environment = '' + for key, var in env_vars.items(): + environment += '{}={} && '.format(key, var)I like this idea. I noticed three things, though: 1. We do not seem to use format() anywhere else. Maybe use the % operator for consistency? But then again, % is obsolete, so we should probably rather replace all the existing formatting operations by format() (in another patch)...
Yeah, I'll see about replacing the other uses.
2. Is there any reason to &&-chain the environment variable assignments? Does this even work? When I run FOO=bar sh -c 'echo $FOO' it prints "bar" as expected, but when I insert "&&" between the first two tokens it doesn't.
Thing is that the command will actually be sh -c 'FOO=bar echo $FOO' which won't work. Either sh -c 'FOO=bar && echo $FOO' or sh -c 'FOO=bar; echo $FOO' is needed.
3. I notice this is not strictly needed now but if we write a helper function, it might be a good idea to quote the arguments (e.g. using shlex.quote). It is not unlikely that we want to add arguments that may contains spaces some day. This is the most security critical part of the Git interface. If we do not escape things properly, users might be able to execute arbitrary code on the server. We should be extra cautious here...
Yes, that's fair, will add that. -- Sincerely, Johannes Löthberg PGP Key ID: 0x50FB9B273A9D0BB5 https://theos.kyriasis.com/~kyrias/
signature.asc
Description: PGP signature
