Rename duplicate filenames aren't enforced
Closed this issue · 5 comments
Due to how Upload::load
works in framework
, it checks for duplicate filenames via:
while(file_exists("$base/$relativeFilePath") {
$i = isset($i) ? ($i+1) : 2;
$oldFilePath = $relativeFilePath;
$prefix = $this->config()->version_prefix;
$pattern = '/' . preg_quote($prefix) . '([0-9]+$)/';
if(preg_match($pattern, $fileTitle, $matches)) {
$fileTitle = preg_replace($pattern, $prefix . ($matches[1] + 1), $fileTitle);
} else {
$fileTitle .= $prefix . $i;
}
$relativeFilePath = $relativeFolderPath . $fileTitle . $fileSuffix;
if($oldFilePath == $relativeFilePath && $i > 2) {
user_error("Couldn't fix $relativeFilePath with $i tries", E_USER_ERROR);
}
}
My proposed (and tested) fix would be to patch framework with:
while(file_exists("$base/$relativeFilePath") || File::get()->find('Filename', $relativeFilePath)) {
// ...
}
This is only brought up because a client explicitly wants to disallow duplicate filenames for S3/CDNContent.
Any performance considerations here to be wary of?
@nyeholt Opinions?
@silbinarywolf Is there a way to do that check in the module beforeWrite or something? Otherwise, happy to put this in place.
@nyeholt I'd say we could potentially put it in the validate
function. of CdnFile extension.
However, File::validate
only exists in 3.2+ from what I've been told, so I think I proposed the patch as that would work regardless.
I'm okay with only supporting 3.2 and up with this fix if that means no framework patches - it can exist for those needing support for < 3.1
This is now in.