[core] roll back database transaction when upload fails - fixes #1007

This commit is contained in:
Shish 2024-01-09 21:01:10 +00:00
parent 4d011fa5e5
commit b2f67363a1
9 changed files with 45 additions and 50 deletions

View file

@ -23,10 +23,6 @@ class ImageAdditionEvent extends Event
}
}
class ImageAdditionException extends SCoreException
{
}
/**
* An image is being deleted.
*/

View file

@ -16,6 +16,7 @@ namespace Shimmie2;
*/
function add_dir(string $base, ?array $extra_tags = []): array
{
global $database;
$results = [];
foreach (list_files($base) as $full_path) {
@ -24,6 +25,7 @@ function add_dir(string $base, ?array $extra_tags = []): array
$tags = array_merge(path_to_tags($short_path), $extra_tags);
try {
$database->execute("SAVEPOINT upload");
$dae = send_event(new DataUploadEvent($full_path, [
'filename' => pathinfo($filename, PATHINFO_BASENAME),
'tags' => $tags,
@ -32,7 +34,9 @@ function add_dir(string $base, ?array $extra_tags = []): array
foreach($dae->images as $image) {
$results[] = new UploadSuccess($filename, $image->id);
}
$database->execute("RELEASE SAVEPOINT upload");
} catch (UploadException $ex) {
$database->execute("ROLLBACK TO SAVEPOINT upload");
$results[] = new UploadError($filename, $ex->getMessage());
}
}

View file

@ -264,7 +264,7 @@ class DanbooruApi extends Extension
*/
private function api_add_post(): void
{
global $user, $page;
global $database, $user, $page;
$danboorup_kludge = 1; // danboorup for firefox makes broken links out of location: /path
// Check first if a login was supplied, if it wasn't check if the user is logged in via cookie
@ -342,6 +342,7 @@ class DanbooruApi extends Extension
//log_debug("danbooru_api", "upload($filename): fileinfo(".var_export($fileinfo,TRUE)."), metadata(".var_export($metadata,TRUE).")...");
try {
$database->execute("SAVEPOINT upload");
// Fire off an event which should process the new file and add it to the db
$dae = send_event(new DataUploadEvent($file, [
'filename' => pathinfo($filename, PATHINFO_BASENAME),
@ -363,8 +364,9 @@ class DanbooruApi extends Extension
} else {
$page->add_http_header("Location: $newid");
}
$database->execute("RELEASE SAVEPOINT upload");
} catch (UploadException $ex) {
// Did something screw up?
$database->execute("ROLLBACK TO SAVEPOINT upload");
$page->set_code(409);
$page->add_http_header("X-Danbooru-Errors: exception - " . $ex->getMessage());
}

View file

@ -33,12 +33,9 @@ class ImageBanTest extends ShimmiePHPUnitTestCase
$this->assertEquals(404, $page->code);
// Can't repost
try {
$this->assertException(UploadException::class, function () {
$this->post_image("tests/pbx_screenshot.jpg", "pbx");
$this->fail();
} catch (UploadException $e) {
$this->assertTrue(true);
}
});
// Remove ban
send_event(new RemoveImageHashBanEvent($this->hash));

View file

@ -321,7 +321,7 @@ class OuroborosAPI extends Extension
*/
protected function postCreate(OuroborosPost $post, ?string $md5 = '')
{
global $config;
global $config, $database;
$handler = $config->get_string(ImageConfig::UPLOAD_COLLISION_HANDLER);
if (!empty($md5) && !($handler == ImageConfig::COLLISION_MERGE)) {
$img = Image::by_hash($md5);
@ -383,20 +383,20 @@ class OuroborosAPI extends Extension
}
$meta['extension'] = pathinfo($meta['filename'], PATHINFO_EXTENSION);
try {
send_event(new DataUploadEvent($meta['file'], $meta));
$image = Image::by_hash($meta['hash']);
$database->execute("SAVEPOINT upload");
$dae = send_event(new DataUploadEvent($meta['file'], $meta));
$image = $dae->images[0];
if (!is_null($image)) {
$this->sendResponse(200, make_link('post/view/' . $image->id), true);
return;
} else {
// Fail, unsupported file?
$this->sendResponse(500, 'Unknown error');
return;
}
$database->execute("RELEASE SAVEPOINT upload");
} catch (UploadException $e) {
$database->execute("ROLLBACK TO SAVEPOINT upload");
// Cleanup in case shit hit the fan
$this->sendResponse(500, $e->getMessage());
return;
}
}

View file

@ -69,7 +69,7 @@ class ReplaceFile extends Extension
$target = warehouse_path(Image::IMAGE_DIR, $event->new_hash);
if (!@copy($event->tmp_filename, $target)) {
$errors = error_get_last();
throw new UploadException(
throw new ImageReplaceException(
"Failed to copy file from uploads ({$event->tmp_filename}) to archive ($target): ".
"{$errors['type']} / {$errors['message']}"
);

View file

@ -33,12 +33,10 @@ class ResolutionLimitTest extends ShimmiePHPUnitTestCase
$config->set_string("upload_ratios", "4:3 16:9");
$this->log_in_as_user();
try {
$e = $this->assertException(UploadException::class, function () {
$this->post_image("tests/pbx_screenshot.jpg", "pbx computer screenshot");
$this->fail("Invalid-size image was allowed");
} catch (UploadException $e) {
$this->assertEquals("Post too small", $e->getMessage());
}
});
$this->assertEquals("Post too small", $e->getMessage());
}
public function testResLimitLarge()
@ -50,12 +48,10 @@ class ResolutionLimitTest extends ShimmiePHPUnitTestCase
$config->set_int("upload_max_width", 100);
$config->set_string("upload_ratios", "4:3 16:9");
try {
$e = $this->assertException(UploadException::class, function () {
$this->post_image("tests/pbx_screenshot.jpg", "pbx computer screenshot");
$this->fail("Invalid-size image was allowed");
} catch (UploadException $e) {
$this->assertEquals("Post too large", $e->getMessage());
}
});
$this->assertEquals("Post too large", $e->getMessage());
}
public function testResLimitRatio()
@ -67,12 +63,10 @@ class ResolutionLimitTest extends ShimmiePHPUnitTestCase
$config->set_int("upload_max_width", -1);
$config->set_string("upload_ratios", "16:9");
try {
$e = $this->assertException(UploadException::class, function () {
$this->post_image("tests/pbx_screenshot.jpg", "pbx computer screenshot");
$this->fail("Invalid-size image was allowed");
} catch (UploadException $e) {
$this->assertEquals("Post needs to be in one of these ratios: 16:9", $e->getMessage());
}
});
$this->assertEquals("Post needs to be in one of these ratios: 16:9", $e->getMessage());
}
# reset to defaults, otherwise this can interfere with

View file

@ -315,7 +315,7 @@ class Upload extends Extension
*/
private function try_upload(array $file, array $tags, ?string $source = null): array
{
global $page, $config;
global $page, $config, $database;
// blank file boxes cause empty uploads, no need for error message
if (empty($file['name'])) {
@ -337,17 +337,17 @@ class Upload extends Extension
continue;
}
try {
$database->execute("SAVEPOINT upload");
// check if the upload was successful
if ($error !== UPLOAD_ERR_OK) {
throw new UploadException($this->upload_error_message($error));
}
$metadata = [];
$metadata['filename'] = pathinfo($name, PATHINFO_BASENAME);
$metadata['tags'] = $tags;
$metadata['source'] = $source;
$event = new DataUploadEvent($tmp_name, $metadata);
$event = new DataUploadEvent($tmp_name, [
'filename' => pathinfo($name, PATHINFO_BASENAME),
'tags' => $tags,
'source' => $source,
]);
send_event($event);
if (count($event->images) == 0) {
throw new UploadException("MIME type not supported: " . $event->mime);
@ -355,7 +355,9 @@ class Upload extends Extension
foreach($event->images as $image) {
$results[] = new UploadSuccess($name, $image->id);
}
$database->execute("RELEASE SAVEPOINT upload");
} catch (UploadException $ex) {
$database->execute("ROLLBACK TO SAVEPOINT upload");
$results[] = new UploadError($name, $ex->getMessage());
}
}
@ -368,12 +370,13 @@ class Upload extends Extension
*/
private function try_transload(string $url, array $tags, string $source = null): array
{
global $page, $config, $user;
global $page, $config, $user, $database;
$results = [];
$tmp_filename = tempnam(ini_get('upload_tmp_dir'), "shimmie_transload");
try {
$database->execute("SAVEPOINT upload");
// Fetch file
$headers = fetch_url($url, $tmp_filename);
if (is_null($headers)) {
@ -410,7 +413,9 @@ class Upload extends Extension
foreach($event->images as $image) {
$results[] = new UploadSuccess($url, $image->id);
}
$database->execute("RELEASE SAVEPOINT upload");
} catch (UploadException $ex) {
$database->execute("ROLLBACK TO SAVEPOINT upload");
$results[] = new UploadError($url, $ex->getMessage());
} finally {
if (file_exists($tmp_filename)) {

View file

@ -64,11 +64,10 @@ class UploadTest extends ShimmiePHPUnitTestCase
{
$this->post_image("tests/pbx_screenshot.jpg", "pbx computer screenshot");
try {
$e = $this->assertException(UploadException::class, function () {
$this->post_image("tests/pbx_screenshot.jpg", "pbx computer screenshot");
} catch (UploadException $e) {
$this->assertStringContainsString("already has hash", $e->getMessage());
}
});
$this->assertStringContainsString("already has hash", $e->getMessage());
}
public function testRejectUnknownFiletype()
@ -81,12 +80,10 @@ class UploadTest extends ShimmiePHPUnitTestCase
{
// FIXME: huge.dat is rejected for other reasons; manual testing shows that this works
file_put_contents("data/huge.jpg", file_get_contents("tests/pbx_screenshot.jpg") . str_repeat("U", 1024 * 1024 * 3));
try {
$e = $this->assertException(UploadException::class, function () {
$this->post_image("data/huge.jpg", "test");
$this->fail("Uploading huge.jpg didn't fail...");
} catch (UploadException $e) {
$this->assertEquals("File too large (3.0MB > 1.0MB)", $e->error);
}
});
unlink("data/huge.jpg");
$this->assertEquals("File too large (3.0MB > 1.0MB)", $e->getMessage());
}
}