Fix/limit thumbnail size #678
Conversation
Codecov Report
@@ Coverage Diff @@
## staging #678 +/- ##
===========================================
- Coverage 19.70% 19.64% -0.07%
===========================================
Files 66 66
Lines 7530 7559 +29
===========================================
+ Hits 1484 1485 +1
- Misses 5819 5846 +27
- Partials 227 228 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| wg.Done() | ||
| }() | ||
|
|
||
| if count == 0 { |
There was a problem hiding this comment.
We can do this count checking out of the gorountine, so if it's not 0, we don't need to start the goroutine.
| if thumbHeader != nil { | ||
| if thumbHeader.Size > MaxThumbnailSize { | ||
| return common.NewError("max_thumbnail_size", | ||
| fmt.Sprintf("Thumbnail size %d should not be greater than %d", thumbHeader.Size, MaxThumbnailSize)) |
There was a problem hiding this comment.
We'd better not capitalize error messages, as they are usually printed following other context.
| for _, subDir := range subDirs { | ||
| var found bool | ||
| for _, ref := range dirRef.Children { | ||
| if ref.Name == subDir && ref.Type == DIRECTORY { |
There was a problem hiding this comment.
Perhaps we should break immediately if the dirRef.Children do not have the subDir, basically this checking all start from the root path, if any level of the path do not exist in the corresponding dirRef, there must be something wrong.
There was a problem hiding this comment.
It does break with return statement.
There was a problem hiding this comment.
@peterlimg @lpoli , this PR breaks multiple things
- breaks 204 status for partial delete
- it is incorrect to delete thumbnail if hash is not empty. Because hash always has value even it is empty.
Changes
Delete object function has been optimized by removal redundant of logic and addition of goroutine for disk operations.
Fixes
Thumbnail file size is limited to 1MB