BUG/MEDIUM: cli: Fix parsing of big payloads with dynamically allocat… · haproxy/haproxy@bca0aae · GitHub
Skip to content

Commit bca0aae

Browse files
capflamcapflam
committed
BUG/MEDIUM: cli: Fix parsing of big payloads with dynamically allocated buffer
Since the 3.4, a dedicated buffer is used for big payload on the CLI. When the command line exceeds the buffer size because of a big payload, a dedicated buffer is dynamically allocated to copy the payload. However, on realloc, input data were moved in the dynamic buffer without parsing them. This could prevent the end of the payload from being correctly detected. To fix the issue, instead of moving input data in the newly allocated buffer, we retry the parsing. But a payload line may be longer than tune.bufsize. In this case, only the beginning of the line is present in the input buffer, and the parsing always fails to find its end. The input buffer is never emptied and the flag reporting a full input buffer is never cleared, so the peer is never unblocked and the remaining part of the line is never received. The CLI keeps looping on the same partial line, burning the CPU, and the command is never executed. This was already the case before, when the input data were moved in the dynamic buffer without parsing them: the part of a line already received could be moved several times without progress. Thus, when the parsing fails because the input buffer contains an incomplete payload line, the part already received is moved into the payload buffer, waiting for more data, and the full-input-buffer flag is cleared. For this purpose, the beginning of the incomplete current payload line is saved in the CLI context, and updated when the payload buffer is reallocated. When a full line is received, the end of the payload is searched in the whole assembled line: the end of the payload must also be detected when the line marking it is itself received in several chunks. This patch should fix the issue #3524. It must be backported to 3.4.
1 parent 7b37fe8 commit bca0aae

3 files changed

Lines changed: 177 additions & 2 deletions

File tree

‎include/haproxy/applet-t.h‎

Lines changed: 1 addition & 0 deletions

‎reg-tests/cli/payload_parsing.vtc‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
varnishtest "Test CLI payloads with long lines and multiple input refills"
2+
3+
# Two aspects of the CLI payload parsing are tested here, with a small
4+
# tune.bufsize (1024 bytes) to force the payload buffer to be dynamically
5+
# allocated and the input to be received in several refills:
6+
#
7+
# 1. A payload line may be longer than tune.bufsize. The part of the line
8+
# already received must be moved into the payload buffer while waiting
9+
# for the remaining part, and the peer must be unblocked, otherwise the
10+
# CLI keeps looping on the same partial line forever, burning the CPU,
11+
# and the command is never executed.
12+
#
13+
# 2. A payload made of several lines shorter than tune.bufsize, but bigger
14+
# than the payload buffer, must keep being parsed line by line after the
15+
# payload buffer was enlarged, so that the end of the payload is still
16+
# detected and the command is executed (issue #3524).
17+
#
18+
# A third payload mixes lines shorter and longer than tune.bufsize, to be
19+
# sure the parsing resumes correctly after a line reassembled from
20+
# multiple input refills.
21+
#
22+
# A fourth payload uses a customized pattern to mark the end of the
23+
# payload, sent in several chunks: the end of the payload must be detected
24+
# in the line reassembled from the beginning of the pattern received as an
25+
# incomplete line and its end received later.
26+
27+
feature ignore_unknown_macro
28+
29+
shell {
30+
echo "k0 v0" > ${tmpdir}/map
31+
}
32+
33+
haproxy h1 -conf {
34+
global
35+
tune.bufsize 1024
36+
stats socket "${tmpdir}/h1/stats" level admin
37+
38+
defaults
39+
mode http
40+
timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
41+
timeout client "${HAPROXY_TEST_TIMEOUT-5s}"
42+
timeout server "${HAPROXY_TEST_TIMEOUT-5s}"
43+
44+
45+
frontend fe
46+
bind "fd@${fe}"
47+
http-request set-header X-V %[req.hdr(x-t),map("${tmpdir}/map")]
48+
} -start
49+
50+
# 1. A single payload line of 4003 bytes, longer than tune.bufsize, sent
51+
# in chunks. If the line is not properly reassembled, the CLI is stuck
52+
# and never closes the connection.
53+
client c1 -connect ${h1_cli_sock} {
54+
send "add map ${tmpdir}/map <<"
55+
send "\n"
56+
send "k1 "
57+
send_n 2000 "a"
58+
send_n 2000 "b"
59+
send "\n"
60+
send "\n"
61+
expect_close
62+
} -run
63+
64+
# 2. A payload made of 4 lines of 250 bytes, bigger than the initial
65+
# payload buffer: the end of the payload is received after the payload
66+
# buffer was enlarged and must still be detected.
67+
client c2 -connect ${h1_cli_sock} {
68+
send "add map ${tmpdir}/map <<"
69+
send "\n"
70+
send "k2 a"
71+
send_n 246 "a"
72+
send "\n"
73+
send "k3 b"
74+
send_n 246 "b"
75+
send "\n"
76+
send "k4 c"
77+
send_n 246 "c"
78+
send "\n"
79+
send "k5 d"
80+
send_n 246 "d"
81+
send "\n"
82+
send "\n"
83+
expect_close
84+
} -run
85+
86+
# 3. A payload mixing lines shorter and longer than tune.bufsize. The
87+
# parsing must resume correctly after a reassembled line.
88+
client c3 -connect ${h1_cli_sock} {
89+
send "add map ${tmpdir}/map <<"
90+
send "\n"
91+
send "k6 "
92+
send_n 1500 "e"
93+
send_n 1500 "f"
94+
send "\n"
95+
send "k7 g"
96+
send_n 246 "g"
97+
send "\n"
98+
send "k8 "
99+
send_n 1500 "h"
100+
send_n 1500 "i"
101+
send "\n"
102+
send "\n"
103+
expect_close
104+
} -run
105+
106+
# 4. A payload with a customized end pattern, split across two input
107+
# refills: the CLI first receives "12", an incomplete line, then "3\n"
108+
# completing the end pattern. The end of the payload must be detected.
109+
# If it is not, the command is never executed and the CLI is stuck.
110+
client c4 -connect ${h1_cli_sock} {
111+
send "add map ${tmpdir}/map <<123"
112+
send "\n"
113+
send "k9 v9"
114+
send "\n"
115+
send "12"
116+
delay 0.5
117+
send "3"
118+
send "\n"
119+
expect_close
120+
} -run
121+
122+
123+
haproxy h1 -cli {
124+
# The value of the long lines cannot be displayed, the output is
125+
# truncated by the trash chunk size. But the entry must be found,
126+
# which proves the payload line was properly reassembled, the end of
127+
# the payload detected and the command executed.
128+
send "get map ${tmpdir}/map k1"
129+
expect ~ "type=str, case=sensitive, found=yes,.*key=\"k1\"[^,]*"
130+
send "get map ${tmpdir}/map k2"
131+
expect ~ "type=str, case=sensitive, found=yes,.*key=\"k2\", value=\"aaa.*"
132+
send "get map ${tmpdir}/map k5"
133+
expect ~ "type=str, case=sensitive, found=yes,.*key=\"k5\", value=\"ddd.*"
134+
send "get map ${tmpdir}/map k6"
135+
expect ~ "type=str, case=sensitive, found=yes,.*key=\"k6\"[^,]*"
136+
send "get map ${tmpdir}/map k7"
137+
expect ~ "type=str, case=sensitive, found=yes,.*key=\"k7\", value=\"ggg.*"
138+
send "get map ${tmpdir}/map k8"
139+
expect ~ "type=str, case=sensitive, found=yes,.*key=\"k8\"[^,]*"
140+
send "get map ${tmpdir}/map k9"
141+
expect ~ "type=str, case=sensitive, found=yes,.*key=\"k9\", value=\"v9\", type=\"str\""
142+
}

‎src/cli.c‎

Lines changed: 34 additions & 2 deletions

0 commit comments

Comments
 (0)