Skip to content

Commit eec9940

Browse files
committed
validation
Signed-off-by: Piotr Janik <piotr.janik@sap.com>
1 parent c6eb851 commit eec9940

4 files changed

Lines changed: 86 additions & 12 deletions

File tree

api/oci/config/httpconfig.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ func (a *HTTPConfig) ApplyTo(_ cfgcpi.Context, target interface{}) error {
3131
if !ok {
3232
return cfgcpi.ErrNoContext(HTTPConfigType)
3333
}
34+
if err := a.HTTPSettings.Validate(); err != nil {
35+
return err
36+
}
3437
s, err := t.GetHTTPSettings()
3538
if err != nil {
3639
return err

api/oci/config/httpconfig_test.go

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,39 @@ configurations:
109109
Expect(err.Error()).To(ContainSubstring("invalid duration: notaduration"))
110110
})
111111

112-
It("rejects negative duration", func() {
112+
DescribeTable("rejects negative duration for timeout fields",
113+
func(field string, cfg *config.HTTPConfig) {
114+
ctx := cpi.New()
115+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(MatchError(
116+
ContainSubstring("invalid value for " + field),
117+
))
118+
},
119+
Entry("timeout", "timeout",
120+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("-5m")}}),
121+
Entry("tcpDialTimeout", "tcpDialTimeout",
122+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPDialTimeout: dur("-5m")}}),
123+
Entry("tlsHandshakeTimeout", "tlsHandshakeTimeout",
124+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TLSHandshakeTimeout: dur("-5m")}}),
125+
Entry("responseHeaderTimeout", "responseHeaderTimeout",
126+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{ResponseHeaderTimeout: dur("-5m")}}),
127+
Entry("idleConnTimeout", "idleConnTimeout",
128+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("-5m")}}),
129+
)
130+
131+
It("allows -1 tcpKeepAlive to disable keep-alive probes", func() {
113132
ctx := cpi.New()
114-
raw := []byte(`{"type":"http.config.ocm.software/v1alpha1","timeout":"-5m"}`)
115-
_, err := ctx.ConfigContext().GetConfigForData(raw, nil)
116-
Expect(err).To(HaveOccurred())
117-
Expect(err.Error()).To(ContainSubstring("negative duration not allowed: -5m"))
133+
cfg := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPKeepAlive: dur("-1s")}}
134+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
135+
g := MustGetHTTPSettings(ctx)
136+
Expect(g.TCPKeepAlive.TimeDuration()).To(HaveValue(Equal(-1 * time.Second)))
137+
})
138+
139+
It("rejects tcpKeepAlive values more negative than -1", func() {
140+
ctx := cpi.New()
141+
cfg := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPKeepAlive: dur("-2s")}}
142+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(MatchError(
143+
ContainSubstring("invalid value for tcpKeepAlive"),
144+
))
118145
})
119146

120147
It("default settings are nil", func() {

api/oci/extensions/repositories/ocireg/repository.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ func configureTransport(ctx cpi.Context, scheme string, creds credentials.Creden
205205
}
206206

207207
if scheme == "https" {
208+
//nolint:gosec // there are OCI servers (quay.io) not working with min version.
208209
baseTransport.TLSClientConfig = &tls.Config{
209210
RootCAs: func() *x509.CertPool {
210211
rootCAs := rootcertsattr.Get(ctx).GetRootCertPool(true)

api/oci/internal/httpsettings.go

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package internal
33
import (
44
"encoding/json"
55
"fmt"
6+
"strings"
67
"time"
78
)
89

@@ -11,20 +12,14 @@ import (
1112
type Duration string
1213

1314
// UnmarshalJSON implements the json.Unmarshaller interface.
14-
// Negative durations are rejected because timeout values must be
15-
// zero (disabled) or positive.
1615
func (d *Duration) UnmarshalJSON(b []byte) error {
1716
var str string
1817
if err := json.Unmarshal(b, &str); err != nil {
1918
return err
2019
}
21-
pd, err := time.ParseDuration(str)
22-
if err != nil {
20+
if _, err := time.ParseDuration(str); err != nil {
2321
return fmt.Errorf("invalid duration: %s", str)
2422
}
25-
if pd < 0 {
26-
return fmt.Errorf("negative duration not allowed: %s", str)
27-
}
2823
*d = Duration(str)
2924
return nil
3025
}
@@ -44,6 +39,30 @@ func (d *Duration) TimeDuration() (*time.Duration, error) {
4439
return &pd, nil
4540
}
4641

42+
// nonNegative requires the duration to be zero or positive.
43+
func nonNegative(d Duration) bool {
44+
return !strings.HasPrefix(string(d), "-")
45+
}
46+
47+
// nonNegativeOrMinusOne requires the duration to be zero, positive,
48+
// or -1 (to disable keep-alive probes).
49+
func nonNegativeOrMinusOne(d Duration) bool {
50+
return !strings.HasPrefix(string(d), "-") || strings.HasPrefix(string(d), "-1")
51+
}
52+
53+
func validateDuration(name string, d *Duration, valid func(Duration) bool) error {
54+
if d == nil {
55+
return nil
56+
}
57+
if _, err := d.TimeDuration(); err != nil {
58+
return err
59+
}
60+
if !valid(*d) {
61+
return fmt.Errorf("invalid value for %s: %s", name, string(*d))
62+
}
63+
return nil
64+
}
65+
4766
// HTTPSettings contains the timeout settings for HTTP clients.
4867
// All timeout values use Duration (Go duration strings in config).
4968
// If not set (nil), the http.DefaultTransport value from the Go
@@ -63,6 +82,7 @@ type HTTPSettings struct {
6382
TCPDialTimeout *Duration `json:"tcpDialTimeout,omitempty"`
6483

6584
// TCPKeepAlive is the interval between TCP keep-alive probes.
85+
// Use -1 to disable keep-alive probes.
6686
TCPKeepAlive *Duration `json:"tcpKeepAlive,omitempty"`
6787

6888
// TLSHandshakeTimeout is the maximum time to wait for a TLS handshake.
@@ -74,3 +94,26 @@ type HTTPSettings struct {
7494
// IdleConnTimeout is the maximum time an idle connection remains open.
7595
IdleConnTimeout *Duration `json:"idleConnTimeout,omitempty"`
7696
}
97+
98+
// Validate checks that timeout values are non-negative.
99+
// TCPKeepAlive additionally allows -1 to disable keep-alive probes
100+
// (consistent with Go's net.Dialer.KeepAlive).
101+
func (s *HTTPSettings) Validate() error {
102+
for _, check := range []struct {
103+
name string
104+
val *Duration
105+
valid func(Duration) bool
106+
}{
107+
{"timeout", s.Timeout, nonNegative},
108+
{"tcpDialTimeout", s.TCPDialTimeout, nonNegative},
109+
{"tcpKeepAlive", s.TCPKeepAlive, nonNegativeOrMinusOne},
110+
{"tlsHandshakeTimeout", s.TLSHandshakeTimeout, nonNegative},
111+
{"responseHeaderTimeout", s.ResponseHeaderTimeout, nonNegative},
112+
{"idleConnTimeout", s.IdleConnTimeout, nonNegative},
113+
} {
114+
if err := validateDuration(check.name, check.val, check.valid); err != nil {
115+
return err
116+
}
117+
}
118+
return nil
119+
}

0 commit comments

Comments
 (0)