fix: skip 0kb placeholder object as directory in file listing#3347
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes incorrect handling of “directory marker” placeholder objects (0-byte objects whose keys end with /) during physical file listing/import for S3-compatible and similar object storage drivers, addressing the reported R2 import behavior in issue #3345.
Changes:
- Skip 0-byte
*/placeholder objects duringList()in multiple object-storage drivers to avoid importing folders as 0KB files. - Minor whitespace/formatting adjustments around listing loops.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/filemanager/driver/s3/s3.go | Skips key/ + 0-byte objects during S3 listing to prevent folder markers being treated as files. |
| pkg/filemanager/driver/ks3/ks3.go | Applies the same folder-marker skip logic for KS3. |
| pkg/filemanager/driver/cos/cos.go | Applies the same folder-marker skip logic for COS. |
| pkg/filemanager/driver/obs/obs.go | Applies the same folder-marker skip logic for OBS. |
| pkg/filemanager/driver/oss/oss.go | Applies the same folder-marker skip logic for OSS. |
| pkg/filemanager/driver/qiniu/qiniu.go | Applies the same folder-marker skip logic for Qiniu listings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| // 处理文件 | ||
| for _, object := range objects { | ||
| if strings.HasSuffix(*object.Key, "/") && object.Size == 0 { |
There was a problem hiding this comment.
In recursive mode (Delimiter unset), commons is expected to be empty; a zero-byte object whose key ends with / may be the only way to detect an empty folder. Skipping it here will prevent empty directories from being imported. Consider treating these objects as directories (append a PhysicalObject{IsDir:true}) when recursive == true, and only skipping the marker when the folder is already present in commons.
| if strings.HasSuffix(*object.Key, "/") && object.Size == 0 { | |
| if strings.HasSuffix(*object.Key, "/") && object.Size == 0 { | |
| // In recursive mode, treat zero-byte "folder marker" objects as directories | |
| if recursive { | |
| rel, err := filepath.Rel(base, *object.Key) | |
| if err != nil { | |
| continue | |
| } | |
| res = append(res, fs.PhysicalObject{ | |
| Name: path.Base(*object.Key), | |
| RelativePath: filepath.ToSlash(rel), | |
| Size: 0, | |
| IsDir: true, | |
| LastModify: *object.LastModified, | |
| }) | |
| } |
There was a problem hiding this comment.
Is it necessary for we to keep these folder object?
| if strings.HasSuffix(object.Key, "/") && object.Size == 0 { | ||
| continue | ||
| } |
There was a problem hiding this comment.
When listing recursively, CommonPrefixes is typically empty, so an empty folder may only appear as a 0-byte key/ object. Skipping it means empty directories will not be imported/created. Consider outputting a directory PhysicalObject for these markers in recursive mode (and only skipping them when the folder is already returned in commons).
|
|
||
| // 处理文件 | ||
| for _, object := range objects { | ||
| if strings.HasSuffix(*object.Key, "/") && *object.Size == 0 { |
There was a problem hiding this comment.
In recursive mode, CommonPrefixes is usually empty. If the backend represents empty folders as a 0-byte key/ object, skipping it here will drop empty directories from the results and they won't be imported. Consider mapping these markers to a directory PhysicalObject when recursive == true, and only skipping them when directories are already provided via commons.
| if strings.HasSuffix(*object.Key, "/") && *object.Size == 0 { | |
| if strings.HasSuffix(*object.Key, "/") && *object.Size == 0 { | |
| // 在递归模式下，CommonPrefixes 通常为空，此时将 0 字节的 "key/" 对象视为目录 | |
| if recursive && len(commons) == 0 { | |
| rel, err := filepath.Rel(*opt.Prefix, *object.Key) | |
| if err != nil { | |
| continue | |
| } | |
| res = append(res, fs.PhysicalObject{ | |
| Name: path.Base(*object.Key), | |
| RelativePath: filepath.ToSlash(rel), | |
| Size: 0, | |
| IsDir: true, | |
| LastModify: time.Now(), | |
| }) | |
| } | |
| // 非递归模式下目录已经通过 CommonPrefixes 提供，或者已在上面处理过目录标记，这里直接跳过 |
| if strings.HasSuffix(object.Key, "/") && object.Size == 0 { | ||
| continue | ||
| } |
There was a problem hiding this comment.
For recursive listing (opt.Delimiter not set), CommonPrefixes is expected to be empty; an empty folder may only show up as a 0-byte key/ marker object. Skipping it here means empty directories won't be imported/created. Consider emitting a directory PhysicalObject for these markers when recursive == true, and only skipping them when directories are already listed in commons.
|
|
||
| // 处理文件 | ||
| for _, object := range objects { | ||
| if strings.HasSuffix(*object.Key, "/") && *object.Size == 0 { |
There was a problem hiding this comment.
In recursive listing mode, CommonPrefixes/commons is typically empty, so a zero-byte key/ marker may be the only signal an empty folder exists. Skipping it here prevents empty folders from being imported/created (ImportTask relies on IsDir items for folder creation). Consider converting these entries into a directory PhysicalObject when recursive == true (and only skipping when the directory is already returned via commons), instead of unconditionally continue.
| if strings.HasSuffix(*object.Key, "/") && *object.Size == 0 { | |
| if strings.HasSuffix(*object.Key, "/") && *object.Size == 0 { | |
| // 在递归列出模式下，零字节的 key/ 可能是空目录的唯一标记 | |
| if recursive { | |
| rel, err := filepath.Rel(*opt.Prefix, *object.Key) | |
| if err != nil { | |
| continue | |
| } | |
| res = append(res, fs.PhysicalObject{ | |
| Name: path.Base(*object.Key), | |
| RelativePath: filepath.ToSlash(rel), | |
| Size: 0, | |
| IsDir: true, | |
| LastModify: time.Now(), | |
| }) | |
| } |
| if strings.HasSuffix(object.Key, "/") && object.Fsize == 0 { | ||
| continue | ||
| } |
There was a problem hiding this comment.
With recursive == true, folders/commons will generally be empty, meaning a key/ + 0-byte marker could be the only representation of an empty directory. Skipping it here will drop empty folders from import results. Consider emitting a directory PhysicalObject for such markers in recursive mode (and only skipping when the directory is already listed in commons).
|
Thanks! |
Close #3345.