Skip to content

Commit 516bc6d

Browse files
committed
fix(lint,test): use reflect.Pointer and remove TLS test races
- Replace deprecated reflect.Ptr alias with reflect.Pointer in lookup (govet inline analyzer in golangci-lint v2.12.2 flags the alias) - Bind TLS test listeners synchronously via startTLSServer helper to fix a connection-refused race in TestTLSLogging/TestTLSConfig where the server was signaled ready before the port was bound - Replace the flaky external expired.badssl.com call in TestHTTP/Skip_SSL_Verification with a local expired-cert server, and assert a verifying client rejects it so the test proves skip-verify
1 parent 51910b8 commit 516bc6d

3 files changed

Lines changed: 84 additions & 49 deletions

File tree

http/http_test.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,22 @@ func TestHTTP(t *testing.T) {
5757
})
5858

5959
t.Run("Skip SSL Verification", func(t *testing.T) {
60-
req := http.NewClient().InsecureSkipVerify(true).R(ctx)
61-
response, err := req.Get("https://expired.badssl.com/")
60+
port := "18091"
61+
defer startTLSServer(t, port, expiredServerTLS(t))()
62+
target := fmt.Sprintf("https://localhost:%s/", port)
63+
64+
// A verifying client (explicit TLSConfig opts out of the default
65+
// insecure-skip behaviour) must reject the expired certificate.
66+
verifying, err := http.NewClient().TLSConfig(http.TLSConfig{})
67+
if err != nil {
68+
t.Fatalf("error: %v", err)
69+
}
70+
if _, err := verifying.R(ctx).Get(target); err == nil {
71+
t.Fatal("expected TLS verification error for expired certificate")
72+
}
73+
74+
// InsecureSkipVerify bypasses the expiry check and the request succeeds.
75+
response, err := http.NewClient().InsecureSkipVerify(true).R(ctx).Get(target)
6276
if err != nil {
6377
t.Fatalf("error: %v", err)
6478
}

http/http_tls_test.go

Lines changed: 65 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"fmt"
1313
"math"
1414
"math/big"
15+
"net"
1516
"net/http"
1617
"testing"
1718
"time"
@@ -112,26 +113,7 @@ func TestTLSConfig(t *testing.T) {
112113
for _, td := range testData {
113114
t.Run(td.name, func(t *testing.T) {
114115
port := "18080"
115-
server, err := tlsServer(port, td.serverTLS)
116-
if err != nil {
117-
t.Fatal(err)
118-
}
119-
120-
serverReady := make(chan struct{})
121-
go func() {
122-
close(serverReady)
123-
err := server.ListenAndServeTLS("", "")
124-
logger.Infof("server error: %v", err)
125-
}()
126-
127-
serverTerminate := make(chan struct{})
128-
go func() {
129-
<-serverTerminate
130-
_ = server.Shutdown(context.Background())
131-
}()
132-
133-
<-serverReady
134-
defer func() { serverTerminate <- struct{}{} }()
116+
defer startTLSServer(t, port, td.serverTLS)()
135117

136118
client, err := chttp.NewClient().TLSConfig(td.clientTLS)
137119
if err != nil {
@@ -161,15 +143,31 @@ func TestTLSConfig(t *testing.T) {
161143
}
162144
}
163145

164-
func tlsServer(port string, tlsConfig *tls.Config) (*http.Server, error) {
146+
// startTLSServer binds the listener synchronously before serving so the port is
147+
// guaranteed to accept connections by the time it returns. It returns a cleanup
148+
// function that shuts the server down.
149+
func startTLSServer(t *testing.T, port string, tlsConfig *tls.Config) func() {
150+
t.Helper()
151+
152+
listener, err := net.Listen("tcp", net.JoinHostPort("", port))
153+
if err != nil {
154+
t.Fatal(err)
155+
}
156+
165157
server := &http.Server{
166-
Addr: fmt.Sprintf(":%s", port),
167158
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
168159
_, _ = w.Write([]byte("Hello, World!"))
169160
}),
170161
TLSConfig: tlsConfig,
171162
}
172-
return server, nil
163+
164+
go func() {
165+
if err := server.ServeTLS(listener, "", ""); err != nil && err != http.ErrServerClosed {
166+
logger.Infof("server error: %v", err)
167+
}
168+
}()
169+
170+
return func() { _ = server.Shutdown(context.Background()) }
173171
}
174172

175173
func createCert(parent *x509.Certificate, signerKey any, cn string) (*x509.Certificate, *tls.Certificate, []byte, []byte, error) {
@@ -240,6 +238,48 @@ func createCert(parent *x509.Certificate, signerKey any, cn string) (*x509.Certi
240238
return template, &certificate, pemBytes, privateKeyBytes, nil
241239
}
242240

241+
// expiredServerTLS returns a *tls.Config holding a self-signed certificate that
242+
// already expired, for exercising InsecureSkipVerify against an invalid cert
243+
// without depending on an external host.
244+
func expiredServerTLS(t *testing.T) *tls.Config {
245+
t.Helper()
246+
247+
template := &x509.Certificate{
248+
SerialNumber: big.NewInt(1),
249+
Subject: pkix.Name{CommonName: "localhost"},
250+
DNSNames: []string{"localhost"},
251+
NotBefore: time.Now().Add(-48 * time.Hour),
252+
NotAfter: time.Now().Add(-24 * time.Hour),
253+
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment,
254+
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
255+
}
256+
257+
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
258+
if err != nil {
259+
t.Fatal(err)
260+
}
261+
262+
der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key)
263+
if err != nil {
264+
t.Fatal(err)
265+
}
266+
267+
keyDER, err := x509.MarshalECPrivateKey(key)
268+
if err != nil {
269+
t.Fatal(err)
270+
}
271+
272+
cert, err := tls.X509KeyPair(
273+
pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}),
274+
pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}),
275+
)
276+
if err != nil {
277+
t.Fatal(err)
278+
}
279+
280+
return &tls.Config{Certificates: []tls.Certificate{cert}}
281+
}
282+
243283
func TestTLSLogging(t *testing.T) {
244284
// Enable trace logging to see TLS output
245285
logger.StandardLogger().SetLogLevel(5)
@@ -261,28 +301,9 @@ func TestTLSLogging(t *testing.T) {
261301

262302
t.Run("TLS logging with valid certificate", func(t *testing.T) {
263303
port := "18090"
264-
server, err := tlsServer(port, &tls.Config{
304+
defer startTLSServer(t, port, &tls.Config{
265305
Certificates: []tls.Certificate{*serverCrt},
266-
})
267-
if err != nil {
268-
t.Fatal(err)
269-
}
270-
271-
serverReady := make(chan struct{})
272-
go func() {
273-
close(serverReady)
274-
err := server.ListenAndServeTLS("", "")
275-
logger.Infof("server error: %v", err)
276-
}()
277-
278-
serverTerminate := make(chan struct{})
279-
go func() {
280-
<-serverTerminate
281-
_ = server.Shutdown(context.Background())
282-
}()
283-
284-
<-serverReady
285-
defer func() { serverTerminate <- struct{}{} }()
306+
})()
286307

287308
client, err := chttp.NewClient().TLSConfig(chttp.TLSConfig{CA: string(caPEM)})
288309
if err != nil {

lookup/lookup.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func getValueByName(v reflect.Value, key string) (reflect.Value, error) {
9797
return value, err
9898
}
9999
switch v.Kind() {
100-
case reflect.Ptr, reflect.Interface:
100+
case reflect.Pointer, reflect.Interface:
101101
return getValueByName(v.Elem(), key)
102102
case reflect.Struct:
103103
if f, ok := ValueByTagName(v, key); ok {
@@ -123,7 +123,7 @@ func getValueByName(v reflect.Value, key string) (reflect.Value, error) {
123123
value = value.Index(index)
124124
}
125125

126-
if value.Kind() == reflect.Ptr || value.Kind() == reflect.Interface {
126+
if value.Kind() == reflect.Pointer || value.Kind() == reflect.Interface {
127127
value = value.Elem()
128128
}
129129

@@ -313,7 +313,7 @@ func lookupType(ty reflect.Type, path ...string) (reflect.Type, bool) {
313313
}
314314
// Aggregate.
315315
return lookupType(ty.Elem(), path...)
316-
case reflect.Ptr:
316+
case reflect.Pointer:
317317
return lookupType(ty.Elem(), path...)
318318
case reflect.Interface:
319319
// We can't know from here without a value. Let's just return this type.

0 commit comments

Comments
 (0)