lib/backup/common: consistently use canonical path with / directory separators 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 https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4704

This is a follow-up for f2df8ad480
This commit is contained in:
Aliaksandr Valialkin 2023-09-18 16:10:16 +02:00
parent f267733d9a
commit f93a7b8457
No known key found for this signature in database
GPG key ID: A72BEC6CD3D0DED1
9 changed files with 34 additions and 64 deletions

View file

@ -2,6 +2,7 @@ package common
import ( import (
"fmt" "fmt"
"path/filepath"
"regexp" "regexp"
"sort" "sort"
"strconv" "strconv"
@ -15,6 +16,9 @@ import (
// Each source file can be split into parts with up to MaxPartSize sizes. // Each source file can be split into parts with up to MaxPartSize sizes.
type Part struct { type Part struct {
// Path is the path to file for backup. // 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 Path string
// FileSize is the size of the whole file for the given part. // FileSize is the size of the whole file for the given part.
@ -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) 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})$`) var partNameRegexp = regexp.MustCompile(`^(.+)[/\\]([0-9A-F]{16})_([0-9A-F]{16})_([0-9A-F]{16})$`)
// ParseFromRemotePath parses p from remotePath. // ParseFromRemotePath parses p from remotePath.
// //
// Returns true on success. // Returns true on success.
//
// remotePath must be in canonical form received from ToCanonicalPath().
func (p *Part) ParseFromRemotePath(remotePath string) bool { func (p *Part) ParseFromRemotePath(remotePath string) bool {
tmp := partNameRegexp.FindStringSubmatch(remotePath) tmp := partNameRegexp.FindStringSubmatch(remotePath)
if len(tmp) != 5 { if len(tmp) != 5 {

View file

@ -10,9 +10,10 @@ import (
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" "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) { func AppendFiles(dst []string, dir string) ([]string, error) {
d, err := os.Open(dir) d, err := os.Open(dir)
if err != nil { if err != nil {

View file

@ -77,7 +77,7 @@ func (fs *FS) ListParts() ([]common.Part, error) {
if err != nil { if err != nil {
return nil, fmt.Errorf("cannot stat %q: %w", file, err) return nil, fmt.Errorf("cannot stat %q: %w", file, err)
} }
path := file[len(dir):] path := common.ToCanonicalPath(file[len(dir):])
size := uint64(fi.Size()) size := uint64(fi.Size())
if size == 0 { if size == 0 {
parts = append(parts, common.Part{ parts = append(parts, common.Part{
@ -146,8 +146,9 @@ func (fs *FS) NewWriteCloser(p common.Part) (io.WriteCloser, error) {
return blwc, nil return blwc, nil
} }
// DeletePath deletes the given path from fs and returns the size // DeletePath deletes the given path from fs and returns the size for the deleted file.
// 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) { func (fs *FS) DeletePath(path string) (uint64, error) {
p := common.Part{ p := common.Part{
Path: path, Path: path,
@ -187,7 +188,7 @@ func (fs *FS) mkdirAll(filePath string) error {
} }
func (fs *FS) path(p common.Part) string { func (fs *FS) path(p common.Part) string {
return filepath.Join(fs.Dir, p.Path) return p.LocalPath(fs.Dir)
} }
type limitedReadCloser struct { type limitedReadCloser struct {

View file

@ -58,7 +58,8 @@ func (fs *FS) ListParts() ([]common.Part, error) {
continue continue
} }
var p common.Part 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) logger.Infof("skipping unknown file %s", file)
continue continue
} }
@ -68,7 +69,6 @@ func (fs *FS) ListParts() ([]common.Part, error) {
return nil, fmt.Errorf("cannot stat file %q for part %q: %w", file, p.Path, err) return nil, fmt.Errorf("cannot stat file %q for part %q: %w", file, p.Path, err)
} }
p.ActualSize = uint64(fi.Size()) p.ActualSize = uint64(fi.Size())
p.Path = pathToCanonical(p.Path)
parts = append(parts, p) parts = append(parts, p)
} }
return parts, nil return parts, nil
@ -76,7 +76,6 @@ func (fs *FS) ListParts() ([]common.Part, error) {
// DeletePart deletes the given part p from fs. // DeletePart deletes the given part p from fs.
func (fs *FS) DeletePart(p common.Part) error { func (fs *FS) DeletePart(p common.Part) error {
p.Path = canonicalPathToLocal(p.Path)
path := fs.path(p) path := fs.path(p)
if err := os.Remove(path); err != nil { if err := os.Remove(path); err != nil {
return fmt.Errorf("cannot remove %q: %w", path, err) return fmt.Errorf("cannot remove %q: %w", path, err)
@ -97,7 +96,6 @@ func (fs *FS) CopyPart(srcFS common.OriginFS, p common.Part) error {
if !ok { if !ok {
return fmt.Errorf("cannot perform server-side copying from %s to %s: both of them must be fsremote", srcFS, fs) 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) srcPath := src.path(p)
dstPath := fs.path(p) dstPath := fs.path(p)
if err := fs.mkdirAll(dstPath); err != nil { if err := fs.mkdirAll(dstPath); err != nil {
@ -142,7 +140,6 @@ func (fs *FS) CopyPart(srcFS common.OriginFS, p common.Part) error {
// DownloadPart download part p from fs to w. // DownloadPart download part p from fs to w.
func (fs *FS) DownloadPart(p common.Part, w io.Writer) error { func (fs *FS) DownloadPart(p common.Part, w io.Writer) error {
p.Path = canonicalPathToLocal(p.Path)
path := fs.path(p) path := fs.path(p)
r, err := os.Open(path) r, err := os.Open(path)
if err != nil { if err != nil {
@ -198,14 +195,13 @@ func (fs *FS) mkdirAll(filePath string) error {
} }
func (fs *FS) path(p common.Part) string { 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. // DeleteFile deletes filePath at fs.
// //
// The function does nothing if the filePath doesn't exist. // The function does nothing if the filePath doesn't exist.
func (fs *FS) DeleteFile(filePath string) error { func (fs *FS) DeleteFile(filePath string) error {
filePath = canonicalPathToLocal(filePath)
path := filepath.Join(fs.Dir, filePath) path := filepath.Join(fs.Dir, filePath)
err := os.Remove(path) err := os.Remove(path)
if err != nil && !os.IsNotExist(err) { if err != nil && !os.IsNotExist(err) {
@ -247,6 +243,5 @@ func (fs *FS) HasFile(filePath string) (bool, error) {
// ReadFile returns the content of filePath at fs. // ReadFile returns the content of filePath at fs.
func (fs *FS) ReadFile(filePath string) ([]byte, error) { func (fs *FS) ReadFile(filePath string) ([]byte, error) {
path := filepath.Join(fs.Dir, filePath) path := filepath.Join(fs.Dir, filePath)
return os.ReadFile(path) return os.ReadFile(path)
} }

View file

@ -1,12 +0,0 @@
//go:build freebsd || openbsd || dragonfly || netbsd
// +build freebsd openbsd dragonfly netbsd
package fsremote
func pathToCanonical(path string) string {
return path
}
func canonicalPathToLocal(path string) string {
return path
}

View file

@ -1,9 +0,0 @@
package fsremote
func pathToCanonical(path string) string {
return path
}
func canonicalPathToLocal(path string) string {
return path
}

View file

@ -1,9 +0,0 @@
package fsremote
func pathToCanonical(path string) string {
return path
}
func canonicalPathToLocal(path string) string {
return path
}

View file

@ -1,9 +0,0 @@
package fsremote
func pathToCanonical(path string) string {
return path
}
func canonicalPathToLocal(path string) string {
return path
}

View file

@ -1,11 +0,0 @@
package fsremote
import "strings"
func pathToCanonical(path string) string {
return strings.ReplaceAll(path, "\\", "/")
}
func canonicalPathToLocal(path string) string {
return strings.ReplaceAll(path, "/", "\\")
}