-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
🐛 bug: align upload root config #3934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
441aa8b
b550622
c4813fa
8c2d0a2
3fa3d3d
42791c6
65ba9c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import ( | |
| "errors" | ||
| "fmt" | ||
| "io" | ||
| "io/fs" | ||
| "net" | ||
| "net/http" | ||
| "net/http/httputil" | ||
|
|
@@ -90,6 +91,9 @@ type App struct { | |
| mountFields *mountFields | ||
| // state management | ||
| state *State | ||
| // upload root cache | ||
| uploadRoot string | ||
| uploadRootErr error | ||
| // Route stack divided by HTTP methods | ||
| stack [][]*Route | ||
| // customConstraints is a list of external constraints | ||
|
|
@@ -99,7 +103,8 @@ type App struct { | |
| // custom binders | ||
| customBinders []CustomBinder | ||
| // Route stack divided by HTTP methods and route prefixes | ||
| treeStack []map[int][]*Route | ||
| treeStack []map[int][]*Route | ||
| uploadRootOnce sync.Once | ||
| // sendfilesMutex is a mutex used for sendfile operations | ||
| sendfilesMutex sync.RWMutex | ||
| mutex sync.Mutex | ||
|
|
@@ -159,6 +164,22 @@ type Config struct { //nolint:govet // Aligning the struct fields is not necessa | |
| // Default: 4 * 1024 * 1024 | ||
| BodyLimit int `json:"body_limit"` | ||
|
|
||
| // RootDir defines the directory where files can be persisted using SaveFile and SaveFileToStorage. | ||
| // The path must be relative to the current working directory or an absolute path on the host system. | ||
| // | ||
| // Default: "." | ||
| RootDir string `json:"root_dir"` | ||
|
|
||
| // RootFS provides an fs.FS implementation rooted at RootDir used to validate upload targets. | ||
| // | ||
| // Default: os.DirFS(RootDir) | ||
| RootFS fs.FS `json:"-"` | ||
|
Comment on lines
+173
to
+176
|
||
|
|
||
| // RootPerms are the permissions applied when creating RootDir. | ||
| // | ||
| // Default: 0o750 | ||
| RootPerms fs.FileMode `json:"root_perms"` | ||
|
|
||
| // Maximum number of concurrent connections. | ||
| // | ||
| // Default: 256 * 1024 | ||
|
|
@@ -562,6 +583,12 @@ func New(config ...Config) *App { | |
| if app.config.BodyLimit <= 0 { | ||
| app.config.BodyLimit = DefaultBodyLimit | ||
| } | ||
| if app.config.RootDir == "" { | ||
| app.config.RootDir = "." | ||
| } | ||
| if app.config.RootPerms == 0 { | ||
| app.config.RootPerms = 0o750 | ||
| } | ||
| if app.config.Concurrency <= 0 { | ||
| app.config.Concurrency = DefaultConcurrency | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,10 +7,16 @@ package fiber | |||||||||||||||
| import ( | ||||||||||||||||
| "context" | ||||||||||||||||
| "crypto/tls" | ||||||||||||||||
| "errors" | ||||||||||||||||
| "fmt" | ||||||||||||||||
| "io" | ||||||||||||||||
| "io/fs" | ||||||||||||||||
| "maps" | ||||||||||||||||
| "mime/multipart" | ||||||||||||||||
| "os" | ||||||||||||||||
| pathpkg "path" | ||||||||||||||||
| "path/filepath" | ||||||||||||||||
| "slices" | ||||||||||||||||
| "strconv" | ||||||||||||||||
| "strings" | ||||||||||||||||
| "time" | ||||||||||||||||
|
|
@@ -453,12 +459,26 @@ func (c *DefaultCtx) IsPreflight() bool { | |||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // SaveFile saves any multipart file to disk. | ||||||||||||||||
| func (*DefaultCtx) SaveFile(fileheader *multipart.FileHeader, path string) error { | ||||||||||||||||
| return fasthttp.SaveMultipartFile(fileheader, path) | ||||||||||||||||
| func (c *DefaultCtx) SaveFile(fileheader *multipart.FileHeader, path string) error { | ||||||||||||||||
| _, absolutePath, err := resolveUploadPath(c.app, path) | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| return err | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| if err := os.MkdirAll(filepath.Dir(absolutePath), 0o755); err != nil { //nolint:gosec // upload subdirectories need shared execute permissions for reads | ||||||||||||||||
| return fmt.Errorf("failed to prepare upload path: %w", err) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| return fasthttp.SaveMultipartFile(fileheader, absolutePath) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // SaveFileToStorage saves any multipart file to an external storage system. | ||||||||||||||||
| func (c *DefaultCtx) SaveFileToStorage(fileheader *multipart.FileHeader, path string, storage Storage) error { | ||||||||||||||||
| safePath, _, err := resolveUploadPath(c.app, path) | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| return err | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| file, err := fileheader.Open() | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| return fmt.Errorf("failed to open: %w", err) | ||||||||||||||||
|
|
@@ -488,13 +508,195 @@ func (c *DefaultCtx) SaveFileToStorage(fileheader *multipart.FileHeader, path st | |||||||||||||||
|
|
||||||||||||||||
| data := append([]byte(nil), buf.Bytes()...) | ||||||||||||||||
|
|
||||||||||||||||
| if err := storage.SetWithContext(c.Context(), path, data, 0); err != nil { | ||||||||||||||||
| if err := storage.SetWithContext(c.Context(), safePath, data, 0); err != nil { | ||||||||||||||||
| return fmt.Errorf("failed to store: %w", err) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| return nil | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| //nolint:nonamedreturns // names clarify path handling through normalization and validation | ||||||||||||||||
| func resolveUploadPath(app *App, path string) (normalizedPath, absolutePath string, err error) { | ||||||||||||||||
| if app == nil { | ||||||||||||||||
| return "", "", fmt.Errorf("invalid upload root: %w", errors.New("app is nil")) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| uploadRoot, err := getRootDir(app) | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| return "", "", err | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| uploadFS := app.config.RootFS | ||||||||||||||||
| if uploadFS == nil { | ||||||||||||||||
| uploadFS = os.DirFS(uploadRoot) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| normalizedPath, err = sanitizeUploadPath(path, uploadFS) | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| return "", "", err | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| relativePath := filepath.FromSlash(normalizedPath) | ||||||||||||||||
| absolutePath = filepath.Join(uploadRoot, relativePath) | ||||||||||||||||
| if !isWithinRoot(uploadRoot, absolutePath) { | ||||||||||||||||
| return "", "", errUploadOutsideRoot | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| return normalizedPath, absolutePath, nil | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| func getRootDir(app *App) (string, error) { | ||||||||||||||||
| if app == nil { | ||||||||||||||||
| return "", fmt.Errorf("invalid upload root: %w", errors.New("app is nil")) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| app.uploadRootOnce.Do(func() { | ||||||||||||||||
| root := app.config.RootDir | ||||||||||||||||
| if root == "" { | ||||||||||||||||
| root = "." | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| perms := app.config.RootPerms | ||||||||||||||||
| if perms == 0 { | ||||||||||||||||
| perms = 0o750 | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| absoluteRoot, err := filepath.Abs(root) | ||||||||||||||||
| if err != nil { | ||||||||||||||||
| app.uploadRootErr = fmt.Errorf("invalid upload root: %w", err) | ||||||||||||||||
| return | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| if err = os.MkdirAll(absoluteRoot, perms); err != nil { | ||||||||||||||||
| app.uploadRootErr = fmt.Errorf("invalid upload root: %w", err) | ||||||||||||||||
| return | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| resolvedRoot, err := filepath.EvalSymlinks(absoluteRoot) | ||||||||||||||||
| if err == nil { | ||||||||||||||||
| absoluteRoot = resolvedRoot | ||||||||||||||||
| } else if !errors.Is(err, fs.ErrNotExist) { | ||||||||||||||||
| app.uploadRootErr = fmt.Errorf("invalid upload root: %w", err) | ||||||||||||||||
| return | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
|
||||||||||||||||
| // Ensure the resolved directory has the intended permissions | |
| if err := os.Chmod(absoluteRoot, perms); err != nil { | |
| app.uploadRootErr = fmt.Errorf("invalid upload root: %w", err) | |
| return | |
| } |
gaby marked this conversation as resolved.
Show resolved
Hide resolved
gaby marked this conversation as resolved.
Show resolved
Hide resolved
gaby marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rejectSymlinkTraversal function performs a directory listing for each path segment, which can be inefficient for deeply nested upload paths. Consider using fs.Stat instead of fs.ReadDir to check individual path components, as it would be more efficient and still allow symlink detection through the file info.
Copilot
AI
Dec 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rejectSymlinkTraversal function performs directory traversal on every upload, which can cause significant performance degradation especially for deeply nested paths or directories with many entries. Each directory is read fully using fs.ReadDir, and this happens for every path component. For frequent uploads, this overhead can be substantial. Consider caching directory metadata or implementing a more efficient validation approach.
Uh oh!
There was an error while loading. Please reload this page.