Re: [PATCH 1/3] sideband: mask control characters
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jan 15, 2025, 14:49 UTC
- Message-ID
- <f2ce08c4-f70e-487a-8dd9-286ee5bc683d@gmail.com>
- In-Reply-To
- <f7fb7a38333cf6527345e3dbefaeb2cd8ade6429.1736878772.git.gitgitgadget@gmail.com>
Hi Dscho
Just a couple of small comments
On 14/01/2025 18:19, Johannes Schindelin via GitGitGadget wrote:
Show 7 quoted lines
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> +static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n)
> +{
> + strbuf_grow(dest, n);
> + for (; n && *src; src++, n--) {
> + if (!iscntrl(*src) || *src == '\t' || *src == '\n')Isn't it a bug to pass '\n' to maybe_colorize_sideband() ?
> + strbuf_addch(dest, *src);
> + else {
> + strbuf_addch(dest, '^');
> + strbuf_addch(dest, 0x40 + *src);This will escape DEL ('\x7f') as "^\xbf" which is invalid in utf-8 locales. Perhaps we could use "^?" for that instead.
Show 10 quoted lines
> +test_expect_success 'disallow (color) control sequences in sideband' ' > + write_script .git/color-me-surprised <<-\EOF && > + printf "error: Have you \\033[31mread\\033[m this?\\n" >&2 > + exec "$@" > + EOF > + test_config_global uploadPack.packObjectshook ./color-me-surprised && > + test_commit need-at-least-one-commit && > + git clone --no-local . throw-away 2>stderr && > + test_decode_color <stderr >decoded && > + test_grep ! RED decoded
I'd be happier if we used test_cmp() here so that we check that the sanitized version matches what we expect and the test does not pass if there a typo in the script above stops it from writing the SGR code for red.
Best Wishes
Phillip