diff --git a/internal/dataprovider/share.go b/internal/dataprovider/share.go index 75ba26aa..1a0a6bf0 100644 --- a/internal/dataprovider/share.go +++ b/internal/dataprovider/share.go @@ -18,6 +18,7 @@ import ( "encoding/json" "fmt" "net" + "slices" "strconv" "strings" "time" @@ -101,16 +102,13 @@ func (s *Share) GetSignature() string { } func (s *Share) getACopy() Share { - allowFrom := make([]string, len(s.AllowFrom)) - copy(allowFrom, s.AllowFrom) - return Share{ ID: s.ID, ShareID: s.ShareID, Name: s.Name, Description: s.Description, Scope: s.Scope, - Paths: s.Paths, + Paths: slices.Clone(s.Paths), Username: s.Username, CreatedAt: s.CreatedAt, UpdatedAt: s.UpdatedAt, @@ -119,7 +117,7 @@ func (s *Share) getACopy() Share { Password: s.Password, MaxTokens: s.MaxTokens, UsedTokens: s.UsedTokens, - AllowFrom: allowFrom, + AllowFrom: slices.Clone(s.AllowFrom), } } diff --git a/internal/httpd/api_shares.go b/internal/httpd/api_shares.go index 77ffb869..c8541f8f 100644 --- a/internal/httpd/api_shares.go +++ b/internal/httpd/api_shares.go @@ -320,12 +320,14 @@ func (s *httpdServer) downloadFromShare(w http.ResponseWriter, r *http.Request) return } baseDir := "/" + files := share.Paths if info != nil && info.IsDir() { + // zip entry names are relative to the shared directory baseDir = share.Paths[0] - share.Paths[0] = "/" + files = []string{"/"} } w.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=\"share-%v.zip\"", share.Name)) - renderCompressedFiles(w, connection, baseDir, share.Paths, &share) + renderCompressedFiles(w, connection, baseDir, files, &share) return } if status, err := downloadFile(w, r, connection, share.Paths[0], info, false, &share); err != nil { diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 9451acc7..6c9b7884 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -15,6 +15,7 @@ package httpd_test import ( + "archive/zip" "bytes" "crypto/rand" "encoding/base64" @@ -15308,6 +15309,95 @@ func TestShareUsage(t *testing.T) { executeRequest(req) } +func TestShareDirDownloadPreservesPaths(t *testing.T) { + u := getTestUser() + u.Username = "share_zip_paths_" + xid.New().String() + user, _, err := httpdtest.AddUser(u, http.StatusCreated) + assert.NoError(t, err) + + sharedDir := "shared" + sharedFileName := "visible.dat" + outsideFileName := "outside.dat" + err = os.MkdirAll(filepath.Join(user.GetHomeDir(), sharedDir), os.ModePerm) + assert.NoError(t, err) + err = createTestFile(filepath.Join(user.GetHomeDir(), sharedDir, sharedFileName), 32768) + assert.NoError(t, err) + err = createTestFile(filepath.Join(user.GetHomeDir(), outsideFileName), 32768) + assert.NoError(t, err) + + token, err := getJWTAPIUserTokenFromTestServer(user.Username, defaultPassword) + assert.NoError(t, err) + + share := dataprovider.Share{ + Name: "test_share_zip_paths", + Scope: dataprovider.ShareScopeRead, + Paths: []string{"/" + sharedDir}, + } + asJSON, err := json.Marshal(share) + assert.NoError(t, err) + req, err := http.NewRequest(http.MethodPost, userSharesPath, bytes.NewBuffer(asJSON)) + assert.NoError(t, err) + setBearerForReq(req, token) + rr := executeRequest(req) + checkResponseCode(t, http.StatusCreated, rr) + objectID := rr.Header().Get("X-Object-ID") + assert.NotEmpty(t, objectID) + + getZipEntries := func() []string { + r, errReq := http.NewRequest(http.MethodGet, sharesPath+"/"+objectID, nil) + assert.NoError(t, errReq) + resp := executeRequest(r) + checkResponseCode(t, http.StatusOK, resp) + zipReader, errZip := zip.NewReader(bytes.NewReader(resp.Body.Bytes()), int64(resp.Body.Len())) + assert.NoError(t, errZip) + var entries []string + for _, f := range zipReader.File { + entries = append(entries, f.Name) + } + return entries + } + listShare := func() []string { + r, errReq := http.NewRequest(http.MethodGet, sharesPath+"/"+objectID+"/dirs", nil) + assert.NoError(t, errReq) + resp := executeRequest(r) + checkResponseCode(t, http.StatusOK, resp) + var contents []map[string]any + err := json.Unmarshal(resp.Body.Bytes(), &contents) + assert.NoError(t, err) + var names []string + for _, c := range contents { + names = append(names, c["name"].(string)) + } + return names + } + // zip entry names are relative to the shared directory + assert.Equal(t, []string{"/", sharedFileName}, getZipEntries()) + // downloading the whole share must not expand the shared paths + req, err = http.NewRequest(http.MethodGet, path.Join(userSharesPath, objectID), nil) + assert.NoError(t, err) + setBearerForReq(req, token) + rr = executeRequest(req) + checkResponseCode(t, http.StatusOK, rr) + var savedShare dataprovider.Share + err = json.Unmarshal(rr.Body.Bytes(), &savedShare) + assert.NoError(t, err) + assert.Equal(t, []string{"/" + sharedDir}, savedShare.Paths) + // browsing the share still returns the shared directory contents + assert.Equal(t, []string{sharedFileName}, listShare()) + // files outside the shared directory remain unreachable + req, err = http.NewRequest(http.MethodGet, sharesPath+"/"+objectID+"/files?path=/"+outsideFileName, nil) + assert.NoError(t, err) + rr = executeRequest(req) + checkResponseCode(t, http.StatusNotFound, rr) + // a new download returns the same archive + assert.Equal(t, []string{"/", sharedFileName}, getZipEntries()) + + _, err = httpdtest.RemoveUser(user, http.StatusOK) + assert.NoError(t, err) + err = os.RemoveAll(user.GetHomeDir()) + assert.NoError(t, err) +} + func TestSharePasswordPolicy(t *testing.T) { g := getTestGroup() g.UserSettings.Filters.PasswordStrength = 70