Skip to content

Commit 68a45c3

Browse files
gitstergitster
committed
Merge branch 'js/neuter-sideband' into seen
Invalidate control characters in sideband messages, to avoid terminal state getting messed up. Comments? * js/neuter-sideband: sideband: delay sanitizing by default to Git v3.0 sideband: offer to configure sanitizing on a per-URL basis sideband: add options to allow more control sequences to be passed through sideband: do allow ANSI color sequences by default sideband: introduce an "escape hatch" to allow control characters sideband: mask control characters
2 parents 58a9142 + d3a5fdd commit 68a45c3

File tree

6 files changed

+341
-2
lines changed

6 files changed

+341
-2
lines changed

‎Documentation/config.adoc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,8 @@ include::config/sequencer.adoc[]
523523

524524
include::config/showbranch.adoc[]
525525

526+
include::config/sideband.adoc[]
527+
526528
include::config/sparse.adoc[]
527529

528530
include::config/splitindex.adoc[]

‎Documentation/config/sideband.adoc‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
sideband.allowControlCharacters::
2+
ifdef::with-breaking-changes[]
3+
By default, control characters that are delivered via the sideband
4+
are masked, except ANSI color sequences. This prevents potentially
5+
endif::with-breaking-changes[]
6+
ifndef::with-breaking-changes[]
7+
By default, no control characters delivered via the sideband
8+
are masked. This is unsafe and will change in Git v3.* to only
9+
allow ANSI color sequences by default, preventing potentially
10+
endif::with-breaking-changes[]
11+
unwanted ANSI escape sequences from being sent to the terminal. Use
12+
this config setting to override this behavior (the value can be
13+
a comma-separated list of the following keywords):
14+
+
15+
--
16+
`default`::
17+
ifndef::with-breaking-changes[]
18+
Allow any control sequence. This default is unsafe and will
19+
change to `color` in Git v3.*.
20+
endif::with-breaking-changes[]
21+
`color`::
22+
Allow ANSI color sequences, line feeds and horizontal tabs,
23+
but mask all other control characters. This is the default.
24+
`cursor:`:
25+
Allow control sequences that move the cursor. This is
26+
disabled by default.
27+
`erase`::
28+
Allow control sequences that erase charactrs. This is
29+
disabled by default.
30+
`false`::
31+
Mask all control characters other than line feeds and
32+
horizontal tabs.
33+
`true`::
34+
Allow all control characters to be sent to the terminal.
35+
--
36+
37+
sideband.<url>.*::
38+
Apply the `sideband.*` option selectively to specific URLs. The
39+
same URL matching logic applies as for `http.<url>.*` settings.

‎sideband.c‎

Lines changed: 183 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "help.h"
1111
#include "pkt-line.h"
1212
#include "write-or-die.h"
13+
#include "urlmatch.h"
1314

1415
struct keyword_entry {
1516
/*
@@ -26,6 +27,91 @@ static struct keyword_entry keywords[] = {
2627
{ "error", GIT_COLOR_BOLD_RED },
2728
};
2829

30+
static enum {
31+
ALLOW_CONTROL_SEQUENCES_UNSET = -1,
32+
ALLOW_NO_CONTROL_CHARACTERS = 0,
33+
ALLOW_ANSI_COLOR_SEQUENCES = 1<<0,
34+
ALLOW_ANSI_CURSOR_MOVEMENTS = 1<<1,
35+
ALLOW_ANSI_ERASE = 1<<2,
36+
ALLOW_ALL_CONTROL_CHARACTERS = 1<<3,
37+
#ifdef WITH_BREAKING_CHANGES
38+
ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES,
39+
#else
40+
ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ALL_CONTROL_CHARACTERS,
41+
#endif
42+
} allow_control_characters = ALLOW_CONTROL_SEQUENCES_UNSET;
43+
44+
static inline int skip_prefix_in_csv(const char *value, const char *prefix,
45+
const char **out)
46+
{
47+
if (!skip_prefix(value, prefix, &value) ||
48+
(*value && *value != ','))
49+
return 0;
50+
*out = value + !!*value;
51+
return 1;
52+
}
53+
54+
int sideband_allow_control_characters_config(const char *var, const char *value)
55+
{
56+
switch (git_parse_maybe_bool(value)) {
57+
case 0:
58+
allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
59+
return 0;
60+
case 1:
61+
allow_control_characters = ALLOW_ALL_CONTROL_CHARACTERS;
62+
return 0;
63+
default:
64+
break;
65+
}
66+
67+
allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
68+
while (*value) {
69+
if (skip_prefix_in_csv(value, "default", &value))
70+
allow_control_characters |= ALLOW_DEFAULT_ANSI_SEQUENCES;
71+
else if (skip_prefix_in_csv(value, "color", &value))
72+
allow_control_characters |= ALLOW_ANSI_COLOR_SEQUENCES;
73+
else if (skip_prefix_in_csv(value, "cursor", &value))
74+
allow_control_characters |= ALLOW_ANSI_CURSOR_MOVEMENTS;
75+
else if (skip_prefix_in_csv(value, "erase", &value))
76+
allow_control_characters |= ALLOW_ANSI_ERASE;
77+
else if (skip_prefix_in_csv(value, "true", &value))
78+
allow_control_characters = ALLOW_ALL_CONTROL_CHARACTERS;
79+
else if (skip_prefix_in_csv(value, "false", &value))
80+
allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS;
81+
else
82+
warning(_("unrecognized value for '%s': '%s'"), var, value);
83+
}
84+
return 0;
85+
}
86+
87+
static int sideband_config_callback(const char *var, const char *value,
88+
const struct config_context *ctx UNUSED,
89+
void *data UNUSED)
90+
{
91+
if (!strcmp(var, "sideband.allowcontrolcharacters"))
92+
return sideband_allow_control_characters_config(var, value);
93+
94+
return 0;
95+
}
96+
97+
void sideband_apply_url_config(const char *url)
98+
{
99+
struct urlmatch_config config = URLMATCH_CONFIG_INIT;
100+
char *normalized_url;
101+
102+
if (!url)
103+
BUG("must not call sideband_apply_url_config(NULL)");
104+
105+
config.section = "sideband";
106+
config.collect_fn = sideband_config_callback;
107+
108+
normalized_url = url_normalize(url, &config.url);
109+
repo_config(the_repository, urlmatch_config_entry, &config);
110+
free(normalized_url);
111+
string_list_clear(&config.vars, 1);
112+
urlmatch_config_release(&config);
113+
}
114+
29115
/* Returns a color setting (GIT_COLOR_NEVER, etc). */
30116
static enum git_colorbool use_sideband_colors(void)
31117
{
@@ -39,6 +125,14 @@ static enum git_colorbool use_sideband_colors(void)
39125
if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN)
40126
return use_sideband_colors_cached;
41127

128+
if (allow_control_characters == ALLOW_CONTROL_SEQUENCES_UNSET) {
129+
if (!repo_config_get_value(the_repository, "sideband.allowcontrolcharacters", &value))
130+
sideband_allow_control_characters_config("sideband.allowcontrolcharacters", value);
131+
132+
if (allow_control_characters == ALLOW_CONTROL_SEQUENCES_UNSET)
133+
allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES;
134+
}
135+
42136
if (!repo_config_get_string_tmp(the_repository, key, &value))
43137
use_sideband_colors_cached = git_config_colorbool(key, value);
44138
else if (!repo_config_get_string_tmp(the_repository, "color.ui", &value))
@@ -66,6 +160,93 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref
66160
list_config_item(list, prefix, keywords[i].keyword);
67161
}
68162

163+
static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)
164+
{
165+
int i;
166+
167+
/*
168+
* Valid ANSI color sequences are of the form
169+
*
170+
* ESC [ [<n> [; <n>]*] m
171+
*
172+
* These are part of the Select Graphic Rendition sequences which
173+
* contain more than just color sequences, for more details see
174+
* https://en.wikipedia.org/wiki/ANSI_escape_code#SGR.
175+
*
176+
* The cursor movement sequences are:
177+
*
178+
* ESC [ n A - Cursor up n lines (CUU)
179+
* ESC [ n B - Cursor down n lines (CUD)
180+
* ESC [ n C - Cursor forward n columns (CUF)
181+
* ESC [ n D - Cursor back n columns (CUB)
182+
* ESC [ n E - Cursor next line, beginning (CNL)
183+
* ESC [ n F - Cursor previous line, beginning (CPL)
184+
* ESC [ n G - Cursor to column n (CHA)
185+
* ESC [ n ; m H - Cursor position (row n, col m) (CUP)
186+
* ESC [ n ; m f - Same as H (HVP)
187+
*
188+
* The sequences to erase characters are:
189+
*
190+
*
191+
* ESC [ 0 J - Clear from cursor to end of screen (ED)
192+
* ESC [ 1 J - Clear from cursor to beginning of screen (ED)
193+
* ESC [ 2 J - Clear entire screen (ED)
194+
* ESC [ 3 J - Clear entire screen + scrollback (ED) - xterm extension
195+
* ESC [ 0 K - Clear from cursor to end of line (EL)
196+
* ESC [ 1 K - Clear from cursor to beginning of line (EL)
197+
* ESC [ 2 K - Clear entire line (EL)
198+
* ESC [ n M - Delete n lines (DL)
199+
* ESC [ n P - Delete n characters (DCH)
200+
* ESC [ n X - Erase n characters (ECH)
201+
*
202+
* For a comprehensive list of common ANSI Escape sequences, see
203+
* https://www.xfree86.org/current/ctlseqs.html
204+
*/
205+
206+
if (n < 3 || src[0] != '\x1b' || src[1] != '[')
207+
return 0;
208+
209+
for (i = 2; i < n; i++) {
210+
if (((allow_control_characters & ALLOW_ANSI_COLOR_SEQUENCES) &&
211+
src[i] == 'm') ||
212+
((allow_control_characters & ALLOW_ANSI_CURSOR_MOVEMENTS) &&
213+
strchr("ABCDEFGHf", src[i])) ||
214+
((allow_control_characters & ALLOW_ANSI_ERASE) &&
215+
strchr("JKMPX", src[i]))) {
216+
strbuf_add(dest, src, i + 1);
217+
return i;
218+
}
219+
if (!isdigit(src[i]) && src[i] != ';')
220+
break;
221+
}
222+
223+
return 0;
224+
}
225+
226+
static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n)
227+
{
228+
int i;
229+
230+
if ((allow_control_characters & ALLOW_ALL_CONTROL_CHARACTERS)) {
231+
strbuf_add(dest, src, n);
232+
return;
233+
}
234+
235+
strbuf_grow(dest, n);
236+
for (; n && *src; src++, n--) {
237+
if (!iscntrl(*src) || *src == '\t' || *src == '\n') {
238+
strbuf_addch(dest, *src);
239+
} else if (allow_control_characters != ALLOW_NO_CONTROL_CHARACTERS &&
240+
(i = handle_ansi_sequence(dest, src, n))) {
241+
src += i;
242+
n -= i;
243+
} else {
244+
strbuf_addch(dest, '^');
245+
strbuf_addch(dest, *src == 0x7f ? '?' : 0x40 + *src);
246+
}
247+
}
248+
}
249+
69250
/*
70251
* Optionally highlight one keyword in remote output if it appears at the start
71252
* of the line. This should be called for a single line only, which is
@@ -81,7 +262,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
81262
int i;
82263

83264
if (!want_color_stderr(use_sideband_colors())) {
84-
strbuf_add(dest, src, n);
265+
strbuf_add_sanitized(dest, src, n);
85266
return;
86267
}
87268

@@ -114,7 +295,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
114295
}
115296
}
116297

117-
strbuf_add(dest, src, n);
298+
strbuf_add_sanitized(dest, src, n);
118299
}
119300

120301

‎sideband.h‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,18 @@ int demultiplex_sideband(const char *me, int status,
3030

3131
void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);
3232

33+
/*
34+
* Apply sideband configuration for the given URL. This should be called
35+
* when a transport is created to allow URL-specific configuration of
36+
* sideband behavior (e.g., sideband.<url>.allowControlCharacters).
37+
*/
38+
void sideband_apply_url_config(const char *url);
39+
40+
/*
41+
* Parse and set the sideband allow control characters configuration.
42+
* The var parameter should be the key name (without section prefix).
43+
* Returns 0 if the variable was recognized and handled, non-zero otherwise.
44+
*/
45+
int sideband_allow_control_characters_config(const char *var, const char *value);
46+
3347
#endif

‎t/t5409-colorize-remote-messages.sh‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,104 @@ test_expect_success 'fallback to color.ui' '
9898
grep "<BOLD;RED>error<RESET>: error" decoded
9999
'
100100

101+
if test_have_prereq WITH_BREAKING_CHANGES
102+
then
103+
TURN_ON_SANITIZING=already.turned=on
104+
else
105+
TURN_ON_SANITIZING=sideband.allowControlCharacters=color
106+
fi
107+
108+
test_expect_success 'disallow (color) control sequences in sideband' '
109+
write_script .git/color-me-surprised <<-\EOF &&
110+
printf "error: Have you \\033[31mread\\033[m this?\\a\\n" >&2
111+
exec "$@"
112+
EOF
113+
test_config_global uploadPack.packObjectsHook ./color-me-surprised &&
114+
test_commit need-at-least-one-commit &&
115+
116+
git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
117+
test_decode_color <stderr >decoded &&
118+
test_grep RED decoded &&
119+
test_grep "\\^G" stderr &&
120+
tr -dc "\\007" <stderr >actual &&
121+
test_must_be_empty actual &&
122+
123+
rm -rf throw-away &&
124+
git -c sideband.allowControlCharacters=false \
125+
clone --no-local . throw-away 2>stderr &&
126+
test_decode_color <stderr >decoded &&
127+
test_grep ! RED decoded &&
128+
test_grep "\\^G" stderr &&
129+
130+
rm -rf throw-away &&
131+
git -c sideband.allowControlCharacters clone --no-local . throw-away 2>stderr &&
132+
test_decode_color <stderr >decoded &&
133+
test_grep RED decoded &&
134+
tr -dc "\\007" <stderr >actual &&
135+
test_file_not_empty actual
136+
'
137+
138+
test_decode_csi() {
139+
awk '{
140+
while (match($0, /\033/) != 0) {
141+
printf "%sCSI ", substr($0, 1, RSTART-1);
142+
$0 = substr($0, RSTART + RLENGTH, length($0) - RSTART - RLENGTH + 1);
143+
}
144+
print
145+
}'
146+
}
147+
148+
test_expect_success 'control sequences in sideband allowed by default (in Git v3.8)' '
149+
write_script .git/color-me-surprised <<-\EOF &&
150+
printf "error: \\033[31mcolor\\033[m\\033[Goverwrite\\033[Gerase\\033[K\\033?25l\\n" >&2
151+
exec "$@"
152+
EOF
153+
test_config_global uploadPack.packObjectsHook ./color-me-surprised &&
154+
test_commit need-at-least-one-commit-at-least &&
155+
156+
rm -rf throw-away &&
157+
git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
158+
test_decode_color <stderr >color-decoded &&
159+
test_decode_csi <color-decoded >decoded &&
160+
test_grep ! "CSI \\[K" decoded &&
161+
test_grep ! "CSI \\[G" decoded &&
162+
test_grep "\\^\\[?25l" decoded &&
163+
164+
rm -rf throw-away &&
165+
git -c sideband.allowControlCharacters=erase,cursor,color \
166+
clone --no-local . throw-away 2>stderr &&
167+
test_decode_color <stderr >color-decoded &&
168+
test_decode_csi <color-decoded >decoded &&
169+
test_grep "RED" decoded &&
170+
test_grep "CSI \\[K" decoded &&
171+
test_grep "CSI \\[G" decoded &&
172+
test_grep ! "\\^\\[\\[K" decoded &&
173+
test_grep ! "\\^\\[\\[G" decoded
174+
'
175+
176+
test_expect_success 'allow all control sequences for a specific URL' '
177+
write_script .git/eraser <<-\EOF &&
178+
printf "error: Ohai!\\r\\033[K" >&2
179+
exec "$@"
180+
EOF
181+
test_config_global uploadPack.packObjectsHook ./eraser &&
182+
test_commit one-more-please &&
183+
184+
rm -rf throw-away &&
185+
git -c $TURN_ON_SANITIZING clone --no-local . throw-away 2>stderr &&
186+
test_decode_color <stderr >color-decoded &&
187+
test_decode_csi <color-decoded >decoded &&
188+
test_grep ! "CSI \\[K" decoded &&
189+
test_grep "\\^\\[\\[K" decoded &&
190+
191+
rm -rf throw-away &&
192+
git -c sideband.allowControlCharacters=false \
193+
-c "sideband.file://.allowControlCharacters=true" \
194+
clone --no-local "file://$PWD" throw-away 2>stderr &&
195+
test_decode_color <stderr >color-decoded &&
196+
test_decode_csi <color-decoded >decoded &&
197+
test_grep "CSI \\[K" decoded &&
198+
test_grep ! "\\^\\[\\[K" decoded
199+
'
200+
101201
test_done

0 commit comments

Comments
 (0)