diff --git a/.github/workflows/quark-mkdir-hardening-validation.yml b/.github/workflows/quark-mkdir-hardening-validation.yml new file mode 100644 index 00000000000..d75c39195fa --- /dev/null +++ b/.github/workflows/quark-mkdir-hardening-validation.yml @@ -0,0 +1,37 @@ +name: quark-mkdir-hardening-validation + +on: + pull_request: + branches: + - main + +permissions: + contents: read + +jobs: + validate: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: true + - name: Check formatting + run: | + files="drivers/quark_uc/driver.go drivers/quark_uc/driver_test.go" + out="$(gofmt -l $files)" + if [ -n "$out" ]; then + printf '%s\n' "$out" + exit 1 + fi + - name: Vet Quark driver + run: go vet ./drivers/quark_uc + - name: Test Quark driver + run: go test ./drivers/quark_uc + - name: Race test Quark driver + run: go test -race ./drivers/quark_uc + - name: Shuffled repeated race test + run: go test -race -shuffle=on -count=2 ./drivers/quark_uc + - name: Test op and WebDAV + run: go test ./internal/op ./server/webdav diff --git a/drivers/quark_uc/driver.go b/drivers/quark_uc/driver.go index 264150dc920..1236544f9b3 100644 --- a/drivers/quark_uc/driver.go +++ b/drivers/quark_uc/driver.go @@ -4,9 +4,12 @@ import ( "bytes" "context" "encoding/hex" + "fmt" "hash" + "html" "io" "net/http" + "strconv" "strings" "sync" "time" @@ -22,6 +25,14 @@ import ( log "github.com/sirupsen/logrus" ) +const ( + mkdirConflictMessage = "file is doloading[同名冲突]" + mkdirVisibilityAttempts = 6 + mkdirVisibilityInitialBackoff = 200 * time.Millisecond + mkdirVisibilityMaxBackoff = 800 * time.Millisecond + mkdirVisibilityMaxPages = 1000 +) + type QuarkOrUC struct { model.Storage Addition @@ -100,7 +111,7 @@ func (d *QuarkOrUC) Link(ctx context.Context, file model.Obj, args model.LinkArg return d.getDownloadLink(file) } -func (d *QuarkOrUC) MakeDir(ctx context.Context, parentDir model.Obj, dirName string) error { +func (d *QuarkOrUC) MakeDir(ctx context.Context, parentDir model.Obj, dirName string) (model.Obj, error) { data := base.Json{ "dir_init_lock": false, "dir_path": "", @@ -108,12 +119,121 @@ func (d *QuarkOrUC) MakeDir(ctx context.Context, parentDir model.Obj, dirName st "pdir_fid": parentDir.GetID(), } _, err := d.request("/file", http.MethodPost, func(req *resty.Request) { - req.SetBody(data) + req.SetContext(ctx).SetBody(data) }, nil) - if err == nil { - time.Sleep(time.Second) + if err != nil && err.Error() != mkdirConflictMessage { + return nil, err + } + + // Quark may acknowledge a mkdir before /file/sort reflects the new + // directory. Confirm visibility and return the verified object so op.MakeDir + // can update an existing parent cache directly instead of immediately + // re-listing the eventually-consistent backend. + obj, _, visibilityErr := d.waitForDirVisible(ctx, parentDir.GetID(), dirName) + if visibilityErr == nil { + return obj, nil + } + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + if err != nil { + // Preserve the existing Quark conflict error when the directory cannot + // be confirmed. A same-name conflict is only treated as success after + // the matching directory is visible in the parent listing. + return nil, err + } + + // The create request itself succeeded. A bounded visibility miss (whether + // caused by clean-but-stale listings or listing errors) must not turn that + // successful create into a false failure. Returning nil,nil makes + // op.MakeDir clear the parent cache, so later reads cannot remain pinned to + // a stale listing. + return nil, nil +} + +func (d *QuarkOrUC) waitForDirVisible(ctx context.Context, parentID, dirName string) (model.Obj, bool, error) { + backoff := mkdirVisibilityInitialBackoff + var lastErr error + listed := false + for attempt := 0; attempt < mkdirVisibilityAttempts; attempt++ { + if err := ctx.Err(); err != nil { + return nil, listed, err + } + + obj, err := d.findDir(ctx, parentID, dirName) + if err == nil { + lastErr = nil + listed = true + if obj != nil { + return obj, true, nil + } + } else { + lastErr = err + } + if attempt == mkdirVisibilityAttempts-1 { + break + } + + select { + case <-ctx.Done(): + return nil, listed, ctx.Err() + case <-time.After(backoff): + } + if backoff < mkdirVisibilityMaxBackoff { + backoff *= 2 + if backoff > mkdirVisibilityMaxBackoff { + backoff = mkdirVisibilityMaxBackoff + } + } + } + + if lastErr != nil { + return nil, listed, fmt.Errorf("failed to confirm created directory %q: %w", dirName, lastErr) + } + return nil, listed, fmt.Errorf("created directory %q did not become visible after %d attempts", dirName, mkdirVisibilityAttempts) +} + +func (d *QuarkOrUC) findDir(ctx context.Context, parentID, dirName string) (model.Obj, error) { + const size = 100 + query := map[string]string{ + "pdir_fid": parentID, + "_size": strconv.Itoa(size), + "_fetch_total": "1", + "fetch_all_file": "1", + "fetch_risk_file_name": "1", + } + for page := 1; ; page++ { + if err := ctx.Err(); err != nil { + return nil, err + } + if page > mkdirVisibilityMaxPages { + return nil, fmt.Errorf("quark mkdir visibility listing exceeded %d pages for parent %s", mkdirVisibilityMaxPages, parentID) + } + + query["_page"] = strconv.Itoa(page) + var resp SortResp + _, err := d.request("/file/sort", http.MethodGet, func(req *resty.Request) { + req.SetContext(ctx).SetQueryParams(query) + }, &resp) + if err != nil { + return nil, err + } + for i := range resp.Data.List { + file := &resp.Data.List[i] + file.FileName = html.UnescapeString(file.FileName) + if !file.File && file.FileName == dirName { + return file, nil + } + } + + if resp.Metadata.Total > 0 { + if page*size >= resp.Metadata.Total { + return nil, nil + } + } else if len(resp.Data.List) < size { + return nil, nil + } } - return err } func (d *QuarkOrUC) Move(ctx context.Context, srcObj, dstDir model.Obj) error { diff --git a/drivers/quark_uc/driver_test.go b/drivers/quark_uc/driver_test.go new file mode 100644 index 00000000000..435770d1f76 --- /dev/null +++ b/drivers/quark_uc/driver_test.go @@ -0,0 +1,301 @@ +package quark + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/alist-org/alist/v3/internal/model" +) + +func TestMakeDirWaitsUntilDirectoryIsVisible(t *testing.T) { + listCalls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/1/clouddrive/file": + writeJSON(w, http.StatusOK, Resp{Status: 200, Code: 0, Message: "ok"}) + case r.Method == http.MethodGet && r.URL.Path == "/1/clouddrive/file/sort": + listCalls++ + if listCalls == 1 { + writeJSON(w, http.StatusOK, map[string]any{ + "status": 200, + "code": 0, + "data": map[string]any{"list": []any{}}, + "metadata": map[string]any{ + "_size": 100, "_page": 1, "_count": 0, "_total": 0, + }, + }) + return + } + writeDirectoryList(w, "dir-1", "Pool") + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + d := newTestDriver(srv.URL) + parent := &model.Object{ID: "parent", Name: "parent", IsFolder: true} + obj, err := d.MakeDir(context.Background(), parent, "Pool") + if err != nil { + t.Fatalf("MakeDir: %v", err) + } + if obj == nil || obj.GetID() != "dir-1" || obj.GetName() != "Pool" || !obj.IsDir() { + t.Fatalf("MakeDir object = %#v, want visible directory dir-1/Pool", obj) + } + if listCalls != 2 { + t.Fatalf("list calls = %d, want 2", listCalls) + } +} + +func TestMakeDirKeepsSuccessfulCreateWhenVisibilityChecksAllError(t *testing.T) { + listCalls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/1/clouddrive/file": + writeJSON(w, http.StatusOK, Resp{Status: 200, Code: 0, Message: "ok"}) + case r.Method == http.MethodGet && r.URL.Path == "/1/clouddrive/file/sort": + listCalls++ + writeJSON(w, http.StatusInternalServerError, Resp{Status: http.StatusInternalServerError, Code: 500, Message: "temporary list failure"}) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + d := newTestDriver(srv.URL) + parent := &model.Object{ID: "parent", Name: "parent", IsFolder: true} + obj, err := d.MakeDir(context.Background(), parent, "Pool") + if err != nil { + t.Fatalf("MakeDir should preserve successful create: %v", err) + } + if obj != nil { + t.Fatalf("MakeDir object = %#v, want nil so op.MakeDir clears parent cache", obj) + } + if listCalls != mkdirVisibilityAttempts { + t.Fatalf("list calls = %d, want %d", listCalls, mkdirVisibilityAttempts) + } +} + +func TestMakeDirKeepsSuccessfulCreateWhenCleanListingsRemainStale(t *testing.T) { + listCalls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/1/clouddrive/file": + writeJSON(w, http.StatusOK, Resp{Status: 200, Code: 0, Message: "ok"}) + case r.Method == http.MethodGet && r.URL.Path == "/1/clouddrive/file/sort": + listCalls++ + writeJSON(w, http.StatusOK, map[string]any{ + "status": 200, + "code": 0, + "data": map[string]any{"list": []any{}}, + "metadata": map[string]any{ + "_size": 100, "_page": 1, "_count": 0, "_total": 0, + }, + }) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + d := newTestDriver(srv.URL) + parent := &model.Object{ID: "parent", Name: "parent", IsFolder: true} + obj, err := d.MakeDir(context.Background(), parent, "Pool") + if err != nil { + t.Fatalf("MakeDir should preserve successful create despite stale listings: %v", err) + } + if obj != nil { + t.Fatalf("MakeDir object = %#v, want nil so op.MakeDir clears parent cache", obj) + } + if listCalls != mkdirVisibilityAttempts { + t.Fatalf("list calls = %d, want %d", listCalls, mkdirVisibilityAttempts) + } +} + +func TestMakeDirRecoversSameNameConflictAfterVisibilityCheck(t *testing.T) { + listCalls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/1/clouddrive/file": + writeJSON(w, http.StatusConflict, Resp{Status: http.StatusConflict, Code: 32003, Message: mkdirConflictMessage}) + case r.Method == http.MethodGet && r.URL.Path == "/1/clouddrive/file/sort": + listCalls++ + writeDirectoryList(w, "dir-existing", "Pool") + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + d := newTestDriver(srv.URL) + parent := &model.Object{ID: "parent", Name: "parent", IsFolder: true} + obj, err := d.MakeDir(context.Background(), parent, "Pool") + if err != nil { + t.Fatalf("MakeDir should recover verified same-name conflict: %v", err) + } + if obj == nil || obj.GetID() != "dir-existing" || obj.GetName() != "Pool" || !obj.IsDir() { + t.Fatalf("MakeDir object = %#v, want verified existing directory", obj) + } + if listCalls != 1 { + t.Fatalf("list calls = %d, want 1", listCalls) + } +} + +func TestMakeDirPreservesConflictWhenDirectoryNeverAppears(t *testing.T) { + listCalls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/1/clouddrive/file": + writeJSON(w, http.StatusConflict, Resp{Status: http.StatusConflict, Code: 32003, Message: mkdirConflictMessage}) + case r.Method == http.MethodGet && r.URL.Path == "/1/clouddrive/file/sort": + listCalls++ + writeJSON(w, http.StatusOK, map[string]any{ + "status": 200, + "code": 0, + "data": map[string]any{"list": []any{}}, + "metadata": map[string]any{ + "_size": 100, "_page": 1, "_count": 0, "_total": 0, + }, + }) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + d := newTestDriver(srv.URL) + parent := &model.Object{ID: "parent", Name: "parent", IsFolder: true} + obj, err := d.MakeDir(context.Background(), parent, "Pool") + if obj != nil { + t.Fatalf("MakeDir object = %#v, want nil on unverified conflict", obj) + } + if err == nil || err.Error() != mkdirConflictMessage { + t.Fatalf("MakeDir error = %v, want %q", err, mkdirConflictMessage) + } + if listCalls != mkdirVisibilityAttempts { + t.Fatalf("list calls = %d, want %d", listCalls, mkdirVisibilityAttempts) + } +} + +func TestWaitForDirVisibleHonorsCanceledContext(t *testing.T) { + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + http.NotFound(w, r) + })) + defer srv.Close() + + d := newTestDriver(srv.URL) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + obj, _, err := d.waitForDirVisible(ctx, "parent", "Pool") + if obj != nil { + t.Fatalf("waitForDirVisible object = %#v, want nil", obj) + } + if !errors.Is(err, context.Canceled) { + t.Fatalf("waitForDirVisible error = %v, want context.Canceled", err) + } + if requests != 0 { + t.Fatalf("requests = %d, want 0 for pre-canceled context", requests) + } +} + +func TestFindDirRejectsSameNameFile(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || r.URL.Path != "/1/clouddrive/file/sort" { + http.NotFound(w, r) + return + } + writeJSON(w, http.StatusOK, map[string]any{ + "status": 200, + "code": 0, + "data": map[string]any{"list": []map[string]any{{ + "fid": "file-1", "file_name": "Pool", "file": true, + }}}, + "metadata": map[string]any{ + "_size": 100, "_page": 1, "_count": 1, "_total": 1, + }, + }) + })) + defer srv.Close() + + d := newTestDriver(srv.URL) + obj, err := d.findDir(context.Background(), "parent", "Pool") + if err != nil { + t.Fatalf("findDir: %v", err) + } + if obj != nil { + t.Fatalf("findDir accepted a same-name file as a directory: %#v", obj) + } +} + +func TestFindDirPaginatesWithoutTotalAndUnescapesName(t *testing.T) { + listCalls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet || r.URL.Path != "/1/clouddrive/file/sort" { + http.NotFound(w, r) + return + } + listCalls++ + switch r.URL.Query().Get("_page") { + case "1": + list := make([]map[string]any, 100) + for i := range list { + list[i] = map[string]any{ + "fid": "other", "file_name": "Other", "file": false, + } + } + writeJSON(w, http.StatusOK, map[string]any{ + "status": 200, + "code": 0, + "data": map[string]any{"list": list}, + "metadata": map[string]any{ + "_size": 100, "_page": 1, "_count": 100, "_total": 0, + }, + }) + case "2": + writeJSON(w, http.StatusOK, map[string]any{ + "status": 200, + "code": 0, + "data": map[string]any{"list": []map[string]any{{ + "fid": "dir-target", "file_name": "A&B", "file": false, + }}}, + "metadata": map[string]any{ + "_size": 100, "_page": 2, "_count": 1, "_total": 0, + }, + }) + default: + t.Fatalf("unexpected page: %s", r.URL.Query().Get("_page")) + } + })) + defer srv.Close() + + d := newTestDriver(srv.URL) + obj, err := d.findDir(context.Background(), "parent", "A&B") + if err != nil { + t.Fatalf("findDir: %v", err) + } + if obj == nil || obj.GetID() != "dir-target" || obj.GetName() != "A&B" || !obj.IsDir() { + t.Fatalf("findDir object = %#v, want decoded directory dir-target/A&B", obj) + } + if listCalls != 2 { + t.Fatalf("list calls = %d, want 2", listCalls) + } +} + +func writeDirectoryList(w http.ResponseWriter, fid, name string) { + writeJSON(w, http.StatusOK, map[string]any{ + "status": 200, + "code": 0, + "data": map[string]any{"list": []map[string]any{{ + "fid": fid, "file_name": name, "file": false, + }}}, + "metadata": map[string]any{ + "_size": 100, "_page": 1, "_count": 1, "_total": 1, + }, + }) +}