mirror of
https://github.com/Rain-kl/OpenFlare.git
synced 2026-09-29 22:06:38 +08:00
fix(objectstore): decouple WebDAV targetPath from logical storage key and fix basePath duplication
- WebDAV Put now returns PutResult with driver-agnostic logical relative key (relKey), keeping database records decoupled from mount basePath. - targetPath mounts logical keys to the remote WebDAV server path, and transparently handles legacy database records containing basePath or duplicate basePath prefixes. - Make localBackend path resolution resilient to keys with leading slashes or legacy absolute paths outside local root by safely mounting them as relative paths. - Add comprehensive unit tests for WebDAV targetPath, relKey, end-to-end roundtrip with in-memory WebDAV server, and local storage leading slash handling.
This commit is contained in:
@@ -91,30 +91,32 @@ func (b *localBackend) Test(_ context.Context) error {
|
||||
return os.MkdirAll(b.root, storageDirPerm)
|
||||
}
|
||||
|
||||
func isWithinRoot(root, target string) bool {
|
||||
absRoot, err := filepath.Abs(root)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
absTarget, err := filepath.Abs(target)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
rel, err := filepath.Rel(absRoot, absTarget)
|
||||
return err == nil && !strings.HasPrefix(rel, "..")
|
||||
}
|
||||
|
||||
func (b *localBackend) path(key string) (string, error) {
|
||||
if filepath.IsAbs(key) {
|
||||
cleanPath := filepath.Clean(key)
|
||||
absRoot, err := filepath.Abs(b.root)
|
||||
if err != nil {
|
||||
return "", err
|
||||
if isWithinRoot(b.root, cleanPath) {
|
||||
return cleanPath, nil
|
||||
}
|
||||
absPath, err := filepath.Abs(cleanPath)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
rel, err := filepath.Rel(absRoot, absPath)
|
||||
if err != nil || strings.HasPrefix(rel, "..") {
|
||||
return "", errors.New("storage key escapes local root")
|
||||
}
|
||||
return cleanPath, nil
|
||||
}
|
||||
cleanKey := filepath.Clean(filepath.FromSlash(strings.TrimPrefix(key, "/")))
|
||||
if cleanKey == "." || cleanKey == "" || strings.HasPrefix(cleanKey, "..") {
|
||||
return "", fmt.Errorf("invalid local storage key %q", key)
|
||||
}
|
||||
path := filepath.Join(b.root, cleanKey)
|
||||
rel, err := filepath.Rel(b.root, path)
|
||||
if err != nil || strings.HasPrefix(rel, "..") {
|
||||
if !isWithinRoot(b.root, path) {
|
||||
return "", errors.New("storage key escapes local root")
|
||||
}
|
||||
return path, nil
|
||||
|
||||
@@ -48,4 +48,19 @@ func TestLocalBackendRoundTrip(t *testing.T) {
|
||||
if _, err := backend.Get(ctx, key); err == nil {
|
||||
t.Errorf("Get(%q) after Delete() returned nil error", key)
|
||||
}
|
||||
|
||||
// Test key with leading slash
|
||||
slashKey := "/uploads/2026/06/13/slash_test.txt"
|
||||
putSlashRes, err := backend.Put(ctx, slashKey, bytes.NewBufferString(content), int64(len(content)), "text/plain")
|
||||
if err != nil {
|
||||
t.Fatalf("Put(%q) returned error: %v", slashKey, err)
|
||||
}
|
||||
if putSlashRes.Key != slashKey {
|
||||
t.Errorf("Put(%q) key = %q, want %q", slashKey, putSlashRes.Key, slashKey)
|
||||
}
|
||||
objSlash, err := backend.Get(ctx, slashKey)
|
||||
if err != nil {
|
||||
t.Fatalf("Get(%q) returned error: %v", slashKey, err)
|
||||
}
|
||||
_ = objSlash.Body.Close()
|
||||
}
|
||||
|
||||
@@ -32,11 +32,15 @@ type webDAVBackend struct {
|
||||
}
|
||||
|
||||
func newWebDAVBackend(cfg WebDAVConfig) (*webDAVBackend, error) {
|
||||
basePath := strings.Trim(path.Clean("/"+cfg.BasePath), "/")
|
||||
if basePath == "." {
|
||||
basePath = ""
|
||||
}
|
||||
return &webDAVBackend{
|
||||
endpoint: strings.TrimRight(cfg.Endpoint, "/"),
|
||||
username: cfg.Username,
|
||||
password: cfg.Password,
|
||||
basePath: strings.Trim(cfg.BasePath, "/"),
|
||||
basePath: basePath,
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -50,27 +54,27 @@ func (b *webDAVBackend) newClient(ctx context.Context) *gowebdav.Client {
|
||||
}
|
||||
|
||||
func (b *webDAVBackend) Put(ctx context.Context, key string, body io.Reader, size int64, _ string) (PutResult, error) {
|
||||
key = b.key(key)
|
||||
target := b.targetPath(key)
|
||||
client := b.newClient(ctx)
|
||||
if dir := path.Dir(key); dir != "." && dir != "/" {
|
||||
if dir := path.Dir(target); dir != "." && dir != "/" {
|
||||
if err := client.MkdirAll(dir, storageDirPerm); err != nil {
|
||||
return PutResult{}, fmt.Errorf("create WebDAV directory: %w", err)
|
||||
}
|
||||
}
|
||||
if err := client.WriteStreamWithLength(key, body, size, storageFilePerm); err != nil {
|
||||
if err := client.WriteStreamWithLength(target, body, size, storageFilePerm); err != nil {
|
||||
return PutResult{}, fmt.Errorf("put WebDAV object: %w", err)
|
||||
}
|
||||
return PutResult{Key: key}, nil
|
||||
return PutResult{Key: b.relKey(key)}, nil
|
||||
}
|
||||
|
||||
func (b *webDAVBackend) Get(ctx context.Context, key string) (*Object, error) {
|
||||
key = b.key(key)
|
||||
target := b.targetPath(key)
|
||||
client := b.newClient(ctx)
|
||||
info, err := client.Stat(key)
|
||||
info, err := client.Stat(target)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("stat WebDAV object: %w", err)
|
||||
}
|
||||
body, err := client.ReadStream(key)
|
||||
body, err := client.ReadStream(target)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("get WebDAV object: %w", err)
|
||||
}
|
||||
@@ -83,7 +87,7 @@ func (b *webDAVBackend) Get(ctx context.Context, key string) (*Object, error) {
|
||||
|
||||
func (b *webDAVBackend) Delete(ctx context.Context, key string) error {
|
||||
client := b.newClient(ctx)
|
||||
if err := client.Remove(b.key(key)); err != nil {
|
||||
if err := client.Remove(b.targetPath(key)); err != nil {
|
||||
return fmt.Errorf("delete WebDAV object: %w", err)
|
||||
}
|
||||
return nil
|
||||
@@ -97,6 +101,34 @@ func (b *webDAVBackend) Test(ctx context.Context) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (b *webDAVBackend) key(key string) string {
|
||||
return "/" + path.Join(b.basePath, strings.TrimLeft(key, "/"))
|
||||
// relKey extracts the clean, normalized, relative logical key (e.g. "uploads/2026/09/17/xxx.jpg")
|
||||
// to be persisted in the database, stripping any driver-specific basePath and leading slashes.
|
||||
func (b *webDAVBackend) relKey(key string) string {
|
||||
cleanKey := strings.Trim(path.Clean("/"+strings.ReplaceAll(key, "\\", "/")), "/")
|
||||
if cleanKey == "." {
|
||||
return ""
|
||||
}
|
||||
if b.basePath != "" {
|
||||
for cleanKey == b.basePath || strings.HasPrefix(cleanKey, b.basePath+"/") {
|
||||
cleanKey = strings.TrimPrefix(cleanKey, b.basePath)
|
||||
cleanKey = strings.TrimPrefix(cleanKey, "/")
|
||||
}
|
||||
}
|
||||
return cleanKey
|
||||
}
|
||||
|
||||
// targetPath resolves any key (relative, legacy with basePath, or corrupted with duplicate basePath)
|
||||
// into the absolute path used to access the object on the WebDAV server.
|
||||
func (b *webDAVBackend) targetPath(key string) string {
|
||||
rel := b.relKey(key)
|
||||
if b.basePath == "" {
|
||||
if rel == "" {
|
||||
return "/"
|
||||
}
|
||||
return "/" + rel
|
||||
}
|
||||
if rel == "" {
|
||||
return "/" + b.basePath
|
||||
}
|
||||
return "/" + b.basePath + "/" + rel
|
||||
}
|
||||
|
||||
@@ -0,0 +1,257 @@
|
||||
// Copyright 2026 Arctel.net
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
|
||||
package objectstore
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"io"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"golang.org/x/net/webdav"
|
||||
)
|
||||
|
||||
func TestWebDAVTargetPath(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
basePath string
|
||||
key string
|
||||
expected string
|
||||
}{
|
||||
{
|
||||
name: "no base path, relative key",
|
||||
basePath: "",
|
||||
key: "uploads/2026/09/17/1.jpg",
|
||||
expected: "/uploads/2026/09/17/1.jpg",
|
||||
},
|
||||
{
|
||||
name: "no base path, leading slash key",
|
||||
basePath: "",
|
||||
key: "/uploads/2026/09/17/1.jpg",
|
||||
expected: "/uploads/2026/09/17/1.jpg",
|
||||
},
|
||||
{
|
||||
name: "with base path, relative key (uploading)",
|
||||
basePath: "/DockerData/openflare_data_webdav",
|
||||
key: "uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
{
|
||||
name: "with base path, key with leading slash (uploading)",
|
||||
basePath: "/DockerData/openflare_data_webdav",
|
||||
key: "/uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
{
|
||||
name: "with base path, key already has base path (reading from legacy DB)",
|
||||
basePath: "/DockerData/openflare_data_webdav",
|
||||
key: "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
{
|
||||
name: "with base path without leading slash, key already has base path",
|
||||
basePath: "DockerData/openflare_data_webdav",
|
||||
key: "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
{
|
||||
name: "with base path with trailing slash, key already has base path",
|
||||
basePath: "/DockerData/openflare_data_webdav/",
|
||||
key: "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
{
|
||||
name: "with base path, key already has double base path from previous bug",
|
||||
basePath: "/DockerData/openflare_data_webdav",
|
||||
key: "/DockerData/openflare_data_webdav/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
{
|
||||
name: "key with similar prefix name that is not a directory match",
|
||||
basePath: "/data",
|
||||
key: "/data_backup/uploads/1.jpg",
|
||||
expected: "/data/data_backup/uploads/1.jpg",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
backend, err := newWebDAVBackend(WebDAVConfig{
|
||||
Endpoint: "http://127.0.0.1:5005",
|
||||
BasePath: tt.basePath,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("newWebDAVBackend failed: %v", err)
|
||||
}
|
||||
actual := backend.targetPath(tt.key)
|
||||
if actual != tt.expected {
|
||||
t.Errorf("targetPath(%q) = %q, want %q", tt.key, actual, tt.expected)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWebDAVRelKey(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
basePath string
|
||||
key string
|
||||
expected string
|
||||
}{
|
||||
{
|
||||
name: "relative key remains relative",
|
||||
basePath: "/DockerData/openflare_data_webdav",
|
||||
key: "uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
{
|
||||
name: "leading slash key is stripped to relative",
|
||||
basePath: "/DockerData/openflare_data_webdav",
|
||||
key: "/uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
{
|
||||
name: "legacy key with basePath is stripped to relative",
|
||||
basePath: "/DockerData/openflare_data_webdav",
|
||||
key: "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
{
|
||||
name: "corrupted key with duplicate basePath is stripped to relative",
|
||||
basePath: "/DockerData/openflare_data_webdav",
|
||||
key: "/DockerData/openflare_data_webdav/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
{
|
||||
name: "empty basePath preserves relative key",
|
||||
basePath: "",
|
||||
key: "/uploads/2026/09/17/105102954490499072.jpg",
|
||||
expected: "uploads/2026/09/17/105102954490499072.jpg",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
backend, err := newWebDAVBackend(WebDAVConfig{
|
||||
Endpoint: "http://127.0.0.1:5005",
|
||||
BasePath: tt.basePath,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("newWebDAVBackend failed: %v", err)
|
||||
}
|
||||
actual := backend.relKey(tt.key)
|
||||
if actual != tt.expected {
|
||||
t.Errorf("relKey(%q) = %q, want %q", tt.key, actual, tt.expected)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWebDAVBackendRoundTrip(t *testing.T) {
|
||||
wdHandler := &webdav.Handler{
|
||||
FileSystem: webdav.NewMemFS(),
|
||||
LockSystem: webdav.NewMemLS(),
|
||||
}
|
||||
server := httptest.NewServer(wdHandler)
|
||||
defer server.Close()
|
||||
|
||||
ctx := context.Background()
|
||||
basePath := "/DockerData/openflare_data_webdav"
|
||||
backend, err := newWebDAVBackend(WebDAVConfig{
|
||||
Endpoint: server.URL,
|
||||
BasePath: basePath,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("newWebDAVBackend failed: %v", err)
|
||||
}
|
||||
|
||||
// 1. Test connection
|
||||
if err := backend.Test(ctx); err != nil {
|
||||
t.Fatalf("backend.Test failed: %v", err)
|
||||
}
|
||||
|
||||
// 2. Put object using relative key (typical upload flow)
|
||||
origContent := []byte("test image content 12345")
|
||||
objectKey := "uploads/2026/09/17/105102954490499072.jpg"
|
||||
putRes, err := backend.Put(ctx, objectKey, bytes.NewReader(origContent), int64(len(origContent)), "image/jpeg")
|
||||
if err != nil {
|
||||
t.Fatalf("backend.Put failed: %v", err)
|
||||
}
|
||||
|
||||
// PutResult.Key MUST be the pure logical relative key, decoupled from basePath
|
||||
expectedLogicalKey := "uploads/2026/09/17/105102954490499072.jpg"
|
||||
if putRes.Key != expectedLogicalKey {
|
||||
t.Errorf("putRes.Key = %q, want %q", putRes.Key, expectedLogicalKey)
|
||||
}
|
||||
|
||||
// 3. Get object using logical key (new standard upload flow)
|
||||
obj, err := backend.Get(ctx, putRes.Key)
|
||||
if err != nil {
|
||||
t.Fatalf("backend.Get with putRes.Key failed: %v", err)
|
||||
}
|
||||
defer obj.Body.Close()
|
||||
|
||||
bodyBytes, err := io.ReadAll(obj.Body)
|
||||
if err != nil {
|
||||
t.Fatalf("read body failed: %v", err)
|
||||
}
|
||||
if !bytes.Equal(bodyBytes, origContent) {
|
||||
t.Errorf("read content = %q, want %q", string(bodyBytes), string(origContent))
|
||||
}
|
||||
|
||||
// 4. Get object using legacy key containing basePath (existing DB records from before fix)
|
||||
legacyKey := "/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg"
|
||||
obj2, err := backend.Get(ctx, legacyKey)
|
||||
if err != nil {
|
||||
t.Fatalf("backend.Get with legacyKey failed: %v", err)
|
||||
}
|
||||
defer obj2.Body.Close()
|
||||
bodyBytes2, err := io.ReadAll(obj2.Body)
|
||||
if err != nil {
|
||||
t.Fatalf("read body failed: %v", err)
|
||||
}
|
||||
if !bytes.Equal(bodyBytes2, origContent) {
|
||||
t.Errorf("read content = %q, want %q", string(bodyBytes2), string(origContent))
|
||||
}
|
||||
|
||||
// 5. Get object using accidental double basePath (defensive recovery for corrupted DB records)
|
||||
doubledKey := "/DockerData/openflare_data_webdav/DockerData/openflare_data_webdav/uploads/2026/09/17/105102954490499072.jpg"
|
||||
obj3, err := backend.Get(ctx, doubledKey)
|
||||
if err != nil {
|
||||
t.Fatalf("backend.Get with doubledKey failed: %v", err)
|
||||
}
|
||||
defer obj3.Body.Close()
|
||||
bodyBytes3, err := io.ReadAll(obj3.Body)
|
||||
if err != nil {
|
||||
t.Fatalf("read body failed: %v", err)
|
||||
}
|
||||
if !bytes.Equal(bodyBytes3, origContent) {
|
||||
t.Errorf("read content = %q, want %q", string(bodyBytes3), string(origContent))
|
||||
}
|
||||
|
||||
// 6. Delete object using logical key
|
||||
if err := backend.Delete(ctx, putRes.Key); err != nil {
|
||||
t.Fatalf("backend.Delete failed: %v", err)
|
||||
}
|
||||
|
||||
// 7. Verify object is deleted
|
||||
_, err = backend.Get(ctx, putRes.Key)
|
||||
if err == nil {
|
||||
t.Fatalf("expected error after delete, got nil")
|
||||
}
|
||||
|
||||
// 8. Put again, and delete using legacy key format
|
||||
_, err = backend.Put(ctx, objectKey, bytes.NewReader(origContent), int64(len(origContent)), "image/jpeg")
|
||||
if err != nil {
|
||||
t.Fatalf("second backend.Put failed: %v", err)
|
||||
}
|
||||
if err := backend.Delete(ctx, legacyKey); err != nil {
|
||||
t.Fatalf("backend.Delete with legacyKey failed: %v", err)
|
||||
}
|
||||
_, err = backend.Get(ctx, objectKey)
|
||||
if err == nil {
|
||||
t.Fatalf("expected error after delete with legacyKey, got nil")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user