From 4993c9769d0cedefe98268f653032a1afcd1df30 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Wed, 28 Aug 2024 11:57:12 -0700 Subject: [PATCH] Use a newRequest function rather than shallow clone Because otherwise the body is not copied. --- p2p/http/auth/auth_test.go | 30 +++++++++++++++--------------- p2p/http/auth/client.go | 13 ++++++++----- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/p2p/http/auth/auth_test.go b/p2p/http/auth/auth_test.go index fab82e9580..83e133f81a 100644 --- a/p2p/http/auth/auth_test.go +++ b/p2p/http/auth/auth_test.go @@ -108,21 +108,30 @@ func TestMutualAuth(t *testing.T) { expectedServerID, err := peer.IDFromPrivateKey(serverKey) require.NoError(t, err) - req, err := http.NewRequest("POST", ts.URL, nil) - require.NoError(t, err) - req.Host = "example.com" - serverID, resp, err := clientAuth.AuthenticatedDo(client, req) + newReq := func() *http.Request { + req, err := http.NewRequest("POST", ts.URL, nil) + require.NoError(t, err) + req.Host = "example.com" + return req + } + serverID, resp, err := clientAuth.AuthenticatedDo(client, newReq) require.NoError(t, err) require.Equal(t, expectedServerID, serverID) require.NotZero(t, clientAuth.tokenMap["example.com"]) require.Equal(t, http.StatusOK, resp.StatusCode) // Once more with the auth token - req, err = http.NewRequest("POST", ts.URL, nil) + req, err := http.NewRequest("POST", ts.URL, nil) require.NoError(t, err) req.Host = "example.com" - serverID, resp, err = clientAuth.AuthenticatedDo(client, req) + timesCalled := 0 + newReq = func() *http.Request { + timesCalled++ + return req + } + serverID, resp, err = clientAuth.AuthenticatedDo(client, newReq) require.NotEmpty(t, req.Header.Get("Authorization")) + require.Equal(t, 1, timesCalled, "should only call newRequest once since we have a token") require.NoError(t, err) require.Equal(t, expectedServerID, serverID) require.NotZero(t, clientAuth.tokenMap["example.com"]) @@ -131,12 +140,3 @@ func TestMutualAuth(t *testing.T) { } } } - -// // Test Vectors -// var zeroBytes = make([]byte, 64) -// var zeroKey, _, _ = crypto.GenerateEd25519Key(bytes.NewReader(zeroBytes)) - -// // Peer ID derived from the zero key -// var zeroID, _ = peer.IDFromPublicKey(zeroKey.GetPublic()) - -// TODO add generator for specs table & example diff --git a/p2p/http/auth/client.go b/p2p/http/auth/client.go index 874a4519b7..cb7c4856de 100644 --- a/p2p/http/auth/client.go +++ b/p2p/http/auth/client.go @@ -23,9 +23,10 @@ type tokenInfo struct { } // AuthenticatedDo is like http.Client.Do, but it does the libp2p peer ID auth handshake if needed. -func (a *ClientPeerIDAuth) AuthenticatedDo(client *http.Client, req *http.Request) (peer.ID, *http.Response, error) { - clonedReq := req.Clone(req.Context()) - +// Takes in a function that creates a new request, so that we can retry the +// request if we need to authenticate. +func (a *ClientPeerIDAuth) AuthenticatedDo(client *http.Client, newRequest func() *http.Request) (peer.ID, *http.Response, error) { + req := newRequest() hostname := req.Host a.tokenMapMu.Lock() if a.tokenMap == nil { @@ -59,9 +60,11 @@ func (a *ClientPeerIDAuth) AuthenticatedDo(client *http.Client, req *http.Reques if err != nil { return "", nil, fmt.Errorf("failed to run handshake: %w", err) } - handshake.SetHeader(clonedReq.Header) - resp, err = client.Do(clonedReq) + req = newRequest() + handshake.SetHeader(req.Header) + + resp, err = client.Do(req) if err != nil { return "", nil, fmt.Errorf("failed to do authenticated request: %w", err) }