symbiote/silverstripe-cdncontent

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?

@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.