Skip to content

Commit e542128

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 6c50b3f commit e542128

5 files changed

Lines changed: 215 additions & 3 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/certs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ func (s *SingleRuntime) SetCertEntry(storageName string, payload string) error {
195195
if storageName == "" || payload == "" {
196196
return fmt.Errorf("%s %w", "Argument storageName or payload empty", native_errors.ErrGeneral)
197197
}
198-
cmd := fmt.Sprintf("set ssl cert %s <<\n%s\n", storageName, payload)
198+
cmd := fmt.Sprintf("set ssl cert %s <<\n%s\n", storageName, terminateHeredocPayload(payload))
199199
response, err := s.ExecuteWithResponse(cmd)
200200
if err != nil {
201201
return fmt.Errorf("%s %w", err.Error(), native_errors.ErrGeneral)

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: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
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/v6/runtime/options"
26+
)
27+
28+
// captureSocket is a minimal Unix-socket server that records every byte
29+
// a client writes after immediately sending a fixed reply. It exists so
30+
// these tests can assert the exact bytes client-native sends to HAProxy's
31+
// runtime API for heredoc commands.
32+
type captureSocket struct {
33+
listener net.Listener
34+
reply string
35+
mu sync.Mutex
36+
received []byte
37+
addr string
38+
}
39+
40+
func newCaptureSocket(t *testing.T, reply string) *captureSocket {
41+
t.Helper()
42+
f, err := os.CreateTemp("", "client-native-cap-*")
43+
if err != nil {
44+
t.Fatalf("CreateTemp: %v", err)
45+
}
46+
addr := f.Name()
47+
_ = f.Close()
48+
_ = os.Remove(addr)
49+
l, err := net.Listen("unix", addr)
50+
if err != nil {
51+
t.Fatalf("Listen: %v", err)
52+
}
53+
s := &captureSocket{listener: l, reply: reply, addr: addr}
54+
go s.serve()
55+
t.Cleanup(func() { _ = l.Close(); _ = os.Remove(addr) })
56+
return s
57+
}
58+
59+
func (s *captureSocket) serve() {
60+
for {
61+
conn, err := s.listener.Accept()
62+
if err != nil {
63+
return
64+
}
65+
go func(c net.Conn) {
66+
defer c.Close()
67+
// Send reply right away so the client's read loop sees data
68+
// and unblocks; afterwards the client gets EOF when we close
69+
// at the deferred Close, which lets readFromSocket return.
70+
_, _ = c.Write([]byte(s.reply))
71+
// Drain everything the client sends for up to 500ms.
72+
_ = c.SetReadDeadline(time.Now().Add(500 * time.Millisecond))
73+
buf := make([]byte, 4096)
74+
var got []byte
75+
for {
76+
n, err := c.Read(buf)
77+
if n > 0 {
78+
got = append(got, buf[:n]...)
79+
}
80+
if err != nil {
81+
break
82+
}
83+
}
84+
s.mu.Lock()
85+
s.received = append(s.received, got...)
86+
s.mu.Unlock()
87+
}(conn)
88+
}
89+
}
90+
91+
func (s *captureSocket) Received() string {
92+
s.mu.Lock()
93+
defer s.mu.Unlock()
94+
return string(s.received)
95+
}
96+
97+
// assertHeredocTerminated checks that the bytes sent by the runtime client
98+
// contain a `<<\n` heredoc start AND that the payload after it is followed
99+
// by a blank line (\n\n) BEFORE any `;` command separator (which would
100+
// otherwise be interpreted as part of the heredoc body and stall HAProxy).
101+
func assertHeredocTerminated(t *testing.T, sent string) {
102+
t.Helper()
103+
idx := strings.Index(sent, "<<\n")
104+
if idx < 0 {
105+
t.Fatalf("no `<<\\n` heredoc start in sent bytes:\n%s", quoted(sent))
106+
}
107+
tail := sent[idx+len("<<\n"):]
108+
blankLine := strings.Index(tail, "\n\n")
109+
if blankLine < 0 {
110+
t.Fatalf("heredoc payload is never followed by a blank line; HAProxy will hang.\n sent: %s", quoted(sent))
111+
}
112+
if semi := strings.Index(tail, ";"); semi >= 0 && semi < blankLine {
113+
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",
114+
semi, blankLine, quoted(sent))
115+
}
116+
}
117+
118+
func quoted(s string) string { return `"` + strings.ReplaceAll(s, "\n", `\n`) + `"` }
119+
120+
// TestHeredocPayloadAlwaysTerminated drives each runtime command that
121+
// builds a `<<` heredoc with a payload that explicitly does NOT end in
122+
// "\n", in both master-worker and single-process framing modes, and
123+
// asserts the bytes actually written to the socket are properly
124+
// terminated. Catches the class of bug where a missing payload `\n`
125+
// combined with the master-worker `;quit\n` framing leaves the heredoc
126+
// without a blank-line terminator and HAProxy stalls until the socket
127+
// deadline fires.
128+
func TestHeredocPayloadAlwaysTerminated(t *testing.T) {
129+
const noTrailingNewline = "-----BEGIN CERTIFICATE-----\nMIIB...\n-----END CERTIFICATE-----"
130+
131+
commands := []struct {
132+
name string
133+
call func(s *SingleRuntime) error
134+
}{
135+
{
136+
name: "AddMapPayload",
137+
call: func(s *SingleRuntime) error { return s.AddMapPayload("/etc/map", "k1 v1\nk2 v2") },
138+
},
139+
{
140+
name: "AddMapPayloadVersioned",
141+
call: func(s *SingleRuntime) error { return s.AddMapPayloadVersioned("1", "/etc/map", "k1 v1\nk2 v2") },
142+
},
143+
}
144+
145+
for _, mode := range []struct {
146+
name string
147+
masterWorkerMode bool
148+
}{
149+
{"master-worker", true},
150+
{"single-process", false},
151+
} {
152+
for _, cmd := range commands {
153+
t.Run(mode.name+"/"+cmd.name, func(t *testing.T) {
154+
mock := newCaptureSocket(t, " Transaction created for certificate /etc/cert.pem!\n\n")
155+
156+
s := &SingleRuntime{}
157+
if err := s.Init(mock.addr, mode.masterWorkerMode, options.RuntimeOptions{DoNotCheckRuntimeOnInit: true}); err != nil {
158+
t.Fatalf("Init: %v", err)
159+
}
160+
// Return value of the call is not checked: the mock doesn't
161+
// emit a real success response for every command shape, so
162+
// some calls legitimately error. What we care about is the
163+
// bytes that hit the wire before that.
164+
_ = cmd.call(s)
165+
166+
// Give the serve goroutine a moment to drain.
167+
time.Sleep(50 * time.Millisecond)
168+
169+
assertHeredocTerminated(t, mock.Received())
170+
})
171+
}
172+
}
173+
}

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) (string, 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)