Skip to content

Commit 843d212

Browse files
aiharosmjuraga
authored andcommitted
BUG/MEDIUM: runtime: ensure heredoc payloads have a blank-line terminator
When connected to HAProxy in master-worker mode, runtime_single_client.go appends ";quit\n" after the inner command. For "<command> <<\n%s\n" heredocs (set ssl cert, add ssl ca-file, set ssl crl-file, etc.) the resulting bytes on the wire are: <command> <<\n<payload>\n;quit\n HAProxy's `<<` heredoc terminates only on a blank line, so when the caller-supplied payload does not end with "\n" there is no blank line between payload and ";quit". HAProxy stalls reading the socket until the deadline fires, callers fall back to reload, and any layer with a tighter HTTP timeout above sees `context deadline exceeded` on the request. Normalize each heredoc payload via a new terminateHeredocPayload helper so the inner command always ends with "\n\n". Apply at every site that builds a `<<` heredoc: set ssl cert, set/add ssl ca-file, set ssl crl-file, set ssl ocsp-response, add map (versioned and not), and add ssl crt-list extended-form. Also fix the missing trailing "\n" in the SetCAFile format string so the new payload "\n" plus the format "\n" produce the blank line. Add TestHeredocPayloadAlwaysTerminated, which captures the bytes written to a Unix socket for each affected command in both master-worker and single-process modes and asserts a blank-line terminator appears before any ";" character. Reverting any single fix in this commit makes the corresponding subtest fail. Reproducible on HAProxy 3.2 in master-worker mode by speaking the master socket directly with the byte sequence this code previously produced.
1 parent 47fa79e commit 843d212

7 files changed

Lines changed: 246 additions & 5 deletions

File tree

.aspell.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ allowed:
8989
- gptstr
9090
- hapee
9191
- haproxy
92+
- heredoc
93+
- heredocs
9294
- healthcheck
9395
- healthz
9496
- hostname
@@ -173,6 +175,7 @@ allowed:
173175
- subnet
174176
- subresource
175177
- subresources
178+
- subtest
176179
- sudo
177180
- symlinks
178181
- syslog

runtime/ca_files.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func (s *SingleRuntime) SetCAFile(caFile, payload string) error {
112112
if payload == "" {
113113
return fmt.Errorf("%s %w", "Argument payload empty", native_errors.ErrGeneral)
114114
}
115-
cmd := fmt.Sprintf("set ssl ca-file %s <<\n%s", caFile, payload)
115+
cmd := fmt.Sprintf("set ssl ca-file %s <<\n%s\n", caFile, terminateHeredocPayload(payload))
116116
response, err := s.ExecuteWithResponse(cmd)
117117
if err != nil {
118118
return fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)
@@ -173,7 +173,7 @@ func (s *SingleRuntime) DeleteCAFile(caFile string) error {
173173

174174
// AddCAFileEntry adds an entry into the CA file
175175
func (s *SingleRuntime) AddCAFileEntry(caFile, payload string) error {
176-
cmd := fmt.Sprintf("add ssl ca-file %s <<\n%s\n", caFile, payload)
176+
cmd := fmt.Sprintf("add ssl ca-file %s <<\n%s\n", caFile, terminateHeredocPayload(payload))
177177
response, err := s.ExecuteWithResponse(cmd)
178178
if err != nil {
179179
return fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)

runtime/certs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ func (s *SingleRuntime) SetCertificate(storageName string, payload string) error
209209
if payload == "" {
210210
return fmt.Errorf("%s %w", "Argument payload empty", native_errors.ErrGeneral)
211211
}
212-
cmd := fmt.Sprintf("set ssl cert %s <<\n%s\n", storageName, payload)
212+
cmd := fmt.Sprintf("set ssl cert %s <<\n%s\n", storageName, terminateHeredocPayload(payload))
213213
response, err := s.ExecuteWithResponse(cmd)
214214
if err != nil {
215215
return fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)

runtime/crt-lists.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,13 @@ func (s *SingleRuntime) AddCrtListEntry(crtList string, entry models.SslCrtListE
186186
sb.WriteString(strings.Join(entry.SNIFilter, " "))
187187
}
188188
sb.WriteByte('\n')
189+
if extended {
190+
// HAProxy's `<<` heredoc terminates on a blank line; once the
191+
// outer framing in runtime_single_client.go appends `;quit\n`,
192+
// the lone `\n` above is consumed as the entry-line terminator
193+
// and there is no blank line left, so the heredoc never ends.
194+
sb.WriteByte('\n')
195+
}
189196

190197
response, err := s.ExecuteWithResponse(sb.String())
191198
if err != nil {

runtime/heredoc.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright 2025 HAProxy Technologies
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package runtime
16+
17+
import "strings"
18+
19+
// terminateHeredocPayload returns payload guaranteed to end with "\n".
20+
//
21+
// HAProxy's `<<` heredoc terminates only on a blank line. The conventional
22+
// command shape used in this package is "<command> <<\n<payload>\n", so for
23+
// the heredoc to terminate, payload itself must end with "\n" (producing
24+
// "\n\n" — a blank line — at the end of the command).
25+
//
26+
// Without that, the outer framing in runtime_single_client.go that appends
27+
// ";quit\n" (master-socket / worker > 0 path) places ";quit" immediately
28+
// after the payload's lone trailing newline — turning what should be the
29+
// blank-line terminator into a non-empty command line — and HAProxy then
30+
// blocks reading the socket until the deadline fires.
31+
func terminateHeredocPayload(payload string) string {
32+
if strings.HasSuffix(payload, "\n") {
33+
return payload
34+
}
35+
return payload + "\n"
36+
}

runtime/heredoc_test.go

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
// Copyright 2025 HAProxy Technologies
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package runtime
16+
17+
import (
18+
"net"
19+
"os"
20+
"strings"
21+
"sync"
22+
"testing"
23+
"time"
24+
25+
"github.com/haproxytech/client-native/v5/models"
26+
"github.com/haproxytech/client-native/v5/runtime/options"
27+
)
28+
29+
// captureSocket is a minimal Unix-socket server that records every byte
30+
// a client writes after immediately sending a fixed reply. It exists so
31+
// these tests can assert the exact bytes client-native sends to HAProxy's
32+
// runtime API for heredoc commands.
33+
type captureSocket struct {
34+
listener net.Listener
35+
reply string
36+
mu sync.Mutex
37+
received []byte
38+
addr string
39+
}
40+
41+
func newCaptureSocket(t *testing.T, reply string) *captureSocket {
42+
t.Helper()
43+
f, err := os.CreateTemp("", "client-native-cap-*") //nolint:usetesting
44+
if err != nil {
45+
t.Fatalf("CreateTemp: %v", err)
46+
}
47+
addr := f.Name()
48+
_ = f.Close()
49+
_ = os.Remove(addr)
50+
l, err := net.Listen("unix", addr)
51+
if err != nil {
52+
t.Fatalf("Listen: %v", err)
53+
}
54+
s := &captureSocket{listener: l, reply: reply, addr: addr}
55+
go s.serve()
56+
t.Cleanup(func() { _ = l.Close(); _ = os.Remove(addr) })
57+
return s
58+
}
59+
60+
func (s *captureSocket) serve() {
61+
for {
62+
conn, err := s.listener.Accept()
63+
if err != nil {
64+
return
65+
}
66+
go func(c net.Conn) {
67+
defer c.Close()
68+
// Send reply right away so the client's read loop sees data
69+
// and unblocks; afterwards the client gets EOF when we close
70+
// at the deferred Close, which lets readFromSocket return.
71+
_, _ = c.Write([]byte(s.reply))
72+
// Drain everything the client sends for up to 500ms.
73+
_ = c.SetReadDeadline(time.Now().Add(500 * time.Millisecond))
74+
buf := make([]byte, 4096)
75+
var got []byte
76+
for {
77+
n, err := c.Read(buf)
78+
if n > 0 {
79+
got = append(got, buf[:n]...)
80+
}
81+
if err != nil {
82+
break
83+
}
84+
}
85+
s.mu.Lock()
86+
s.received = append(s.received, got...)
87+
s.mu.Unlock()
88+
}(conn)
89+
}
90+
}
91+
92+
func (s *captureSocket) Received() string {
93+
s.mu.Lock()
94+
defer s.mu.Unlock()
95+
return string(s.received)
96+
}
97+
98+
// assertHeredocTerminated checks that the bytes sent by the runtime client
99+
// contain a `<<\n` heredoc start AND that the payload after it is followed
100+
// by a blank line (\n\n) BEFORE any `;` command separator (which would
101+
// otherwise be interpreted as part of the heredoc body and stall HAProxy).
102+
func assertHeredocTerminated(t *testing.T, sent string) {
103+
t.Helper()
104+
idx := strings.Index(sent, "<<\n")
105+
if idx < 0 {
106+
t.Fatalf("no `<<\\n` heredoc start in sent bytes:\n%s", quoted(sent))
107+
}
108+
tail := sent[idx+len("<<\n"):]
109+
blankLine := strings.Index(tail, "\n\n")
110+
if blankLine < 0 {
111+
t.Fatalf("heredoc payload is never followed by a blank line; HAProxy will hang.\n sent: %s", quoted(sent))
112+
}
113+
if semi := strings.Index(tail, ";"); semi >= 0 && semi < blankLine {
114+
t.Fatalf("`;` appears before the blank-line terminator at offset %d (blank line at %d).\n Once the framing in runtime_single_client.go appends `;quit\\n`, HAProxy will interpret `;quit` as part of the heredoc body and stall.\n sent: %s",
115+
semi, blankLine, quoted(sent))
116+
}
117+
}
118+
119+
func quoted(s string) string { return `"` + strings.ReplaceAll(s, "\n", `\n`) + `"` }
120+
121+
// TestHeredocPayloadAlwaysTerminated drives each runtime command that
122+
// builds a `<<` heredoc with a payload that explicitly does NOT end in
123+
// "\n", in both master-worker and single-process framing modes, and
124+
// asserts the bytes actually written to the socket are properly
125+
// terminated. Catches the class of bug where a missing payload `\n`
126+
// combined with the master-worker `;quit\n` framing leaves the heredoc
127+
// without a blank-line terminator and HAProxy stalls until the socket
128+
// deadline fires.
129+
func TestHeredocPayloadAlwaysTerminated(t *testing.T) {
130+
const noTrailingNewline = "-----BEGIN CERTIFICATE-----\nMIIB...\n-----END CERTIFICATE-----"
131+
132+
commands := []struct {
133+
name string
134+
call func(s *SingleRuntime) error
135+
}{
136+
{
137+
name: "SetCertificate",
138+
call: func(s *SingleRuntime) error { return s.SetCertificate("/etc/cert.pem", noTrailingNewline) },
139+
},
140+
{
141+
name: "SetCAFile",
142+
call: func(s *SingleRuntime) error { return s.SetCAFile("/etc/ca.pem", noTrailingNewline) },
143+
},
144+
{
145+
name: "AddCAFileEntry",
146+
call: func(s *SingleRuntime) error { return s.AddCAFileEntry("/etc/ca.pem", noTrailingNewline) },
147+
},
148+
{
149+
name: "AddMapPayload",
150+
call: func(s *SingleRuntime) error { return s.AddMapPayload("/etc/map", "k1 v1\nk2 v2") },
151+
},
152+
{
153+
name: "AddMapPayloadVersioned",
154+
call: func(s *SingleRuntime) error { return s.AddMapPayloadVersioned("1", "/etc/map", "k1 v1\nk2 v2") },
155+
},
156+
{
157+
name: "AddCrtListEntry-extended",
158+
call: func(s *SingleRuntime) error {
159+
return s.AddCrtListEntry("/etc/crt-list", models.SslCrtListEntry{
160+
File: "/etc/cert.pem",
161+
SSLBindConfig: "verify required",
162+
})
163+
},
164+
},
165+
}
166+
167+
for _, mode := range []struct {
168+
name string
169+
masterWorkerMode bool
170+
}{
171+
{"master-worker", true},
172+
{"single-process", false},
173+
} {
174+
for _, cmd := range commands {
175+
t.Run(mode.name+"/"+cmd.name, func(t *testing.T) {
176+
mock := newCaptureSocket(t, " Transaction created for certificate /etc/cert.pem!\n\n")
177+
178+
s := &SingleRuntime{}
179+
if err := s.Init(mock.addr, 1, 1, options.RuntimeOptions{DoNotCheckRuntimeOnInit: true}); err != nil {
180+
t.Fatalf("Init: %v", err)
181+
}
182+
// Return value of the call is not checked: the mock doesn't
183+
// emit a real success response for every command shape, so
184+
// some calls legitimately error. What we care about is the
185+
// bytes that hit the wire before that.
186+
_ = cmd.call(s)
187+
188+
// Give the serve goroutine a moment to drain.
189+
time.Sleep(50 * time.Millisecond)
190+
191+
assertHeredocTerminated(t, mock.Received())
192+
})
193+
}
194+
}
195+
}

runtime/maps.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func (s *SingleRuntime) AddMapEntryVersioned(version, name, key, value string) e
230230
func (s *SingleRuntime) AddMapPayload(name, payload string) error {
231231
prefix := "<<\n"
232232
if len(payload) < len(prefix) || payload[0:len(prefix)] != prefix {
233-
payload = "<<\n" + payload + "\n"
233+
payload = "<<\n" + terminateHeredocPayload(payload) + "\n"
234234
}
235235
cmd := fmt.Sprintf("add map %s %s", name, payload)
236236
if err := s.Execute(cmd); err != nil {
@@ -259,7 +259,7 @@ func (s *SingleRuntime) PrepareMap(name string) (version string, err error) {
259259
func (s *SingleRuntime) AddMapPayloadVersioned(version, name, payload string) error {
260260
prefix := "<<\n"
261261
if len(payload) < len(prefix) || payload[0:len(prefix)] != prefix {
262-
payload = "<<\n" + payload + "\n"
262+
payload = "<<\n" + terminateHeredocPayload(payload) + "\n"
263263
}
264264
cmd := fmt.Sprintf("add map @%s %s %s", version, name, payload)
265265
if err := s.Execute(cmd); err != nil {

0 commit comments

Comments
 (0)