Browse Source

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
feature/tus-protocol
chrislu 2 days ago
parent
commit
21d1a2d167
  1. 2
      test/tus/README.md
  2. 36
      test/tus/tus_integration_test.go
  3. 8
      weed/server/filer_server_tus_handlers.go

2
test/tus/README.md

@ -201,7 +201,7 @@ curl -X DELETE http://localhost:18888/.tus/.uploads/{upload-id} \
## Architecture ## Architecture
```
```text
Client Filer Volume Servers Client Filer Volume Servers
| | | | | |
|-- POST /.tus/path/file.mp4 ->| | |-- POST /.tus/path/file.mp4 ->| |

36
test/tus/tus_integration_test.go

@ -93,7 +93,11 @@ func startTestCluster(t *testing.T, ctx context.Context) (*TestCluster, error) {
"-mdir", masterDir, "-mdir", masterDir,
"-ip", "127.0.0.1", "-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.Stdout = masterLogFile
masterCmd.Stderr = masterLogFile masterCmd.Stderr = masterLogFile
if err := masterCmd.Start(); err != nil { if err := masterCmd.Start(); err != nil {
@ -102,8 +106,12 @@ func startTestCluster(t *testing.T, ctx context.Context) (*TestCluster, error) {
} }
cluster.masterCmd = masterCmd 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 // Start volume server
volumeCmd := exec.CommandContext(ctx, weedBinary, "volume", 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, "-mserver", "127.0.0.1:"+testMasterPort,
"-ip", "127.0.0.1", "-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.Stdout = volumeLogFile
volumeCmd.Stderr = volumeLogFile volumeCmd.Stderr = volumeLogFile
if err := volumeCmd.Start(); err != nil { if err := volumeCmd.Start(); err != nil {
@ -122,8 +135,12 @@ func startTestCluster(t *testing.T, ctx context.Context) (*TestCluster, error) {
} }
cluster.volumeCmd = volumeCmd 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 // Start filer with TUS enabled
filerCmd := exec.CommandContext(ctx, weedBinary, "filer", 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", "-ip", "127.0.0.1",
"-dataCenter", "dc1", "-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.Stdout = filerLogFile
filerCmd.Stderr = filerLogFile filerCmd.Stderr = filerLogFile
if err := filerCmd.Start(); err != nil { if err := filerCmd.Start(); err != nil {

8
weed/server/filer_server_tus_handlers.go

@ -7,6 +7,7 @@ import (
"fmt" "fmt"
"io" "io"
"net/http" "net/http"
"path"
"strconv" "strconv"
"strings" "strings"
"time" "time"
@ -131,8 +132,11 @@ func (fs *FilerServer) tusCreateHandler(w http.ResponseWriter, r *http.Request)
return 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 // Handle creation-with-upload extension
if r.ContentLength > 0 && r.Header.Get("Content-Type") == "application/offset+octet-stream" { if r.ContentLength > 0 && r.Header.Get("Content-Type") == "application/offset+octet-stream" {

Loading…
Cancel
Save