Skip to content

Commit 13cd45b

Browse files
aiharosGopher Bot
authored andcommitted
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 a314a36 commit 13cd45b

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
@@ -983,6 +983,9 @@ func SerializeHTTPRequestRule(f models.HTTPRequestRule, opt *options.Configurati
983983
Comment: comment,
984984
}
985985
case "deny":
986+
if err := validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
987+
return nil, err
988+
}
986989
contentType := ""
987990
if f.ReturnContentType != nil {
988991
contentType = *f.ReturnContentType
@@ -1113,6 +1116,9 @@ func SerializeHTTPRequestRule(f models.HTTPRequestRule, opt *options.Configurati
11131116
Comment: comment,
11141117
}
11151118
case "return":
1119+
if err := validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
1120+
return nil, err
1121+
}
11161122
contentType := ""
11171123
if f.ReturnContentType != nil {
11181124
contentType = *f.ReturnContentType

configuration/http_response_rule.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,9 @@ func SerializeHTTPResponseRule(f models.HTTPResponseRule, opt *options.Configura
729729
Comment: comment,
730730
}
731731
case "deny":
732+
if err := validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
733+
return nil, err
734+
}
732735
contentType := ""
733736
if f.ReturnContentType != nil {
734737
contentType = *f.ReturnContentType
@@ -796,6 +799,9 @@ func SerializeHTTPResponseRule(f models.HTTPResponseRule, opt *options.Configura
796799
Comment: comment,
797800
}
798801
case "return":
802+
if err := validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
803+
return nil, err
804+
}
799805
contentType := ""
800806
if f.ReturnContentType != nil {
801807
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
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)