Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2 issues : image file type change to ’.jpeg‘, and big image file upload error. #62

Closed
DearTanker opened this issue Mar 15, 2017 · 9 comments

Comments

@DearTanker
Copy link

  • flagrow/upload extension version: 0.4.8 and some previous
  • flarum version: beta6
  • Upload adapter causing issues:

issues 1 : when .jpg upload , it auto change to .jpeg .

issues 2 : when upload a image with more than 2mb , get error and upload fail .PHP configer is ok with 20mb max upload(post_max_size、upload_max_filesize),and 40s max_execution_time.

@DearTanker DearTanker changed the title 2 issues image file type change to ’.jpeg‘, and big image file upload error. 2 issues : image file type change to ’.jpeg‘, and big image file upload error. Mar 15, 2017
@luceos
Copy link
Contributor

luceos commented Mar 15, 2017

  • What is the exact problem you experience with renaming .jpg to .jpeg?
  • Did you update the max file size in the extension setting page too, see image:

image

@DearTanker
Copy link
Author

  • auto rename is not necessary and it's just wrong
  • of course, i have set to 20480

qq20170315-154449 2x

@luceos
Copy link
Contributor

luceos commented Mar 15, 2017

auto rename is not necessary and it's just wrong

I disagree. But feel free to offer proper argumentation so at least I have the chance to reconsider, because "not necessary" and "just wrong" doesn't tell me a thing. The reason to do so is to force actual mime types to be used. This prevents abuse of the upload feature. Feel free to read this article and tell me what you think afterward.

Related to the second thing, what error do you get. Uploads still have to be handled by the frontend, I haven't taken into account (nor done any tests with) long running uploads yet (eg movies of several hundreds megabytes).

@jordanjay29
Copy link

Mind you, there are 6 different accepted JPEG extensions (.jpg, .jpeg, .jpe, .jif, .jfif, .jfi) and they all should work the same. That's because the MIME type is all JPEG. So, no, it's not wrong and while it might not be necessary it provides some consistency for the extension (which now can look for just one file type instead of 6).

@DearTanker
Copy link
Author

@jordanjay29 @luceos so great thinking. i have no word to comment.

just 3.5mb jpg and i get error.

image

but when i use Image Upload 0.3.1, all fine.

@DearTanker
Copy link
Author

by the way, jpg is more popular than jpeg, why not auto change to jpg.

@luceos
Copy link
Contributor

luceos commented Mar 15, 2017

Do you see any errors in your webserver log or storage/logs/flarum.log file that might relate to the upload issue?

@ghost
Copy link

ghost commented Mar 21, 2017

@DearTanker You should check upload_max_filesize & post_max_size in php.ini Also make sure you're editing the correct php.ini. There is usually one for CLI & one for php-fpm.

There are similar limits within the webserver config.

for nginx check: client_max_body_size

@luceos
Copy link
Contributor

luceos commented Apr 23, 2017

I've improved error handling of the upload methods. They now verify ini-settings and throw related exceptions. Not going to change logic for jpeg file types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants