Opened 6 years ago

Closed 6 years ago

#1659 closed bug (fixed)

2MB upload limit for images

Reported by: globetrotter_tt Owned by: jsfan
Priority: critical Milestone: 1.0
Component: BW Gallery Keywords: upload limit, gallery, profile picture
Cc:

Description

Issue:

  • Apparently more and more people are trying to upload pictures that exceed our file size limit of 2MB. (problably nobody knows about this fine script ;-)
  • Additionaly we show a "success message" when a user uploads a bigger picture, but the picture was not uploaded. This is of course quite confusing.

Suggestion:

  • Add the info about the file size limit in edimyprofile
  • Show a flash message when someone tries to upload bigger files, that the upload was not succesful.

Change History (26)

comment:1 Changed 6 years ago by crumbking

This is a big issue. And I would not trust that lots of people know how to resize pictures.

Can we not increase that limit? Let's say 4 MB file size or even more??

If the disc space isn't enough we could resize them and safe them afterwards?

comment:2 Changed 6 years ago by jsfan

+1 for upping the upload size (easily done)

However, I'd up it significantly (maybe 16MB) and then resize to maximum dimensions. That requires code changes. Currently we have about 5G worth of images, so there is still room to go for larger images.

I'd aim at an image size of 1024x768 max. That is plenty for the purposes of BW and probably still only results in files of about 2MB or even less (with not too high a compression factor).

comment:3 Changed 6 years ago by globetrotter_tt

+1 for resizing images to 1024x768 px or similar if keeping the proportion. I noticed also that the quality of an image is reduced after uploading. Maybe this can also be improved?

comment:4 Changed 6 years ago by jsfan

Sounds like there is post-processing happening already them. Maybe it goes through GDlib. I'll have a look. Preserving aspect ratio shouldn't be an issue.

comment:5 follow-up: Changed 6 years ago by James_Oder_Dave

Even with the size set to 16mb, is it possible to check that the image a user is trying to upload is under this size?

Setting to 16mb should be fine, but the issue isn't necessarily the size of the image but feedback that users are given when they go over the upload size.

comment:6 follow-up: Changed 6 years ago by jsfan

Looks like we currently save the original file along with several scaled versions. Unofrtunately, there also seem to be two classes that do scaling (MOD_layoutbits and MOD_images_Image). Not sure if both are in use.

I'd suggest to scale all uploaded files to a maximum resolution (keeping aspect ratio of course) and discard the original file if it was larger. The scaled versions I'd then move into a cache directory where they can be removed any time and are re-created on demand. That will save a lot of space in the long run because all inactive members whose profiles never get any hits will never have those images regenerated once they have been deleted.

comment:7 in reply to: ↑ 5 Changed 6 years ago by crumbking

Replying to James_Oder_Dave:

Even with the size set to 16mb, is it possible to check that the image a user is trying to upload is under this size?

Setting to 16mb should be fine, but the issue isn't necessarily the size of the image but feedback that users are given when they go over the upload size.

+1 for a check and a flash notice after hitting the upload button

  • if above 16MB error flash notice
  • if below and everything is fine: green flash notice

We could use the flash functionality Meinhard introduced with the comments.

comment:8 in reply to: ↑ 6 Changed 6 years ago by crumbking

Replying to jsfan:

Looks like we currently save the original file along with several scaled versions. Unofrtunately, there also seem to be two classes that do scaling (MOD_layoutbits and MOD_images_Image). Not sure if both are in use.

I even guess we have 3 classes. I have seen another one for the group pictures.

+1 one for merging into one class

I'd suggest to scale all uploaded files to a maximum resolution (keeping aspect ratio of course) and discard the original file if it was larger. The scaled versions I'd then move into a cache directory where they can be removed any time and are re-created on demand. That will save a lot of space in the long run because all inactive members whose profiles never get any hits will never have those images regenerated once they have been deleted.

+1

comment:9 Changed 6 years ago by coroa

although there exists still the function _getThumb in MOD_layoutbits it is effectively disabled:

private static function _getThumb(...) { 
...
// method appears to work in old memberphotos folder, so I'm disabling it for now
/* if($file == "") */ return null;
...
}

the only time it might be used is via

rox.view.php:teasermain() -> MOD_layoutbits::smallUserPic_userid(...) -> MOD_layoutbits::_getThumb(...)

one should drop _getThumb and the reference to it in smallUserPic_userid.

MOD_images_Image is probably the closest thing to the general image resizing code used by the forum as well as the profile to GENERATE thumbnails.

Links or paths to those thumbnails are then computed in MOD_layoutbits and its various thumbnail related functions (they don't generate any, only return them)

The only stray code which doesn't use these to facilities is in the searchmembers app:

searchmembers.model.php:LinkWithPicture()->getthumb()

these should probably be dropped completely and just replaced by MOD_layoutbits::linkWithPicture

comment:10 Changed 6 years ago by jsfan

  • Owner set to jsfan
  • Status changed from new to assigned

comment:11 Changed 6 years ago by TimLoal

  • Component changed from BW Profile to BW Gallery

comment:12 Changed 6 years ago by jsfan

  • Milestone changed from Future to 1.0

comment:13 Changed 6 years ago by jsfan

  • Owner changed from jsfan to globetrotter_tt

comment:14 Changed 6 years ago by jsfan

Ready for testing. Deployed alpha at commit 4973ce8.

comment:15 Changed 6 years ago by TimLoal

Great work! :)

Its not 100% clear, if you have deployed this on alpha, but it sounds like you have.

Can you confirm?

Can you also provide a link or links to the affected areas, so that we can test and get it live and provide a short description of what to test/expect and how this should have changed from the existing functionality/behaviour?

LnP

comment:16 Changed 6 years ago by crumbking

I tested the profile pic upload. Just for a test I uploaded the wrong file type (mp3) with 2.8 MB size. I ended up in http://alpha.bewelcome.org/editmyprofile/finish with a "Your profile was successfuly updated" which is the wrong message and I still have my old profile pic.

I guess we should check before the upload for the right file type. If it's wrong show a message.

comment:17 Changed 6 years ago by jsfan

@crumbking: Thanks for that hint. I hadn't noticed that this wasn't handled properly, either. I'll add another check.

I have found out that there are no error messages shown for gallery upload, either. I suspect that this somehow relates to #1732 because there is some kind of redirect now which might not have existed earlier. I'll have a look at this as well.

comment:18 Changed 6 years ago by crumbking

@jsfan:
I'm not sure but did meinhard not implemented some user redirect/info after upload before a while for the gallery? I can't remember ;-)

I checked the gallery upload locally and I'm redirected to the latest pics page (gallery/show/user/crumbking/images) which is fine but I think the max. upload of 2 MB is still in place. Should I change some server stuff locally?

comment:19 Changed 6 years ago by jsfan

  • Owner changed from globetrotter_tt to jsfan

There is more work to do. I'll add the missing error message on the profile image upload and add proper error handling in the process. Maybe I can even fix #1732 in the process as well. :)

Related: #1676

Last edited 6 years ago by jsfan (previous) (diff)

comment:21 Changed 6 years ago by crumbking

Seems to work after a first test.

Minor thing: Somehow it test for the right file type after the upload. Maybe it's possible to check already by the right type directly after hitting the upload button.

In case you upload 10 MB it takes a while and only after the upload you get the error. It could reduce server work load....

comment:22 Changed 6 years ago by jsfan

I don't think you can check for the file type before upload. You could only rely on file extensions then which is a dangerous thing. At most you could ask for a second confirmation, I suppose. All the magic that can be done after upload needs the file to be on the server already.

To be honest, I suspect it will be quite rare that someone will upload a large file which is not an image. So, I don't think this is too important. But I wouldn't object to some JavaScript that reads the extension and displays a warning if it doesn't fit.

comment:23 Changed 6 years ago by jsfan

Fixed the file type check on profile picture now.

Also, displaying a notice on maximum file size as in gallery.

comment:24 Changed 6 years ago by jsfan

Looks like I overdid it on the error checking. I now throw an error when someone didn't even try to update the profile image. I'll have to fix that... :(

comment:25 Changed 6 years ago by jsfan

Fixed and deployed on alpha (523b4c0).

comment:26 Changed 6 years ago by globetrotter_tt

  • Resolution set to fixed
  • Status changed from assigned to closed

I had no further problems with uploading big image files, though I did not test it with a photo >16MB, because i could not find one on my disk.

Note: See TracTickets for help on using tickets.