Skip to content

Commit aef8ae3

Browse files
Merge pull request #42 from nicholas-fedor/38-bug---unchecked-input-size
Fix Input Validation Issues on Client and Server Sides
2 parents 73b8712 + 8dbaadd commit aef8ae3

File tree

14 files changed

+282
-57
lines changed

14 files changed

+282
-57
lines changed

.golangci.yaml

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
linters:
2+
enable-all: true
3+
disable-all: false
4+
fast: true
5+
disable:
6+
- cyclop
7+
- depguard
8+
- funlen
9+
- gci
10+
- gochecknoinits
11+
- gocognit
12+
- lll
13+
- nestif
14+
- testpackage
15+
16+
linters-settings:
17+
gci:
18+
skip-generated: true
19+
no-inline-comments: true
20+
no-prefix-comments: true
21+
custom-order: false
22+
no-lex-order: false
23+
sections:
24+
- standard
25+
- default
26+
gofmt:
27+
simplify: true
28+
29+
output:
30+
print-issued-lines: true
31+
print-linter-name: false
32+
path-prefix: ""
33+
show-stats: false
34+
sort-results: true

cmd/server/main.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,32 +30,41 @@ type Config struct {
3030

3131
// LoadConfig loads server configuration from environment variables.
3232
// It defaults to port ":8080" if PORT is unset and processes TRUSTED_PROXIES as a comma-separated list,
33-
// trimming whitespace and logging warnings for empty entries. Returns the configuration and any error encountered.
33+
// trimming whitespace, logging warnings for empty entries, and filtering them out. Returns the configuration and any error encountered.
3434
func LoadConfig() (Config, error) {
3535
config := Config{Port: defaultPort, StaticDir: defaultStaticDir}
3636
if port := os.Getenv("PORT"); port != "" {
3737
config.Port = ":" + port
3838
}
39+
3940
if proxies := os.Getenv(trustedProxiesEnv); proxies != "" {
40-
config.TrustedProxies = strings.Split(proxies, ",")
41-
for i, proxy := range config.TrustedProxies {
42-
config.TrustedProxies[i] = strings.TrimSpace(proxy)
43-
if config.TrustedProxies[i] == "" {
41+
proxyList := strings.Split(proxies, ",")
42+
43+
var validProxies []string
44+
45+
for _, proxy := range proxyList {
46+
trimmedProxy := strings.TrimSpace(proxy)
47+
if trimmedProxy == "" {
4448
slog.Warn("Empty proxy entry in TRUSTED_PROXIES")
49+
} else {
50+
validProxies = append(validProxies, trimmedProxy)
4551
}
4652
}
53+
54+
config.TrustedProxies = validProxies
4755
}
56+
4857
if staticDir := os.Getenv(staticDirEnv); staticDir != "" {
4958
config.StaticDir = staticDir
5059
} else {
51-
// Resolve defaultStaticDir relative to the executable's directory
5260
exePath, err := os.Executable()
5361
if err != nil {
5462
slog.Warn("Failed to determine executable path, using default static dir", "error", err)
5563
} else {
5664
config.StaticDir = filepath.Join(filepath.Dir(exePath), defaultStaticDir)
5765
}
5866
}
67+
5968
return config, nil
6069
}
6170

@@ -96,6 +105,7 @@ func main() {
96105
}
97106

98107
slog.Info("Starting server", "port", config.Port, "static_dir", config.StaticDir)
108+
99109
if err := router.Run(config.Port); err != nil {
100110
slog.Error("Server failed", "error", err)
101111
os.Exit(1)

cmd/server/main_test.go

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"net/http"
55
"net/http/httptest"
66
"net/url"
7+
"os"
78
"path/filepath"
89
"strings"
910
"testing"
@@ -16,15 +17,19 @@ import (
1617
// It loads the server configuration and sets up the router, failing the test if either step encounters an error.
1718
func setupRouter(t *testing.T) *gin.Engine {
1819
t.Helper()
20+
1921
config, err := LoadConfig()
2022
if err != nil {
2123
t.Fatalf("Failed to load config: %v", err)
2224
}
25+
2326
config.StaticDir = filepath.Join("..", "..", "static")
27+
2428
r, err := SetupRouter(config)
2529
if err != nil {
2630
t.Fatalf("Failed to setup router: %v", err)
2731
}
32+
2833
return r
2934
}
3035

@@ -88,13 +93,15 @@ func TestRouterSetup(t *testing.T) {
8893
for _, tt := range tests {
8994
t.Run(tt.name, func(t *testing.T) {
9095
router := setupRouter(t)
96+
9197
var req *http.Request
9298
if tt.method == "POST" {
9399
req, _ = http.NewRequest(tt.method, tt.path, strings.NewReader(tt.formData.Encode()))
94100
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
95101
} else {
96102
req, _ = http.NewRequest(tt.method, tt.path, nil)
97103
}
104+
98105
resp := httptest.NewRecorder()
99106
router.ServeHTTP(resp, req)
100107
assert.Equal(t, tt.wantStatus, resp.Code, "Status code")
@@ -117,6 +124,7 @@ func TestTrustedProxies(t *testing.T) {
117124
wantBody string
118125
wantError bool
119126
}{
127+
// Existing test cases remain unchanged
120128
{
121129
name: "No trusted proxies",
122130
trustedProxies: "",
@@ -145,6 +153,16 @@ func TestTrustedProxies(t *testing.T) {
145153
wantBody: "",
146154
wantError: true,
147155
},
156+
// New test case for Line 45 - Empty proxy entry
157+
{
158+
name: "Trusted proxies with empty entry",
159+
trustedProxies: "192.168.1.1,,10.0.0.1",
160+
remoteAddr: "192.168.1.1:12345",
161+
xForwardedFor: "203.0.113.1",
162+
wantClientIP: "203.0.113.1",
163+
wantStatus: http.StatusOK,
164+
wantBody: "EUI-64 Calculator",
165+
},
148166
}
149167

150168
for _, tt := range tests {
@@ -158,32 +176,98 @@ func TestTrustedProxies(t *testing.T) {
158176
if tt.wantError {
159177
_, err := SetupRouter(Config{Port: defaultPort, TrustedProxies: strings.Split(tt.trustedProxies, ",")})
160178
assert.Error(t, err, "Expected error for invalid proxy")
179+
161180
return
162181
}
163182

164183
router := setupRouter(t)
165-
req, _ := http.NewRequest("GET", "/", nil)
184+
req, _ := http.NewRequest(http.MethodGet, "/", nil)
166185
req.RemoteAddr = tt.remoteAddr
186+
167187
if tt.xForwardedFor != "" {
168188
req.Header.Set("X-Forwarded-For", tt.xForwardedFor)
169189
}
190+
170191
resp := httptest.NewRecorder()
171192
router.ServeHTTP(resp, req)
172193
assert.Equal(t, tt.wantStatus, resp.Code, "Status code")
173194
assert.Contains(t, resp.Body.String(), tt.wantBody, "Response body")
174195

175196
var gotClientIP string
197+
176198
router.GET("/test-ip", func(c *gin.Context) {
177199
gotClientIP = c.ClientIP()
178200
})
179-
req, _ = http.NewRequest("GET", "/test-ip", nil)
201+
202+
req, _ = http.NewRequest(http.MethodGet, "/test-ip", nil)
180203
req.RemoteAddr = tt.remoteAddr
204+
181205
if tt.xForwardedFor != "" {
182206
req.Header.Set("X-Forwarded-For", tt.xForwardedFor)
183207
}
208+
184209
resp = httptest.NewRecorder()
185210
router.ServeHTTP(resp, req)
186211
assert.Equal(t, tt.wantClientIP, gotClientIP, "Client IP")
187212
})
188213
}
189214
}
215+
216+
// New TestLoadConfig to cover Lines 37 and 56.
217+
func TestLoadConfig(t *testing.T) {
218+
tests := []struct {
219+
name string
220+
portEnv string
221+
trustedProxies string
222+
staticDirEnv string
223+
wantPort string
224+
wantProxies []string
225+
wantStaticDir string
226+
}{
227+
{
228+
name: "Default config",
229+
portEnv: "",
230+
trustedProxies: "",
231+
staticDirEnv: "",
232+
wantPort: defaultPort,
233+
wantProxies: nil,
234+
wantStaticDir: filepath.Join(filepath.Dir(os.Args[0]), defaultStaticDir), // Approximation
235+
},
236+
{
237+
name: "Custom port (Line 37)",
238+
portEnv: "9090",
239+
trustedProxies: "",
240+
staticDirEnv: "",
241+
wantPort: ":9090",
242+
wantProxies: nil,
243+
wantStaticDir: filepath.Join(filepath.Dir(os.Args[0]), defaultStaticDir),
244+
},
245+
{
246+
name: "Custom static dir",
247+
portEnv: "",
248+
trustedProxies: "",
249+
staticDirEnv: "/custom/static",
250+
wantPort: defaultPort,
251+
wantProxies: nil,
252+
wantStaticDir: "/custom/static",
253+
},
254+
// Note: Line 56 (os.Executable failure) is harder to test directly without mocking.
255+
// We can assume it works if STATIC_DIR is unset and defaultStaticDir is used.
256+
}
257+
258+
for _, tt := range tests {
259+
t.Run(tt.name, func(t *testing.T) {
260+
// Clear and set environment variables
261+
t.Setenv("PORT", tt.portEnv)
262+
t.Setenv(trustedProxiesEnv, tt.trustedProxies)
263+
t.Setenv(staticDirEnv, tt.staticDirEnv)
264+
265+
config, err := LoadConfig()
266+
assert.NoError(t, err, "LoadConfig should not error")
267+
268+
assert.Equal(t, tt.wantPort, config.Port, "Port")
269+
assert.Equal(t, tt.wantProxies, config.TrustedProxies, "TrustedProxies")
270+
assert.Equal(t, tt.wantStaticDir, config.StaticDir, "StaticDir")
271+
})
272+
}
273+
}

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ require (
1111

1212
require (
1313
github.com/andybalholm/cascadia v1.3.3 // indirect
14-
github.com/bytedance/sonic v1.12.8 // indirect
14+
github.com/bytedance/sonic v1.12.10 // indirect
1515
github.com/bytedance/sonic/loader v0.2.3 // indirect
1616
github.com/cloudwego/base64x v0.1.5 // indirect
1717
github.com/davecgh/go-spew v1.1.1 // indirect
@@ -22,7 +22,7 @@ require (
2222
github.com/go-playground/validator/v10 v10.25.0 // indirect
2323
github.com/goccy/go-json v0.10.5 // indirect
2424
github.com/json-iterator/go v1.1.12 // indirect
25-
github.com/klauspost/cpuid/v2 v2.2.9 // indirect
25+
github.com/klauspost/cpuid/v2 v2.2.10 // indirect
2626
github.com/leodido/go-urn v1.4.0 // indirect
2727
github.com/mattn/go-isatty v0.0.20 // indirect
2828
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
@@ -32,7 +32,7 @@ require (
3232
github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
3333
github.com/ugorji/go/codec v1.2.12 // indirect
3434
golang.org/x/arch v0.14.0 // indirect
35-
golang.org/x/crypto v0.33.0 // indirect
35+
golang.org/x/crypto v0.35.0 // indirect
3636
golang.org/x/net v0.35.0 // indirect
3737
golang.org/x/sys v0.30.0 // indirect
3838
golang.org/x/text v0.22.0 // indirect

go.sum

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ github.com/a-h/templ v0.3.833 h1:L/KOk/0VvVTBegtE0fp2RJQiBm7/52Zxv5fqlEHiQUU=
44
github.com/a-h/templ v0.3.833/go.mod h1:cAu4AiZhtJfBjMY0HASlyzvkrtjnHWPeEsyGK2YYmfk=
55
github.com/andybalholm/cascadia v1.3.3 h1:AG2YHrzJIm4BZ19iwJ/DAua6Btl3IwJX+VI4kktS1LM=
66
github.com/andybalholm/cascadia v1.3.3/go.mod h1:xNd9bqTn98Ln4DwST8/nG+H0yuB8Hmgu1YHNnWw0GeA=
7-
github.com/bytedance/sonic v1.12.8 h1:4xYRVRlXIgvSZ4e8iVTlMF5szgpXd4AfvuWgA8I8lgs=
8-
github.com/bytedance/sonic v1.12.8/go.mod h1:uVvFidNmlt9+wa31S1urfwwthTWteBgG0hWuoKAXTx8=
7+
github.com/bytedance/sonic v1.12.10 h1:uVCQr6oS5669E9ZVW0HyksTLfNS7Q/9hV6IVS4nEMsI=
8+
github.com/bytedance/sonic v1.12.10/go.mod h1:uVvFidNmlt9+wa31S1urfwwthTWteBgG0hWuoKAXTx8=
99
github.com/bytedance/sonic/loader v0.1.1/go.mod h1:ncP89zfokxS5LZrJxl5z0UJcsk4M4yY2JpfqGeCtNLU=
1010
github.com/bytedance/sonic/loader v0.2.3 h1:yctD0Q3v2NOGfSWPLPvG2ggA2kV6TS6s4wioyEqssH0=
1111
github.com/bytedance/sonic/loader v0.2.3/go.mod h1:N8A3vUdtUebEY2/VQC0MyhYeKUFosQU6FxH2JmUe6VI=
@@ -37,8 +37,8 @@ github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/
3737
github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
3838
github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
3939
github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg=
40-
github.com/klauspost/cpuid/v2 v2.2.9 h1:66ze0taIn2H33fBvCkXuv9BmCwDfafmiIVpKV9kKGuY=
41-
github.com/klauspost/cpuid/v2 v2.2.9/go.mod h1:rqkxqrZ1EhYM9G+hXH7YdowN5R5RGN6NK4QwQ3WMXF8=
40+
github.com/klauspost/cpuid/v2 v2.2.10 h1:tBs3QSyvjDyFTq3uoc/9xFpCuOsJQFNPiAhYdw2skhE=
41+
github.com/klauspost/cpuid/v2 v2.2.10/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0=
4242
github.com/knz/go-libedit v1.10.1/go.mod h1:MZTVkCWyz0oBc7JOWP3wNAzd002ZbM/5hgShxwh4x8M=
4343
github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ=
4444
github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI=
@@ -78,8 +78,8 @@ golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliY
7878
golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
7979
golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8=
8080
golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
81-
golang.org/x/crypto v0.33.0 h1:IOBPskki6Lysi0lo9qQvbxiQ+FvsCC/YWOecCHAixus=
82-
golang.org/x/crypto v0.33.0/go.mod h1:bVdXmD7IV/4GdElGPozy6U7lWdRXA4qyRVGJV57uQ5M=
81+
golang.org/x/crypto v0.35.0 h1:b15kiHdrGCHrP6LvwaQ3c03kgNhhiMgvlhxHQhmg2Xs=
82+
golang.org/x/crypto v0.35.0/go.mod h1:dy7dXNW32cAb/6/PRuTNsix8T+vJAqvuIy5Bli/x0YQ=
8383
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
8484
golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
8585
golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=

0 commit comments

Comments
 (0)