diff options
author | Amitai Schlair <schmonz@magnetic-babysitter.(none)> | 2009-09-03 19:24:42 -0500 |
---|---|---|
committer | Amitai Schlair <schmonz@magnetic-babysitter.(none)> | 2009-09-03 19:24:42 -0500 |
commit | 1888464a0d9de9cf4ac40c298924018437758aa2 (patch) | |
tree | b55aedb4983dd142e966ade8e270fd1d33fc2834 /doc/plugins/contrib/cvs | |
parent | 85844b8494dd5e032d06afd623c25b4bb4a72578 (diff) | |
parent | f582457ea04a6ce66a2c54ec0731e2569aa30d3f (diff) |
Merge branch 'master' of git://github.com/joeyh/ikiwiki
Diffstat (limited to 'doc/plugins/contrib/cvs')
-rw-r--r-- | doc/plugins/contrib/cvs/discussion.mdwn | 29 |
1 files changed, 29 insertions, 0 deletions
diff --git a/doc/plugins/contrib/cvs/discussion.mdwn b/doc/plugins/contrib/cvs/discussion.mdwn index e142452d0..b5f03ac6f 100644 --- a/doc/plugins/contrib/cvs/discussion.mdwn +++ b/doc/plugins/contrib/cvs/discussion.mdwn @@ -93,3 +93,32 @@ the "cvs add <directory>" call and avoid doing anything in that case? >>> if a configured post-commit hook is missing, and it seems fine, >>> probably also thanks to IPC::Cmd. >>> --[[schmonz]] + +---- + + +Further review.. --[[Joey]] + +I don't understand what `cvs_shquote_commit` is +trying to do with the test message, but it seems +highly likely to be insecure; I never trust anything +that relies on safely quoting user input passed to the shell. + +(As an aside, `shell_quote` can die on certian inputs.) + +Seems to me that, if `IPC::Cmd` exposes input to the shell +(which I have not verified but its docs don't specify; a bad sign) +you chose the wrong tool and ended up doing down the wrong +route, dragging in shell quoting problems and fixes. Since you +chose to use `IPC::Cmd` just because you wanted to shut +up CVS stderr, my suggestion would be to use plain `system` +to run the command, with stderr temporarily sent to /dev/null: + + open(my $savederr, ">&STDERR"); + open(STDERR, ">", "/dev/null"); + my $ret=system("cvs", "-Q", @_); + open(STDERR, ">$savederr"); + +`cvs_runcvs` should not take an array reference. It's +usual for this type of function to take a list of parameters +to pass to the command. |