Skip to content

Commit 9059943

Browse files
authored
Merge pull request #489 from maxmind/wstorey/transport-change
Avoid mutating http.DefaultTransport in NewUpdater
2 parents b935db5 + 455cddf commit 9059943

4 files changed

Lines changed: 40 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
- BREAKING CHANGE: `geoipupdate` stops updating on the first error.
66
- A config file can now be used to configure the Docker image. This was
77
documented but was not functional. Fixed by Shizun Ge. GitHub #433.
8+
- `NewUpdater` in the unexported `internal/geoipupdate` package no longer
9+
mutates `http.DefaultTransport` when a proxy is configured. The CLI is the
10+
only caller, so there is no user-visible behavior change. Reported by B.
11+
Conners. GitHub #488.
812

913
## 7.1.1 (2025-07-09)
1014

internal/geoipupdate/geoip_updater.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,12 @@ type Updater struct {
3535

3636
// NewUpdater initialized a new Updater struct.
3737
func NewUpdater(config *Config) (*Updater, error) {
38-
transport := http.DefaultTransport
38+
httpClient := &http.Client{}
3939
if config.Proxy != nil {
40-
proxyFunc := http.ProxyURL(config.Proxy)
41-
transport.(*http.Transport).Proxy = proxyFunc
40+
transport := http.DefaultTransport.(*http.Transport).Clone()
41+
transport.Proxy = http.ProxyURL(config.Proxy)
42+
httpClient.Transport = transport
4243
}
43-
httpClient := &http.Client{Transport: transport}
4444

4545
updateClient, err := client.New(
4646
config.AccountID,

internal/geoipupdate/geoip_updater_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ import (
1111
"log"
1212
"net/http"
1313
"net/http/httptest"
14+
"net/url"
1415
"os"
1516
"path/filepath"
17+
"reflect"
1618
"strings"
1719
"testing"
1820
"time"
@@ -232,6 +234,35 @@ func TestRetryWhenWriting(t *testing.T) {
232234
require.Contains(t, logOutput.String(), `"edition_id":"foo-db-name"`)
233235
}
234236

237+
// TestNewUpdaterDoesNotMutateDefaultTransport verifies that NewUpdater does not
238+
// modify http.DefaultTransport when a proxy is configured. See issue #488.
239+
func TestNewUpdaterDoesNotMutateDefaultTransport(t *testing.T) {
240+
defaultTransport := http.DefaultTransport.(*http.Transport)
241+
originalProxy := defaultTransport.Proxy
242+
243+
proxyURL, err := url.Parse("http://proxy.example.com:8080")
244+
require.NoError(t, err)
245+
246+
tempDir := t.TempDir()
247+
config := &Config{
248+
AccountID: 10,
249+
LicenseKey: "foo",
250+
EditionIDs: []string{"GeoLite2-City"},
251+
DatabaseDirectory: tempDir,
252+
LockFile: filepath.Join(tempDir, ".geoipupdate.lock"),
253+
Proxy: proxyURL,
254+
}
255+
256+
_, err = NewUpdater(config)
257+
require.NoError(t, err)
258+
259+
require.Equal(
260+
t,
261+
reflect.ValueOf(originalProxy).Pointer(),
262+
reflect.ValueOf(defaultTransport.Proxy).Pointer(),
263+
)
264+
}
265+
235266
type mockUpdateClient struct {
236267
i int
237268
outputs []client.DownloadResponse

mise.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)