Skip to content

Commit

Permalink
lib/backup/common: consistently use canonical path with / directory s…
Browse files Browse the repository at this point in the history
…eparators at Part.Path

Previously Part.Path could contain `\` directory separators on Windows OS,
which could result in incorrect filepaths generation when making backups at object storage.

Updates #4704

This is a follow-up for f2df8ad
  • Loading branch information
valyala committed Sep 18, 2023
1 parent 9e50ea6 commit c0b8143
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 64 deletions.
23 changes: 23 additions & 0 deletions lib/backup/common/part.go
Expand Up @@ -2,6 +2,7 @@ package common

import (
"fmt"
"path/filepath"
"regexp"
"sort"
"strconv"
Expand All @@ -15,6 +16,9 @@ import (
// Each source file can be split into parts with up to MaxPartSize sizes.
type Part struct {
// Path is the path to file for backup.
//
// Path must consistently use `/` as directory separator.
// Use ToCanonicalPath() function for converting local directory separators to `/`.
Path string

// FileSize is the size of the whole file for the given part.
Expand Down Expand Up @@ -51,11 +55,30 @@ func (p *Part) RemotePath(prefix string) string {
return fmt.Sprintf("%s/%s/%016X_%016X_%016X", prefix, p.Path, p.FileSize, p.Offset, p.Size)
}

// LocalPath returns local path for p at the given dir.
func (p *Part) LocalPath(dir string) string {
path := p.Path
if filepath.Separator != '/' {
path = strings.ReplaceAll(path, "/", string(filepath.Separator))
}
return filepath.Join(dir, path)
}

// ToCanonicalPath returns canonical path by replacing local directory separators with `/`.
func ToCanonicalPath(path string) string {
if filepath.Separator == '/' {
return path
}
return strings.ReplaceAll(path, string(filepath.Separator), "/")
}

var partNameRegexp = regexp.MustCompile(`^(.+)[/\\]([0-9A-F]{16})_([0-9A-F]{16})_([0-9A-F]{16})$`)

// ParseFromRemotePath parses p from remotePath.
//
// Returns true on success.
//
// remotePath must be in canonical form received from ToCanonicalPath().
func (p *Part) ParseFromRemotePath(remotePath string) bool {
tmp := partNameRegexp.FindStringSubmatch(remotePath)
if len(tmp) != 5 {
Expand Down
5 changes: 3 additions & 2 deletions lib/backup/fscommon/fscommon.go
Expand Up @@ -10,9 +10,10 @@ import (
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
)

// AppendFiles appends all the files from dir to dst.
// AppendFiles appends paths to all the files from local dir to dst.
//
// All the appended files will have dir prefix.
// All the appended filepaths will have dir prefix.
// The returned paths have local OS-specific directory separators.
func AppendFiles(dst []string, dir string) ([]string, error) {
d, err := os.Open(dir)
if err != nil {
Expand Down
9 changes: 5 additions & 4 deletions lib/backup/fslocal/fslocal.go
Expand Up @@ -77,7 +77,7 @@ func (fs *FS) ListParts() ([]common.Part, error) {
if err != nil {
return nil, fmt.Errorf("cannot stat %q: %w", file, err)
}
path := file[len(dir):]
path := common.ToCanonicalPath(file[len(dir):])
size := uint64(fi.Size())
if size == 0 {
parts = append(parts, common.Part{
Expand Down Expand Up @@ -146,8 +146,9 @@ func (fs *FS) NewWriteCloser(p common.Part) (io.WriteCloser, error) {
return blwc, nil
}

// DeletePath deletes the given path from fs and returns the size
// for the deleted file.
// DeletePath deletes the given path from fs and returns the size for the deleted file.
//
// The path must be in canonical form, e.g. it must have `/` directory separators
func (fs *FS) DeletePath(path string) (uint64, error) {
p := common.Part{
Path: path,
Expand Down Expand Up @@ -187,7 +188,7 @@ func (fs *FS) mkdirAll(filePath string) error {
}

func (fs *FS) path(p common.Part) string {
return filepath.Join(fs.Dir, p.Path)
return p.LocalPath(fs.Dir)
}

type limitedReadCloser struct {
Expand Down
11 changes: 3 additions & 8 deletions lib/backup/fsremote/fsremote.go
Expand Up @@ -58,7 +58,8 @@ func (fs *FS) ListParts() ([]common.Part, error) {
continue
}
var p common.Part
if !p.ParseFromRemotePath(file[len(dir):]) {
remotePath := common.ToCanonicalPath(file[len(dir):])
if !p.ParseFromRemotePath(remotePath) {
logger.Infof("skipping unknown file %s", file)
continue
}
Expand All @@ -68,15 +69,13 @@ func (fs *FS) ListParts() ([]common.Part, error) {
return nil, fmt.Errorf("cannot stat file %q for part %q: %w", file, p.Path, err)
}
p.ActualSize = uint64(fi.Size())
p.Path = pathToCanonical(p.Path)
parts = append(parts, p)
}
return parts, nil
}

// DeletePart deletes the given part p from fs.
func (fs *FS) DeletePart(p common.Part) error {
p.Path = canonicalPathToLocal(p.Path)
path := fs.path(p)
if err := os.Remove(path); err != nil {
return fmt.Errorf("cannot remove %q: %w", path, err)
Expand All @@ -97,7 +96,6 @@ func (fs *FS) CopyPart(srcFS common.OriginFS, p common.Part) error {
if !ok {
return fmt.Errorf("cannot perform server-side copying from %s to %s: both of them must be fsremote", srcFS, fs)
}
p.Path = canonicalPathToLocal(p.Path)
srcPath := src.path(p)
dstPath := fs.path(p)
if err := fs.mkdirAll(dstPath); err != nil {
Expand Down Expand Up @@ -142,7 +140,6 @@ func (fs *FS) CopyPart(srcFS common.OriginFS, p common.Part) error {

// DownloadPart download part p from fs to w.
func (fs *FS) DownloadPart(p common.Part, w io.Writer) error {
p.Path = canonicalPathToLocal(p.Path)
path := fs.path(p)
r, err := os.Open(path)
if err != nil {
Expand Down Expand Up @@ -198,14 +195,13 @@ func (fs *FS) mkdirAll(filePath string) error {
}

func (fs *FS) path(p common.Part) string {
return filepath.Join(fs.Dir, p.Path, fmt.Sprintf("%016X_%016X_%016X", p.FileSize, p.Offset, p.Size))
return filepath.Join(p.LocalPath(fs.Dir), fmt.Sprintf("%016X_%016X_%016X", p.FileSize, p.Offset, p.Size))
}

// DeleteFile deletes filePath at fs.
//
// The function does nothing if the filePath doesn't exist.
func (fs *FS) DeleteFile(filePath string) error {
filePath = canonicalPathToLocal(filePath)
path := filepath.Join(fs.Dir, filePath)
err := os.Remove(path)
if err != nil && !os.IsNotExist(err) {
Expand Down Expand Up @@ -247,6 +243,5 @@ func (fs *FS) HasFile(filePath string) (bool, error) {
// ReadFile returns the content of filePath at fs.
func (fs *FS) ReadFile(filePath string) ([]byte, error) {
path := filepath.Join(fs.Dir, filePath)

return os.ReadFile(path)
}
12 changes: 0 additions & 12 deletions lib/backup/fsremote/normilize_path_bsd.go

This file was deleted.

9 changes: 0 additions & 9 deletions lib/backup/fsremote/normilize_path_darwin.go

This file was deleted.

9 changes: 0 additions & 9 deletions lib/backup/fsremote/normilize_path_linux.go

This file was deleted.

9 changes: 0 additions & 9 deletions lib/backup/fsremote/normilize_path_solaris.go

This file was deleted.

11 changes: 0 additions & 11 deletions lib/backup/fsremote/normilize_path_windows.go

This file was deleted.

0 comments on commit c0b8143

Please sign in to comment.