Skip to content

Commit 34591b8

Browse files
aiharosGopher Bot
aiharos
authored and
Gopher Bot
committed
BUG/MEDIUM: configuration: reject unquoted multi-word return content
An http-request/http-response deny or return rule (and an http-error status rule) whose return content contains spaces but is not quoted, e.g. content "Missing Auth", was serialized verbatim into an invalid config line: http-request deny content-type text/html string Missing Auth The structured API reported success because the in-memory rule reads back correctly within the same request. On the next config reload the line fails to parse (the orphan token trips the action parser), and the reader silently skips lines that fail to parse, so the rule was dropped without any error being surfaced to the caller. Validate the return content when serializing the rule and reject a value that does not tokenize back to a single token: multi-word values must be quoted by the caller, matching how the raw configuration endpoint already rejects them. The check mirrors the serializer's emit condition (content is only written when both content and format are non-empty and the format is not default-errorfiles), so content that is never written is left untouched.
1 parent 98a0606 commit 34591b8

5 files changed

Lines changed: 155 additions & 0 deletions

File tree

‎configuration/http_error_rule.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,9 @@ func SerializeHTTPErrorRule(f models.HTTPErrorRule) (types.Action, error) { //no
317317
if f.Type != "status" {
318318
return nil, NewConfError(ErrValidationError, fmt.Sprintf("unsupported action %s in http_error", f.Type))
319319
}
320+
if err := validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
321+
return nil, err
322+
}
320323

321324
contentType := ""
322325
if f.ReturnContentType != nil {

‎configuration/http_request_rule.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,9 @@ func SerializeHTTPRequestRule(f models.HTTPRequestRule, opt *options.Configurati
10351035
Comment: comment,
10361036
}
10371037
case "deny":
1038+
if err := validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
1039+
return nil, err
1040+
}
10381041
contentType := ""
10391042
if f.ReturnContentType != nil {
10401043
contentType = *f.ReturnContentType
@@ -1165,6 +1168,9 @@ func SerializeHTTPRequestRule(f models.HTTPRequestRule, opt *options.Configurati
11651168
Comment: comment,
11661169
}
11671170
case "return":
1171+
if err := validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
1172+
return nil, err
1173+
}
11681174
contentType := ""
11691175
if f.ReturnContentType != nil {
11701176
contentType = *f.ReturnContentType

‎configuration/http_response_rule.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,9 @@ func SerializeHTTPResponseRule(f models.HTTPResponseRule, opt *options.Configura
781781
Comment: comment,
782782
}
783783
case "deny":
784+
if err := validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
785+
return nil, err
786+
}
784787
contentType := ""
785788
if f.ReturnContentType != nil {
786789
contentType = *f.ReturnContentType
@@ -848,6 +851,9 @@ func SerializeHTTPResponseRule(f models.HTTPResponseRule, opt *options.Configura
848851
Comment: comment,
849852
}
850853
case "return":
854+
if err := validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
855+
return nil, err
856+
}
851857
contentType := ""
852858
if f.ReturnContentType != nil {
853859
contentType = *f.ReturnContentType

‎configuration/misc.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package configuration
22

33
import (
4+
"fmt"
5+
6+
"github.com/haproxytech/client-native/v6/config-parser/common"
47
"github.com/haproxytech/client-native/v6/config-parser/parsers/http/actions"
58

69
"github.com/haproxytech/client-native/v6/models"
@@ -21,6 +24,30 @@ func actionHdr2ModelHdr(hdrs []*actions.Hdr) []*models.ReturnHeader {
2124
return headers
2225
}
2326

27+
// validateReturnContent ensures a return-content value can be faithfully
28+
// persisted. The value is emitted verbatim after the content-format keyword
29+
// (string/lf-string/file/lf-file/errorfile), and HAProxy treats it as a single
30+
// token: multi-word values must be quoted by the caller. When they are not, the
31+
// serialized line (e.g. `... string Missing Auth`) fails to parse and is
32+
// silently dropped on the next config reload, even though the transaction
33+
// reports success. Reject such values here instead so the caller gets an error.
34+
//
35+
// The check mirrors the serializer's emit condition: content is only written
36+
// when both the content and the format are non-empty and the format is not
37+
// default-errorfiles, so anything that is not actually emitted cannot break the
38+
// round-trip and is left alone.
39+
func validateReturnContent(format, content string) error {
40+
if content == "" || format == "" || format == "default-errorfiles" {
41+
return nil
42+
}
43+
tokens, comment := common.StringSplitWithCommentIgnoreEmpty(content)
44+
if comment != "" || len(tokens) != 1 || tokens[0] != content {
45+
return NewConfError(ErrValidationError,
46+
fmt.Sprintf("invalid return content %q: multi-word values must be quoted", content))
47+
}
48+
return nil
49+
}
50+
2451
func modelHdr2ActionHdr(hdrs []*models.ReturnHeader) []*actions.Hdr {
2552
if len(hdrs) == 0 {
2653
return nil

‎test/http_request_rule_return_content_test.go‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
// Copyright 2026 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+
16+
package test
17+
18+
import (
19+
"testing"
20+
21+
"github.com/haproxytech/client-native/v6/configuration"
22+
"github.com/haproxytech/client-native/v6/configuration/options"
23+
"github.com/haproxytech/client-native/v6/misc"
24+
"github.com/haproxytech/client-native/v6/models"
25+
"github.com/stretchr/testify/require"
26+
)
27+
28+
// TestSerializeReturnContentValidation guards against the silent-drop bug where
29+
// an unquoted multi-word return content serializes to an invalid HAProxy config
30+
// line (e.g. `... string Missing Auth`) that fails to parse and is silently
31+
// dropped on the next config reload, while the API reports success.
32+
//
33+
// It covers all serialize sites that emit return content: http-request and
34+
// http-response deny/return, and http-error status.
35+
func TestSerializeReturnContentValidation(t *testing.T) {
36+
opt := &options.ConfigurationOptions{}
37+
38+
cases := []struct {
39+
name string
40+
format string
41+
content string
42+
wantErr bool
43+
}{
44+
{"unquoted_multiword", "string", "Missing Auth", true},
45+
{"quoted_multiword", "string", `"Missing Auth"`, false},
46+
{"escaped_space", "string", `Missing\ Auth`, false},
47+
{"single_word", "string", "MissingAuth", false},
48+
{"empty_content", "string", "", false},
49+
// Content is never emitted when the format is empty or default-errorfiles,
50+
// so it cannot break the round-trip and must not be rejected.
51+
{"empty_format_multiword", "", "Missing Auth", false},
52+
{"default_errorfiles_multiword", "default-errorfiles", "Missing Auth", false},
53+
}
54+
55+
for _, tc := range cases {
56+
t.Run(tc.name, func(t *testing.T) {
57+
// http-request deny
58+
_, err := configuration.SerializeHTTPRequestRule(models.HTTPRequestRule{
59+
Type: "deny",
60+
ReturnContentType: misc.Ptr("text/html"),
61+
ReturnContentFormat: tc.format,
62+
ReturnContent: tc.content,
63+
}, opt)
64+
assertContentErr(t, "http-request deny", tc.wantErr, err)
65+
66+
// http-request return
67+
_, err = configuration.SerializeHTTPRequestRule(models.HTTPRequestRule{
68+
Type: "return",
69+
ReturnContentType: misc.Ptr("text/html"),
70+
ReturnContentFormat: tc.format,
71+
ReturnContent: tc.content,
72+
}, opt)
73+
assertContentErr(t, "http-request return", tc.wantErr, err)
74+
75+
// http-response deny
76+
_, err = configuration.SerializeHTTPResponseRule(models.HTTPResponseRule{
77+
Type: "deny",
78+
ReturnContentType: misc.Ptr("text/html"),
79+
ReturnContentFormat: tc.format,
80+
ReturnContent: tc.content,
81+
}, opt)
82+
assertContentErr(t, "http-response deny", tc.wantErr, err)
83+
84+
// http-response return
85+
_, err = configuration.SerializeHTTPResponseRule(models.HTTPResponseRule{
86+
Type: "return",
87+
ReturnContentType: misc.Ptr("text/html"),
88+
ReturnContentFormat: tc.format,
89+
ReturnContent: tc.content,
90+
}, opt)
91+
assertContentErr(t, "http-response return", tc.wantErr, err)
92+
93+
// http-error status (403 is a valid error status code)
94+
_, err = configuration.SerializeHTTPErrorRule(models.HTTPErrorRule{
95+
Type: "status",
96+
Status: 403,
97+
ReturnContentType: misc.Ptr("text/html"),
98+
ReturnContentFormat: tc.format,
99+
ReturnContent: tc.content,
100+
})
101+
assertContentErr(t, "http-error status", tc.wantErr, err)
102+
})
103+
}
104+
}
105+
106+
func assertContentErr(t *testing.T, site string, wantErr bool, err error) {
107+
t.Helper()
108+
if wantErr {
109+
require.Error(t, err, "%s: value should be rejected, not silently dropped", site)
110+
} else {
111+
require.NoError(t, err, "%s: value should be accepted", site)
112+
}
113+
}

0 commit comments

Comments
 (0)