Skip to content

Commit 0cb0ee1

Browse files
committed
Address round 2 review feedback from Aneurysm9
- Remove .sisyphus/handoff/no-proxy-regression-analysis.md (not part of PR) - Delete unused getProxyAddress/getProxyURL and their tests (dead code) - Simplify Rewrite comment (Director deprecation detail in commit history) - Read from r.In.Body instead of r.Out.Body for hash computation
1 parent 7b9af19 commit 0cb0ee1

4 files changed

Lines changed: 2 additions & 275 deletions

File tree

.sisyphus/handoff/no-proxy-regression-analysis.md

Lines changed: 0 additions & 125 deletions
This file was deleted.

internal/aws/awsutil/conn.go

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"errors"
1111
"net/http"
1212
"net/url"
13-
"os"
1413
"time"
1514

1615
"github.com/aws/aws-sdk-go-v2/aws"
@@ -57,34 +56,6 @@ func newHTTPClient(logger *zap.Logger, maxIdle, requestTimeout int, noVerify boo
5756
return http, err
5857
}
5958

60-
// getProxyAddress returns the proxy address from the provided value or HTTPS_PROXY environment variable.
61-
func getProxyAddress(proxyAddress string) string {
62-
var finalProxyAddress string
63-
switch {
64-
case proxyAddress != "":
65-
finalProxyAddress = proxyAddress
66-
67-
case proxyAddress == "" && os.Getenv("HTTPS_PROXY") != "":
68-
finalProxyAddress = os.Getenv("HTTPS_PROXY")
69-
default:
70-
finalProxyAddress = ""
71-
}
72-
return finalProxyAddress
73-
}
74-
75-
// getProxyURL parses and returns the proxy URL from the provided address.
76-
func getProxyURL(finalProxyAddress string) (*url.URL, error) {
77-
var proxyURL *url.URL
78-
var err error
79-
if finalProxyAddress != "" {
80-
proxyURL, err = url.Parse(finalProxyAddress)
81-
} else {
82-
proxyURL = nil
83-
err = nil
84-
}
85-
return proxyURL, err
86-
}
87-
8859
// GetProxyFunc returns a proxy function for use in http.Transport.
8960
// When an explicit proxy address is configured, it returns http.ProxyURL.
9061
// Otherwise, it returns http.ProxyFromEnvironment which respects NO_PROXY.

internal/aws/awsutil/conn_test.go

Lines changed: 0 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -179,125 +179,6 @@ func TestNewHTTPClientWithCustomTimeout(t *testing.T) {
179179
assert.Equal(t, 60*time.Second, client.Timeout)
180180
}
181181

182-
// Test getProxyAddress function
183-
func TestGetProxyAddress(t *testing.T) {
184-
testCases := []struct {
185-
name string
186-
proxyAddress string
187-
envProxy string
188-
expected string
189-
}{
190-
{
191-
name: "Explicit proxy address",
192-
proxyAddress: "http://explicit.proxy.com",
193-
envProxy: "http://env.proxy.com",
194-
expected: "http://explicit.proxy.com",
195-
},
196-
{
197-
name: "Environment proxy address",
198-
proxyAddress: "",
199-
envProxy: "http://env.proxy.com",
200-
expected: "http://env.proxy.com",
201-
},
202-
{
203-
name: "No proxy address",
204-
proxyAddress: "",
205-
envProxy: "",
206-
expected: "",
207-
},
208-
}
209-
210-
for _, tc := range testCases {
211-
t.Run(tc.name, func(t *testing.T) {
212-
if tc.envProxy != "" {
213-
t.Setenv("HTTPS_PROXY", tc.envProxy)
214-
} else {
215-
t.Setenv("HTTPS_PROXY", "")
216-
}
217-
218-
result := getProxyAddress(tc.proxyAddress)
219-
assert.Equal(t, tc.expected, result)
220-
})
221-
}
222-
}
223-
224-
// Test getProxyURL function
225-
func TestGetProxyURL(t *testing.T) {
226-
testCases := []struct {
227-
name string
228-
proxyAddr string
229-
expectError bool
230-
expectNil bool
231-
}{
232-
{
233-
name: "Valid proxy URL",
234-
proxyAddr: "http://proxy.example.com:8080",
235-
expectError: false,
236-
expectNil: false,
237-
},
238-
{
239-
name: "Valid proxy URL with IPv6",
240-
proxyAddr: "http://[::1]:8080",
241-
expectError: false,
242-
expectNil: false,
243-
},
244-
{
245-
name: "Valid proxy URL with full IPv6",
246-
proxyAddr: "http://[2001:db8::1]:3128",
247-
expectError: false,
248-
expectNil: false,
249-
},
250-
{
251-
name: "Empty proxy address",
252-
proxyAddr: "",
253-
expectError: false,
254-
expectNil: true,
255-
},
256-
{
257-
name: "Invalid proxy URL - missing scheme",
258-
proxyAddr: "://invalid",
259-
expectError: true,
260-
expectNil: false,
261-
},
262-
{
263-
name: "Invalid proxy URL - bad IPv6",
264-
proxyAddr: "http://[%10::1]",
265-
expectError: true,
266-
expectNil: false,
267-
},
268-
{
269-
name: "Invalid proxy URL - bad escape",
270-
proxyAddr: "http://%41:8080/",
271-
expectError: true,
272-
expectNil: false,
273-
},
274-
{
275-
name: "Invalid proxy URL - space in host",
276-
proxyAddr: "http://a b.com/",
277-
expectError: true,
278-
expectNil: false,
279-
},
280-
}
281-
282-
for _, tc := range testCases {
283-
t.Run(tc.name, func(t *testing.T) {
284-
url, err := getProxyURL(tc.proxyAddr)
285-
286-
if tc.expectError {
287-
assert.Error(t, err)
288-
} else {
289-
assert.NoError(t, err)
290-
}
291-
292-
if tc.expectNil {
293-
assert.Nil(t, url)
294-
} else if !tc.expectError {
295-
assert.NotNil(t, url)
296-
}
297-
})
298-
}
299-
}
300-
301182
func TestNoProxyIsRespectedWhenUsingEnvProxy(t *testing.T) {
302183
t.Setenv("HTTPS_PROXY", "http://mitmproxy:8080")
303184
t.Setenv("NO_PROXY", ".amazonaws.com,localhost,127.0.0.1")

internal/aws/proxy/server.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func NewServer(cfg *Config, logger *zap.Logger) (Server, error) {
7676
region := awsCfg.Region
7777
serviceName := cfg.ServiceName
7878

79-
// Reverse proxy handler using Rewrite (Director is deprecated and insecure)
79+
// Reverse proxy handler
8080
handler := &httputil.ReverseProxy{
8181
Transport: transport,
8282

@@ -91,7 +91,7 @@ func NewServer(cfg *Config, logger *zap.Logger) (Server, error) {
9191
r.Out.Host = awsURL.Host
9292

9393
// Consume body and calculate payload hash for signing
94-
body, payloadHash, err := consumeBody(r.Out.Body)
94+
body, payloadHash, err := consumeBody(r.In.Body)
9595
if err != nil {
9696
logger.Error("Unable to consume request body", zap.Error(err))
9797
return

0 commit comments

Comments
 (0)