diff --git a/CHANGELOG.md b/CHANGELOG.md index d0a64b4e7..f48638807 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ The following emojis are used to highlight certain changes: - 🛠 `boxo/gateway`: when making a trustless CAR request with the "entity-bytes" parameter, using a negative index greater than the underlying entity length could trigger reading more data than intended - 🛠 `boxo/gateway`: the header configuration `Config.Headers` and `AddAccessControlHeaders` has been replaced by the new middleware provided by `NewHeaders`. +- 🛠 `routing/http/client`: the default HTTP client is no longer a global singleton. Therefore, using `WithUserAgent` won't modify the user agent of existing routing clients. This will also prevent potential race conditions. In addition, incompatible options will now return errors instead of silently failing. ### Security diff --git a/routing/http/client/client.go b/routing/http/client/client.go index efadc2732..16840cab5 100644 --- a/routing/http/client/client.go +++ b/routing/http/client/client.go @@ -28,15 +28,8 @@ import ( ) var ( - _ contentrouter.Client = &Client{} - logger = logging.Logger("routing/http/client") - defaultHTTPClient = &http.Client{ - Transport: &ResponseBodyLimitedTransport{ - RoundTripper: http.DefaultTransport, - LimitBytes: 1 << 20, - UserAgent: defaultUserAgent, - }, - } + _ contentrouter.Client = &Client{} + logger = logging.Logger("routing/http/client") ) const ( @@ -67,53 +60,75 @@ var defaultUserAgent = moduleVersion() var _ contentrouter.Client = &Client{} +func newDefaultHTTPClient(userAgent string) *http.Client { + return &http.Client{ + Transport: &ResponseBodyLimitedTransport{ + RoundTripper: http.DefaultTransport, + LimitBytes: 1 << 20, + UserAgent: userAgent, + }, + } +} + type httpClient interface { Do(req *http.Request) (*http.Response, error) } -type Option func(*Client) +type Option func(*Client) error func WithIdentity(identity crypto.PrivKey) Option { - return func(c *Client) { + return func(c *Client) error { c.identity = identity + return nil } } +// WithHTTPClient sets a custom HTTP Client to be used with [Client]. func WithHTTPClient(h httpClient) Option { - return func(c *Client) { + return func(c *Client) error { c.httpClient = h + return nil } } +// WithUserAgent sets a custom user agent to use with the HTTP Client. This modifies +// the underlying [http.Client]. Therefore, you should not use the same HTTP Client +// with multiple routing clients. +// +// This only works if using a [http.Client] with a [ResponseBodyLimitedTransport] +// set as its transport. Otherwise, an error will be returned. func WithUserAgent(ua string) Option { - return func(c *Client) { + return func(c *Client) error { if ua == "" { - return + return errors.New("empty user agent") } httpClient, ok := c.httpClient.(*http.Client) if !ok { - return + return errors.New("the http client of the Client must be a *http.Client") } transport, ok := httpClient.Transport.(*ResponseBodyLimitedTransport) if !ok { - return + return errors.New("the transport of the http client of the Client must be a *ResponseBodyLimitedTransport") } transport.UserAgent = ua + return nil } } func WithProviderInfo(peerID peer.ID, addrs []multiaddr.Multiaddr) Option { - return func(c *Client) { + return func(c *Client) error { c.peerID = peerID for _, a := range addrs { c.addrs = append(c.addrs, types.Multiaddr{Multiaddr: a}) } + return nil } } func WithStreamResultsRequired() Option { - return func(c *Client) { + return func(c *Client) error { c.accepts = mediaTypeNDJSON + return nil } } @@ -122,13 +137,16 @@ func WithStreamResultsRequired() Option { func New(baseURL string, opts ...Option) (*Client, error) { client := &Client{ baseURL: baseURL, - httpClient: defaultHTTPClient, + httpClient: newDefaultHTTPClient(defaultUserAgent), clock: clock.New(), accepts: strings.Join([]string{mediaTypeNDJSON, mediaTypeJSON}, ","), } for _, opt := range opts { - opt(client) + err := opt(client) + if err != nil { + return nil, err + } } if client.identity != nil && client.peerID.Size() != 0 && !client.peerID.MatchesPublicKey(client.identity.GetPublic()) { diff --git a/routing/http/client/client_test.go b/routing/http/client/client_test.go index 7edd77c10..590deed11 100644 --- a/routing/http/client/client_test.go +++ b/routing/http/client/client_test.go @@ -109,11 +109,10 @@ func makeTestDeps(t *testing.T, clientsOpts []Option, serverOpts []server.Option server := httptest.NewServer(recordingHandler) t.Cleanup(server.Close) serverAddr := "http://" + server.Listener.Addr().String() - recordingHTTPClient := &recordingHTTPClient{httpClient: defaultHTTPClient} + recordingHTTPClient := &recordingHTTPClient{httpClient: newDefaultHTTPClient(testUserAgent)} defaultClientOpts := []Option{ WithProviderInfo(peerID, addrs), WithIdentity(identity), - WithUserAgent(testUserAgent), WithHTTPClient(recordingHTTPClient), } c, err := New(serverAddr, append(defaultClientOpts, clientsOpts...)...)