Skip to content

Commit 82a8808

Browse files
committed
handler: fix race for progress hook
fixes #1320 Signed-off-by: ferhat elmas <[email protected]>
1 parent 606f3eb commit 82a8808

File tree

2 files changed

+96
-1
lines changed

2 files changed

+96
-1
lines changed

pkg/handler/hooks.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func newHookEvent(c *httpContext, info FileInfo) HookEvent {
4242
Method: c.req.Method,
4343
URI: c.req.RequestURI,
4444
RemoteAddr: c.req.RemoteAddr,
45-
Header: c.req.Header,
45+
Header: c.req.Header.Clone(),
4646
},
4747
}
4848
}

pkg/handler/hooks_test.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package handler
2+
3+
import (
4+
"encoding/json"
5+
"net/http/httptest"
6+
"sync"
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
// TestHookEventHeaderRace demonstrates a race condition that occurs when
13+
// the http.Header map is shared between the original request and HookEvent.
14+
// This test will fail with `go test -race` if Header is not cloned.
15+
//
16+
// See: https://github.com/tus/tusd/issues/1320
17+
func TestHookEventHeaderRace(t *testing.T) {
18+
req := httptest.NewRequest("POST", "/files", nil)
19+
req.Header.Set("Upload-Length", "1000")
20+
req.Header.Set("Tus-Resumable", "1.0.0")
21+
req.Host = "example.com"
22+
23+
c := &httpContext{
24+
req: req,
25+
}
26+
27+
info := FileInfo{
28+
ID: "test-id",
29+
Size: 1000,
30+
MetaData: map[string]string{"filename": "test.txt"},
31+
}
32+
33+
event := newHookEvent(c, info)
34+
35+
var wg sync.WaitGroup
36+
const iterations = 100
37+
38+
// Goroutine 1: Simulate async hook processing (JSON encoding)
39+
// This is what happens in invokeHookAsync -> json.Marshal
40+
wg.Add(1)
41+
go func() {
42+
defer wg.Done()
43+
for range iterations {
44+
// This iterates over the Header map
45+
_, _ = json.Marshal(event.HTTPRequest)
46+
}
47+
}()
48+
49+
// Goroutine 2: Simulate concurrent request header modification
50+
// This could happen if another hook event is created for the same request
51+
// or if middleware modifies headers
52+
wg.Add(1)
53+
go func() {
54+
defer wg.Done()
55+
for range iterations {
56+
// This writes to the Header map
57+
c.req.Header.Set("X-Request-ID", "some-value")
58+
c.req.Header.Del("X-Request-ID")
59+
}
60+
}()
61+
62+
wg.Wait()
63+
}
64+
65+
// TestHookEventHeaderIsolation verifies that modifying the original request
66+
// headers after creating a HookEvent does not affect the event headers.
67+
func TestHookEventHeaderIsolation(t *testing.T) {
68+
req := httptest.NewRequest("POST", "/files", nil)
69+
req.Header.Set("X-Original", "original-value")
70+
req.Host = "example.com"
71+
72+
c := &httpContext{
73+
req: req,
74+
}
75+
76+
info := FileInfo{
77+
ID: "test-id",
78+
Size: 1000,
79+
}
80+
81+
event := newHookEvent(c, info)
82+
83+
// Verify original header is present
84+
assert.Equal(t, "original-value", event.HTTPRequest.Header.Get("X-Original"))
85+
86+
// Modify the original request header
87+
c.req.Header.Set("X-Original", "modified-value")
88+
c.req.Header.Set("X-New", "new-value")
89+
90+
// Verify event headers are NOT affected (isolation)
91+
assert.Equal(t, "original-value", event.HTTPRequest.Header.Get("X-Original"),
92+
"event header should remain unchanged after modifying original request")
93+
assert.Empty(t, event.HTTPRequest.Header.Get("X-New"),
94+
"new header added to original request should not appear in event")
95+
}

0 commit comments

Comments
 (0)