Skip to content

Commit a57f034

Browse files
aiharosmjuraga
aiharos
authored and
mjuraga
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. (cherry picked from commit b086e61)
1 parent 494d014 commit a57f034

5 files changed

Lines changed: 152 additions & 0 deletions

File tree

‎configuration/http_error_rule.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,9 @@ func SerializeHTTPErrorRule(f models.HTTPErrorRule) (rule types.Action, err erro
265265
if f.Type != "status" {
266266
return nil, NewConfError(ErrValidationError, fmt.Sprintf("unsupported action %s in http_error", f.Type))
267267
}
268+
if err := validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
269+
return nil, err
270+
}
268271

269272
contentType := ""
270273
if f.ReturnContentType != nil {

‎configuration/http_request_rule.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,9 @@ func SerializeHTTPRequestRule(f models.HTTPRequestRule) (rule types.Action, err
819819
CondTest: f.CondTest,
820820
}
821821
case "deny":
822+
if err = validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
823+
return nil, err
824+
}
822825
contentType := ""
823826
if f.ReturnContentType != nil {
824827
contentType = *f.ReturnContentType
@@ -929,6 +932,9 @@ func SerializeHTTPRequestRule(f models.HTTPRequestRule) (rule types.Action, err
929932
CondTest: f.CondTest,
930933
}
931934
case "return":
935+
if err = validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
936+
return nil, err
937+
}
932938
contentType := ""
933939
if f.ReturnContentType != nil {
934940
contentType = *f.ReturnContentType

‎configuration/http_response_rule.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,9 @@ func SerializeHTTPResponseRule(f models.HTTPResponseRule) (rule types.Action, er
612612
CondTest: f.CondTest,
613613
}
614614
case "deny":
615+
if err = validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
616+
return nil, err
617+
}
615618
contentType := ""
616619
if f.ReturnContentType != nil {
617620
contentType = *f.ReturnContentType
@@ -667,6 +670,9 @@ func SerializeHTTPResponseRule(f models.HTTPResponseRule) (rule types.Action, er
667670
CondTest: f.CondTest,
668671
}
669672
case "return":
673+
if err = validateReturnContent(f.ReturnContentFormat, f.ReturnContent); err != nil {
674+
return nil, err
675+
}
670676
contentType := ""
671677
if f.ReturnContentType != nil {
672678
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/v5/config-parser/common"
47
"github.com/haproxytech/client-native/v5/config-parser/parsers/http/actions"
58

69
"github.com/haproxytech/client-native/v5/misc"
@@ -22,6 +25,30 @@ func actionHdr2ModelHdr(hdrs []*actions.Hdr) []*models.ReturnHeader {
2225
return headers
2326
}
2427

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

‎test/http_request_rule_return_content_test.go‎

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

0 commit comments

Comments
 (0)