aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorchrislu <chris.lu@gmail.com>2025-12-01 15:15:25 -0800
committerchrislu <chris.lu@gmail.com>2025-12-01 15:15:25 -0800
commit21d1a2d167f4bda4cdbe08aa31a3f6e1910fa6b6 (patch)
treea491a92003d700ed9a02caa61cd41f4f7f1bbdf1
parent35192b8bb8e4a0198593ce8fc858dc3d177b8df2 (diff)
downloadseaweedfs-21d1a2d167f4bda4cdbe08aa31a3f6e1910fa6b6.tar.xz
seaweedfs-21d1a2d167f4bda4cdbe08aa31a3f6e1910fa6b6.zip
Address remaining code review comments
- Fix potential open redirect vulnerability by sanitizing uploadLocation path - Add language specifier to README code block - Handle os.Create errors in test setup - Use waitForHTTPServer instead of time.Sleep for master/volume readiness - Improve test reliability and debugging
-rw-r--r--test/tus/README.md2
-rw-r--r--test/tus/tus_integration_test.go36
-rw-r--r--weed/server/filer_server_tus_handlers.go8
3 files changed, 36 insertions, 10 deletions
diff --git a/test/tus/README.md b/test/tus/README.md
index cd014229e..03c980a3d 100644
--- a/test/tus/README.md
+++ b/test/tus/README.md
@@ -201,7 +201,7 @@ curl -X DELETE http://localhost:18888/.tus/.uploads/{upload-id} \
## Architecture
-```
+```text
Client Filer Volume Servers
| | |
|-- POST /.tus/path/file.mp4 ->| |
diff --git a/test/tus/tus_integration_test.go b/test/tus/tus_integration_test.go
index b2c9bb8e8..75763344b 100644
--- a/test/tus/tus_integration_test.go
+++ b/test/tus/tus_integration_test.go
@@ -93,7 +93,11 @@ func startTestCluster(t *testing.T, ctx context.Context) (*TestCluster, error) {
"-mdir", masterDir,
"-ip", "127.0.0.1",
)
- masterLogFile, _ := os.Create(filepath.Join(masterDir, "master.log"))
+ masterLogFile, err := os.Create(filepath.Join(masterDir, "master.log"))
+ if err != nil {
+ os.RemoveAll(dataDir)
+ return nil, fmt.Errorf("failed to create master log: %v", err)
+ }
masterCmd.Stdout = masterLogFile
masterCmd.Stderr = masterLogFile
if err := masterCmd.Start(); err != nil {
@@ -102,8 +106,12 @@ func startTestCluster(t *testing.T, ctx context.Context) (*TestCluster, error) {
}
cluster.masterCmd = masterCmd
- // Wait for master
- time.Sleep(2 * time.Second)
+ // Wait for master to be ready
+ if err := waitForHTTPServer("http://127.0.0.1:"+testMasterPort+"/dir/status", 30*time.Second); err != nil {
+ cluster.Stop()
+ os.RemoveAll(dataDir)
+ return nil, fmt.Errorf("master not ready: %v", err)
+ }
// Start volume server
volumeCmd := exec.CommandContext(ctx, weedBinary, "volume",
@@ -112,7 +120,12 @@ func startTestCluster(t *testing.T, ctx context.Context) (*TestCluster, error) {
"-mserver", "127.0.0.1:"+testMasterPort,
"-ip", "127.0.0.1",
)
- volumeLogFile, _ := os.Create(filepath.Join(volumeDir, "volume.log"))
+ volumeLogFile, err := os.Create(filepath.Join(volumeDir, "volume.log"))
+ if err != nil {
+ cluster.Stop()
+ os.RemoveAll(dataDir)
+ return nil, fmt.Errorf("failed to create volume log: %v", err)
+ }
volumeCmd.Stdout = volumeLogFile
volumeCmd.Stderr = volumeLogFile
if err := volumeCmd.Start(); err != nil {
@@ -122,8 +135,12 @@ func startTestCluster(t *testing.T, ctx context.Context) (*TestCluster, error) {
}
cluster.volumeCmd = volumeCmd
- // Wait for volume server
- time.Sleep(2 * time.Second)
+ // Wait for volume server to register with master
+ if err := waitForHTTPServer("http://127.0.0.1:"+testVolumePort+"/status", 30*time.Second); err != nil {
+ cluster.Stop()
+ os.RemoveAll(dataDir)
+ return nil, fmt.Errorf("volume server not ready: %v", err)
+ }
// Start filer with TUS enabled
filerCmd := exec.CommandContext(ctx, weedBinary, "filer",
@@ -132,7 +149,12 @@ func startTestCluster(t *testing.T, ctx context.Context) (*TestCluster, error) {
"-ip", "127.0.0.1",
"-dataCenter", "dc1",
)
- filerLogFile, _ := os.Create(filepath.Join(filerDir, "filer.log"))
+ filerLogFile, err := os.Create(filepath.Join(filerDir, "filer.log"))
+ if err != nil {
+ cluster.Stop()
+ os.RemoveAll(dataDir)
+ return nil, fmt.Errorf("failed to create filer log: %v", err)
+ }
filerCmd.Stdout = filerLogFile
filerCmd.Stderr = filerLogFile
if err := filerCmd.Start(); err != nil {
diff --git a/weed/server/filer_server_tus_handlers.go b/weed/server/filer_server_tus_handlers.go
index f4fa5959e..7411b2527 100644
--- a/weed/server/filer_server_tus_handlers.go
+++ b/weed/server/filer_server_tus_handlers.go
@@ -7,6 +7,7 @@ import (
"fmt"
"io"
"net/http"
+ "path"
"strconv"
"strings"
"time"
@@ -131,8 +132,11 @@ func (fs *FilerServer) tusCreateHandler(w http.ResponseWriter, r *http.Request)
return
}
- // Build upload location URL
- uploadLocation := fmt.Sprintf("%s/.uploads/%s", tusPrefix, uploadID)
+ // Build upload location URL (ensure it starts with single /)
+ uploadLocation := path.Clean(fmt.Sprintf("%s/.uploads/%s", tusPrefix, uploadID))
+ if !strings.HasPrefix(uploadLocation, "/") {
+ uploadLocation = "/" + uploadLocation
+ }
// Handle creation-with-upload extension
if r.ContentLength > 0 && r.Header.Get("Content-Type") == "application/offset+octet-stream" {