kaffa / textpattern

Automatically exported from code.google.com/p/textpattern
0 stars 0 forks source link

image_data() goes to rampage on error #335

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
image_data() isn't the safest function when it comes to error scenarios. The 
function has a revert functionality that is fired on error.

When an upload error occurs during final file moving, the function does few 
things:

* Removes the row it created to the table.
* Resets auto-increment value to the row's ID it created.

This is done because the function inserts a row to the table before moving the 
uploaded temporary file. This is done to get the file's ID which is used to 
construct the name.

This reverting does impose problems. If the action is an edit to a existing 
image, the whole existing row is deleted from the table. The original images 
are left to the folder stranded (all or one size variation depending on where 
the error occurs).

The auto-increment index reset has a risk of using a wrong value, even when 
it's not an edit. If there are two barrel upload going, and the later fails, 
the index gets a wrong value. Same issue happens if the error occurs on an any 
but not on the most recent image.

Some other smaller things that may need looking into:

* The function replaces the original author on edits.
* ...and the date.
* Overwrites $ID global.

Original issue reported on code.google.com by jukka.m.svahn on 5 Dec 2012 at 7:07

GoogleCodeExporter commented 8 years ago
Purposed changes:

* Only delete row on the initial upload. On edits keep the row and original 
images (if possible) intact.
* Never reset auto-increment value. Let it increase if it wishes to do so.
* Set author only on the initial upload.

Original comment by jukka.m.svahn on 5 Dec 2012 at 7:12

GoogleCodeExporter commented 8 years ago
> * Set author only on the initial upload.

I'd rather set the author on each new upload (but not on any other edit.) On 
uploads, the uploader has access to the new image file, so she might as well be 
the author.

Original comment by r.wetzlmayr on 5 Dec 2012 at 8:06

GoogleCodeExporter commented 8 years ago
This issue was closed by revision r4931.

Original comment by jukka.m.svahn on 5 Dec 2012 at 8:44